[InstCombine] Preserve FMF in foldLogicOfFCmps.

[craig.topper]

Summary

This patch intersects the fast math flags from the two fcmps instead
of dropping them.

I poked at this a bunch with Alive2 for nnan and ninf flags and it seemed
to check out. With the other flags it told me "Couldn't prove the
correctness of the transformation". Not sure if I should just preserve
nnan and ninf?

[lebedev.ri]

https://llvm.org/docs/LangRef.html#id306 says:

Any set of fast-math flags are legal on an fcmp instruction, but the only flags that have any effect on its semantics are those that allow assumptions to be made about the values of input arguments; namely nnan, ninf, and reassoc. See Fast-Math Flags for more information.

So please add tests with reassoc?

[craig.topper]

In D121243#3368116, @lebedev.ri wrote:

https://llvm.org/docs/LangRef.html#id306 says:

Any set of fast-math flags are legal on an fcmp instruction, but the only flags that have any effect on its semantics are those that allow assumptions to be made about the values of input arguments; namely nnan, ninf, and reassoc. See Fast-Math Flags for more information.

So please add tests with reassoc?

Alive2 can't seem to verify the reassoc preservation. Is it better to drop everything but nnan and ninf?

[spatel]

In D121243#3368116, @lebedev.ri wrote:

https://llvm.org/docs/LangRef.html#id306 says:

Any set of fast-math flags are legal on an fcmp instruction, but the only flags that have any effect on its semantics are those that allow assumptions to be made about the values of input arguments; namely nnan, ninf, and reassoc. See Fast-Math Flags for more information.

So please add tests with reassoc?

"fast" implicitly covers that, so more tests with individual flags wouldn't add much IMO.

For this transform only -- because we are guaranteed to repeat the values in each fcmp -- I was expecting that we could 'or' the relevant flags. But there's a surprising corner case with true and false fcmp predicates according to Alive2:
https://alive2.llvm.org/ce/z/Nffn3L

The behavior -- blocking poison via predicate? -- does not seem to be documented in the LangRef.

[craig.topper]

Adding a TODO for unioning the flags. Intersecting at least an improvement for many cases where both instructions have the same flags.

[nlopes]

For this transform only -- because we are guaranteed to repeat the values in each fcmp -- I was expecting that we could 'or' the relevant flags. But there's a surprising corner case with true and false fcmp predicates according to Alive2:
https://alive2.llvm.org/ce/z/Nffn3L

The behavior -- blocking poison via predicate? -- does not seem to be documented in the LangRef.

oops, that seems to be a bug in Alive2.
We convert fcmp true to true, but that is wrong as we lose the fast-math flags. Let me fix that.

[nlopes]

In D121243#3368365, @nlopes wrote:

For this transform only -- because we are guaranteed to repeat the values in each fcmp -- I was expecting that we could 'or' the relevant flags. But there's a surprising corner case with true and false fcmp predicates according to Alive2:
https://alive2.llvm.org/ce/z/Nffn3L

The behavior -- blocking poison via predicate? -- does not seem to be documented in the LangRef.

oops, that seems to be a bug in Alive2.
We convert fcmp true to true, but that is wrong as we lose the fast-math flags. Let me fix that.

Fixed (not poison anymore):

define i1 @src(double %a, double %b) {
%0:
  %cmp = fcmp ninf true double %a, %b
  %cmp1 = fcmp nnan oge double %a, %b
  %retval = and i1 %cmp, %cmp1
  ret i1 %retval
}
=>
define i1 @tgt(double %a, double %b) {
%0:
  %r = fcmp nnan ninf oeq double %a, %b
  ret i1 %r
}
Transformation doesn't verify!

ERROR: Value mismatch

Example:
double %a = #x0015250a74408022 (0.000000000000?)
double %b = #x88018b0635082009 (-0.000000000000?)

Source:
i1 %cmp = #x1 (1)
i1 %cmp1 = #x1 (1)
i1 %retval = #x1 (1)

Target:
i1 %r = #x0 (0)
Source value: #x1 (1)
Target value: #x0 (0)

[spatel]

In D121243#3368365, @nlopes wrote:

We convert fcmp true to true, but that is wrong as we lose the fast-math flags. Let me fix that.

Thanks for the fix! So we can remove the code comment about the fcmp predicates, and I think this patch is fine.
Note that we've (slowly) been moving away from relying on FMF on fcmp and trying to put FMF directly on all FP values instead. I'm not sure how many transforms are predicated on FMF on fcmp at this point, but it may go to zero some day.

[craig.topper]

In D121243#3368552, @spatel wrote:

In D121243#3368365, @nlopes wrote:

We convert fcmp true to true, but that is wrong as we lose the fast-math flags. Let me fix that.

Thanks for the fix! So we can remove the code comment about the fcmp predicates, and I think this patch is fine.
Note that we've (slowly) been moving away from relying on FMF on fcmp and trying to put FMF directly on all FP values instead. I'm not sure how many transforms are predicated on FMF on fcmp at this point, but it may go to zero some day.

So leave the union TODO, but drop the uncertainty about FCMP_TRUE/FALSE?

[craig.topper]

Remove mention of FCMP_TRUE/FALSE from FIXME.

[spatel]

In D121243#3368645, @craig.topper wrote:

In D121243#3368552, @spatel wrote:

In D121243#3368365, @nlopes wrote:

We convert fcmp true to true, but that is wrong as we lose the fast-math flags. Let me fix that.

