[PATCH] D48085: [DAGCombiner] restrict (float)((int) f) --> ftrunc with no-signed-zeros

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11049-11050
+  // replacing casts with a libcall. We also must be allowed to ignore -0.0
+  // because FTRUNC will return -0.0 for (-1.0, -0.0), but using integer
+  // conversions would return +0.0.
+  // FIXME: We should be able to use node-level FMF here.
----------------
lebedev.ri wrote:
> Couldn't we do `and (ftrunc %x), (-1 >> 1)`, i.e. unset the sign bit?
> Or is that worse than not transforming to `ftrunc` in the first place?
You'd want to keep this in FP, so I think that would be:
fabs (ftrunc x)

And that could be better than the alternative in some cases. See D44909 for what this looks like without using ftrunc. If the sequence is more than 2 converts, fabs is likely a winner.

But given the fallout from the original patch, I expect this is the patch most people would prefer. :)
We'll avoid the UB headaches for most code, and code that cares about FP perf likely has some loosened FP constraints anyway.


https://reviews.llvm.org/D48085