[PATCH] D44550: [InstCombine] canonicalize fcmp+select to fabs

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:1577
+    FCmpInst::Predicate Pred = FCI->getPredicate();
+    if (match(FCI->getOperand(1), m_AnyZeroFP())) {
+      // (X <= +/-0.0) ? (0.0 - X) : X --> fabs(X)
----------------
mike.dvoretsky wrote:
> I think there should be an nnan check here. The patterns in DAGCombiner check it implicitly by requiring condition codes that only occur in FP math under nnan, but this code currently doesn't require that, and all of the patterns would direct NaNs of either sign to the same option, creating the same kind of issue as the current patterns have with zero signs (and 0.0 - NaN shouldn't even do anything).
Can you show an example of how this goes wrong with a NaN input? Is there a requirement that we preserve the sign bit of a NaN value in any of the original code sequences?


https://reviews.llvm.org/D44550