Thanks for the fix! So we can remove the code comment about the fcmp predicates, and I think this patch is fine.
Note that we've (slowly) been moving away from relying on FMF on fcmp and trying to put FMF directly on all FP values instead. I'm not sure how many transforms are predicated on FMF on fcmp at this point, but it may go to zero some day.

So leave the union TODO, but drop the uncertainty about FCMP_TRUE/FALSE?

Yes - LGTM.

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

 //===- InstCombineAndOrXor.cpp --------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
Value *InstCombinerImpl::foldLogicOfFCmps(FCmpInst *LHS, FCmpInst *RHS,
   // Since (R & CC0) and (R & CC1) are either R or 0, we actually have this:
   //    bool(R & CC0) || bool(R & CC1)
   //  = bool((R & CC0) | (R & CC1))
   //  = bool(R & (CC0 | CC1)) <= by reversed distribution (contribution? ;)
   if (LHS0 == RHS0 && LHS1 == RHS1) {
     unsigned FCmpCodeL = getFCmpCode(PredL);
     unsigned FCmpCodeR = getFCmpCode(PredR);
     unsigned NewPred = IsAnd ? FCmpCodeL & FCmpCodeR : FCmpCodeL | FCmpCodeR;
+
+    // Intersect the fast math flags.
+    IRBuilder<>::FastMathFlagGuard FMFG(Builder);
+    FastMathFlags FMF = LHS->getFastMathFlags();
+    FMF &= RHS->getFastMathFlags();
+    Builder.setFastMathFlags(FMF);
+
     return getFCmpValue(NewPred, LHS0, LHS1, Builder);
   }
 
   if ((PredL == FCmpInst::FCMP_ORD && PredR == FCmpInst::FCMP_ORD && IsAnd) ||
       (PredL == FCmpInst::FCMP_UNO && PredR == FCmpInst::FCMP_UNO && !IsAnd)) {
     if (LHS0->getType() != RHS0->getType())
       return nullptr;
 

llvm/test/Transforms/InstCombine/and-fcmp.ll

 ; CHECK-LABEL: @auto_gen_135_logical(
 ; CHECK-NEXT:    ret i1 true
 ;
   %cmp = fcmp true double %a, %b
   %cmp1 = fcmp true double %a, %b
   %retval = select i1 %cmp, i1 %cmp1, i1 false
   ret i1 %retval
 }
+
+define i1 @intersect_fmf_1(double %a, double %b) {
+; CHECK-LABEL: @intersect_fmf_1(
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp fast oeq double [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
+;
+  %cmp = fcmp fast ole double %a, %b
+  %cmp1 = fcmp fast oge double %a, %b
+  %retval = and i1 %cmp, %cmp1
+  ret i1 %retval
+}
+
+define i1 @intersect_fmf_2(double %a, double %b) {
+; CHECK-LABEL: @intersect_fmf_2(
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp oeq double [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
+;
+  %cmp = fcmp fast ole double %a, %b
+  %cmp1 = fcmp oge double %a, %b
+  %retval = and i1 %cmp, %cmp1
+  ret i1 %retval
+}
+
+define i1 @intersect_fmf_3(double %a, double %b) {
+; CHECK-LABEL: @intersect_fmf_3(
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp oeq double [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
+;
+  %cmp = fcmp ole double %a, %b
+  %cmp1 = fcmp fast oge double %a, %b
+  %retval = and i1 %cmp, %cmp1
+  ret i1 %retval
+}
+
+define i1 @intersect_fmf_4(double %a, double %b) {
+; CHECK-LABEL: @intersect_fmf_4(
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp oeq double [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
+;
+  %cmp = fcmp ninf ole double %a, %b
+  %cmp1 = fcmp nnan oge double %a, %b
+  %retval = and i1 %cmp, %cmp1
+  ret i1 %retval
+}

llvm/test/Transforms/InstCombine/or-fcmp.ll

 ; CHECK-LABEL: @auto_gen_135_logical(
 ; CHECK-NEXT:    ret i1 true
 ;
   %cmp = fcmp true double %a, %b
   %cmp1 = fcmp true double %a, %b
   %retval = select i1 %cmp, i1 true, i1 %cmp1
   ret i1 %retval
 }
+
+define i1 @intersect_fmf_1(double %a, double %b) {
+; CHECK-LABEL: @intersect_fmf_1(
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp fast one double [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
+;
+  %cmp = fcmp fast olt double %a, %b
+  %cmp1 = fcmp fast ogt double %a, %b
+  %retval = or i1 %cmp, %cmp1
+  ret i1 %retval
+}
+
+define i1 @intersect_fmf_2(double %a, double %b) {
+; CHECK-LABEL: @intersect_fmf_2(
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp one double [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
+;
+  %cmp = fcmp fast olt double %a, %b
+  %cmp1 = fcmp ogt double %a, %b
+  %retval = or i1 %cmp, %cmp1
+  ret i1 %retval
+}
+
+define i1 @intersect_fmf_3(double %a, double %b) {
+; CHECK-LABEL: @intersect_fmf_3(
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp one double [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
+;
+  %cmp = fcmp olt double %a, %b
+  %cmp1 = fcmp fast ogt double %a, %b
+  %retval = or i1 %cmp, %cmp1
+  ret i1 %retval
+}
+
+define i1 @intersect_fmf_4(double %a, double %b) {
+; CHECK-LABEL: @intersect_fmf_4(
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp one double [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    ret i1 [[TMP1]]
+;
+  %cmp = fcmp ninf olt double %a, %b
+  %cmp1 = fcmp nnan ogt double %a, %b
+  %retval = or i1 %cmp, %cmp1
+  ret i1 %retval
+}