[PATCH] D31398: [X86][X86 intrinsics]Folding cmp(sub(a, b), 0) into cmp(a, b) optimization

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a subscriber: scanon.
spatel added a comment.

If this transform is valid (cc'ing @scanon), then should we also do this for general IR?

  define i1 @fcmpsub(double %x, double %y) {
    %sub = fsub nnan ninf nsz double %x, %y
    %cmp = fcmp nnan ninf nsz ugt double %sub, 0.0
    ret i1 %cmp
  }
  
  define i1 @fcmpsub(double %x, double %y) {
    %cmp = fcmp nnan ninf nsz ugt double %x, %y
    ret i1 %cmp
  }



================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:2401
+        // (No NaNs,no inf,no sign zero)
+        FastMathFlags FMFs = I->getFastMathFlags();
+        if ((FMFs.unsafeAlgebra() || (FMFs.noNaNs() && FMFs.noInfs() &&
----------------
Don't we need to check the FMF of the intrinsic too?


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:2402-2403
+        FastMathFlags FMFs = I->getFastMathFlags();
+        if ((FMFs.unsafeAlgebra() || (FMFs.noNaNs() && FMFs.noInfs() &&
+                                      FMFs.noSignedZeros())) &&
+            isa<ConstantAggregateZero>((II->getArgOperand(1)))) {
----------------
Currently, unsafeAlgebra implies all of the other FMF bits, so checking that bit is redundant here.
If we change the definition of unsafeAlgebra in the future (there was a proposal for this recently), then this check will be wrong. Either way, remove the unsafeAlgebra predicate (unless I'm misunderstanding the constraints of this transform).


https://reviews.llvm.org/D31398