[PATCH] D19178: Broaden FoldItoFPtoI to try and establish whether the integer value fits into the float type
Carlos Liam via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 30 17:51:15 PDT 2016
aarzee added inline comments.
================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1483
@@ +1482,3 @@
+ // If the number is of unknown sign, the highest possibly set bit is the
+ // higher of the lowest known zero and the highest not known zero.
+
----------------
eli.friedman wrote:
> Leftover comment?
I got rid of that in my local copy but I didn't update my diff. Thanks for catching that.
================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1507
@@ +1506,3 @@
+ int BitRange = MostSignificantPossiblySetBit -
+ LeastSignificantPossiblySetBit + 1;
+
----------------
eli.friedman wrote:
> Doesn't the + 1 here cancel out the -1 in the setting of MostSignificantPossiblySetBit? That would make the math a bit more straightforward.
The reasoning behind this is that I wanted the values for both least and most significant possibly set bit to be zero indexed; I thought it might be confusing if one was and the other wasn't.
================
Comment at: test/Transforms/InstCombine/sitofp.ll:272
@@ +271,2 @@
+ ret i32 %D
+}
----------------
eli.friedman wrote:
> Did you find any interesting new cases worth having a separate regression test?
Yes, I will add some tests with unknown sign.
https://reviews.llvm.org/D19178
More information about the llvm-commits
mailing list