[PATCH] D19178: Broaden FoldItoFPtoI to try and establish whether the integer value fits into the float type

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 21 09:26:36 PDT 2016


reames added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1423
@@ +1422,3 @@
+// least significant set bits of abs(X) fits into the mantissa, and that the
+// number of trailing zeros fits into the exponent.
+
----------------
I was suggesting that you first post a patch which implements this optimization for integers which can be represented in floating point without scaling (i.e. without using the exponent for anything non-trivial).  Once that's in, the scaling part of the change becomes far easier to reason about in isolation.

To be clear, I am not going to sign off on the change in its current form.  I don't understand floating point deeply enough to reason it in this way.  You're welcome to find an alternate reviewer, but if you want a LGTM from me, splitting it is going to be required.  


http://reviews.llvm.org/D19178





More information about the llvm-commits mailing list