-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Simplify u128->f32 casts thanks to LLVM r334777 #51872
Copy link
Copy link
Closed
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-enhancementCategory: An issue proposing an enhancement or a PR with one.Category: An issue proposing an enhancement or a PR with one.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.
Metadata
Metadata
Assignees
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-enhancementCategory: An issue proposing an enhancement or a PR with one.Category: An issue proposing an enhancement or a PR with one.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.
Type
Fields
Give feedbackNo fields configured for issues without a type.
This issue tracks two improvements we can make to our codegen after some LLVM updates, to avoid them being buried in a closed issue.
As @nikic pointed out, https://reviews.llvm.org/D47807 defines overflow in u128->f32 casts as resulting in infinity. With that patch, we don't need to manually emit the "is the u128 >=
f32::MAX + 0.5 ULP?" check as we currently do and can go back to just emitting a simpleuitofp(as we have always done for {u8,u16,u32,u64} -> float casts). However, because of our policy of supporting older LLVM versions, there are actually two steps we can take here at different times:The latter is honestly the more important step. I doubt there is any significant code size or performance impact from the extra check, but it would be nice to get rid of the tricky code in rustc that is required to implement it.