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

[michael zuckerman via Phabricator via llvm-commits]

m_zuckerman marked an inline comment as done.
m_zuckerman added a comment.

In https://reviews.llvm.org/D31398#715296, @spatel wrote:

> 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
>   }


You are absolutely right, your transform is valid and we will do it after this patch.
Since the intrinsics are lowered with generic IR, mine patch is still valid and we will need them both for a complete solution.



================
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() &&
----------------
spatel wrote:
> Don't we need to check the FMF of the intrinsic too?
No, we don't need to check, this is implied from the flags of the sub.  
According to  http://llvm.org/docs/LangRef.html#fast-math-flags optimizations can assume that the arguments and the result behave as expected from them. Since the compare uses the result and splits them to two arguments (the same arguments as in the sub) we are still working with the early assumption. 

We can continue with the assumptions as long we will work with the same arguments or the same result.



https://reviews.llvm.org/D31398