[FPEnv][InstSimplify] Fold constrained X + -0.0 ==> X

[kpn]

Summary

Currently the fadd optimizations in InstSimplify don't know how to do this "X + -0.0 ==> X" fold when using the constrained intrinsics. This adds the support.

This review is derived from D106362 with some improvements from D107285. I was asked to make a new, smaller version of D106362.

[sepavloff]

LGTM.

llvm/lib/Analysis/InstructionSimplify.cpp

4938

Why not moving this function into header file in this patch? May be to FPEnv.h. You need this function in D107285.

[spatel]

Thanks for the minimal patch!
LGTM - see inline for some cosmetics.

llvm/include/llvm/IR/FPEnv.h

64

Nit: If we don't use the current/correct formatting for a function name (roundingModeMayBe), we're never going to get out of the formatting mess we're in.

Bonus points for fixing all of the functions here (as an NFC preliminary patch). :)

llvm/lib/Analysis/InstructionSimplify.cpp

4957

This is complicated logic, so it would be good to put a code comment/examples on it. Something like:

// With strict/constrained FP, we have these possible edge cases that do not simplify to Op0:
// fadd SNaN, -0.0 --> QNaN
// fadd +0.0, -0.0 --> -0.0 (with round toward negative)

llvm/test/Transforms/InstSimplify/strictfp-fadd.ll

69

Independent of this patch, but I wonder if it is too late to unify the vocabulary on the metadata and enum names.
Ie, why do we have 2 ways to spell "downward" / "TowardNegative", etc?

[kpn]

llvm/include/llvm/IR/FPEnv.h

64

Does this apply to type names that appear in the name of functions? For example, RoundingModeToStr() takes a RoundingMode argument. That was part of why I went with RoundingModeMayBe(). (The rest of the reason was to be consistent with the rest of this file.)

llvm/lib/Analysis/InstructionSimplify.cpp

4938

Yeah, I looked into that. I agree that FPEnv.h is the right place.

But when I tried it I found that FastMathFlags is in Operator.h, and that seems like a heavy file to include in FPEnv.h. In the near future I'm planning on trying to move the FastMathFlags out into a standalone header that I can pull into Operator.h and FPEnv.h, and that will allow me to move canIgnoreSNaN() into FPEnv.h.

4957

Can do!

llvm/test/Transforms/InstSimplify/strictfp-fadd.ll

69

Good question. I see that llvm::RoundingMode mirrors the names in IEEE-754. So if we change anything it should be the strings accepted by the constrained intrinsics. I guess that's something someone could do especially since the constrained intrinsics are still in the experimental namespace? With upgrade support of course.

[spatel]

llvm/include/llvm/IR/FPEnv.h

64

I didn't notice that quirk of naming...
In this case, I think we can fix it with "canRoundingModeBe" ?
That may lower our score on an English test, but still in compliance with:
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

[kpn]

llvm/include/llvm/IR/FPEnv.h

64

That works!

Oh, @sepavloff, are you good with me adding, in a different commit, the word "convert" to the beginning of these four conversion functions?

[sepavloff]

llvm/include/llvm/IR/FPEnv.h

64

I am OK with it.

llvm/include/llvm/IR/FPEnv.h

 /// input in constrained intrinsic exception behavior metadata.
 Optional<StringRef> ExceptionBehaviorToStr(fp::ExceptionBehavior);
 
 /// Returns true if the exception handling behavior and rounding mode
 /// match what is used in the default floating point environment.
 inline bool isDefaultFPEnvironment(fp::ExceptionBehavior EB, RoundingMode RM) {
   return EB == fp::ebIgnore && RM == RoundingMode::NearestTiesToEven;
 }
+
+/// Returns true if the rounding mode RM may be QRM at compile time or
+/// at run time.
+inline bool canRoundingModeBe(RoundingMode RM, RoundingMode QRM) {
+  return RM == QRM || RM == RoundingMode::Dynamic;
+}
 }
 #endif

llvm/lib/Analysis/InstructionSimplify.cpp

for (Value *V : Ops) {
     } else if (ExBehavior != fp::ebStrict) {
       if (IsNan)
         return propagateNaN(cast<Constant>(V));
     }
   }
   return nullptr;
 }
 
+// TODO: Move this out to a header file:
+static inline bool canIgnoreSNaN(fp::ExceptionBehavior EB, FastMathFlags FMF) {
+  return (EB == fp::ebIgnore || FMF.noNaNs());
+}
+
 /// Given operands for an FAdd, see if we can fold the result.  If not, this
 /// returns null.
 static Value *
 SimplifyFAddInst(Value *Op0, Value *Op1, FastMathFlags FMF,
                  const SimplifyQuery &Q, unsigned MaxRecurse,
                  fp::ExceptionBehavior ExBehavior = fp::ebIgnore,
                  RoundingMode Rounding = RoundingMode::NearestTiesToEven) {
   if (isDefaultFPEnvironment(ExBehavior, Rounding))
     if (Constant *C = foldOrCommuteConstant(Instruction::FAdd, Op0, Op1, Q))
       return C;
 
   if (Constant *C = simplifyFPOp({Op0, Op1}, FMF, Q, ExBehavior, Rounding))
     return C;
 
-  if (!isDefaultFPEnvironment(ExBehavior, Rounding))
-    return nullptr;
-
   // fadd X, -0 ==> X
+  // With strict/constrained FP, we have these possible edge cases that do
+  // not simplify to Op0:
+  // fadd SNaN, -0.0 --> QNaN
+  // fadd +0.0, -0.0 --> -0.0 (but only with round toward negative)
+  if (canIgnoreSNaN(ExBehavior, FMF) &&
+      (!canRoundingModeBe(Rounding, RoundingMode::TowardNegative) ||
+       FMF.noSignedZeros()))
     if (match(Op1, m_NegZeroFP()))
       return Op0;
 
+  if (!isDefaultFPEnvironment(ExBehavior, Rounding))
+    return nullptr;
+
   // fadd X, 0 ==> X, when we know X is not -0
   if (match(Op1, m_PosZeroFP()) &&
       (FMF.noSignedZeros() || CannotBeNegativeZero(Op0, Q.TLI)))
     return Op0;
 
   // With nnan: -X + X --> 0.0 (and commuted variant)
   // We don't have to explicitly exclude infinities (ninf): INF + -INF == NaN.
   // Negative zeros are allowed because we always end up with positive zero:

llvm/test/Transforms/InstSimplify/strictfp-fadd.ll

;
   ret <2 x float> %ret
 }
 
 define float @fadd_x_n0_neginf(float %a) #0 {
 ; CHECK-LABEL: @fadd_x_n0_neginf(
 ; CHECK-NEXT:    [[RET:%.*]] = call float @llvm.experimental.constrained.fadd.f32(float [[A:%.*]], float -0.000000e+00, metadata !"round.downward", metadata !"fpexcept.ignore") #[[ATTR0]]
 ; CHECK-NEXT:    ret float [[RET]]
 ;
   %ret = call float @llvm.experimental.constrained.fadd.f32(float %a, float -0.0, metadata !"round.downward", metadata !"fpexcept.ignore") #0
   ret float %ret
 }
 
 define <2 x float> @fadd_vec_x_n0_neginf(<2 x float> %a) #0 {
 ; CHECK-LABEL: @fadd_vec_x_n0_neginf(
 ; CHECK-NEXT:    [[RET:%.*]] = call <2 x float> @llvm.experimental.constrained.fadd.v2f32(<2 x float> [[A:%.*]], <2 x float> <float -0.000000e+00, float -0.000000e+00>, metadata !"round.downward", metadata !"fpexcept.ignore") #[[ATTR0]]
 ; CHECK-NEXT:    ret <2 x float> [[RET]]
 ;
;
   %ret = call <2 x float> @llvm.experimental.constrained.fadd.v2f32(<2 x float> %a, <2 x float><float -0.0, float -0.0>, metadata !"round.dynamic", metadata !"fpexcept.ignore") #0
   ret <2 x float> %ret
 }
 
 ; The rounding mode here needs to not be { tonearest, downward, dynamic }.
 ; Test one of the remaining rounding modes and the rest will be fine.
 define float @fadd_x_n0_towardzero(float %a) #0 {
 ; CHECK-LABEL: @fadd_x_n0_towardzero(
-; CHECK-NEXT:    [[RET:%.*]] = call float @llvm.experimental.constrained.fadd.f32(float [[A:%.*]], float -0.000000e+00, metadata !"round.towardzero", metadata !"fpexcept.ignore") #[[ATTR0]]
-; CHECK-NEXT:    ret float [[RET]]
+; CHECK-NEXT:    ret float [[A:%.*]]
 ;
   %ret = call float @llvm.experimental.constrained.fadd.f32(float %a, float -0.0, metadata !"round.towardzero", metadata !"fpexcept.ignore") #0
   ret float %ret
 }
 
 ; The rounding mode here needs to not be { tonearest, downward, dynamic }.
 ; Test one of the remaining rounding modes and the rest will be fine.
 define <2 x float> @fadd_vec_x_n0_towardzero(<2 x float> %a) #0 {
 ; CHECK-LABEL: @fadd_vec_x_n0_towardzero(
-; CHECK-NEXT:    [[RET:%.*]] = call <2 x float> @llvm.experimental.constrained.fadd.v2f32(<2 x float> [[A:%.*]], <2 x float> <float -0.000000e+00, float -0.000000e+00>, metadata !"round.towardzero", metadata !"fpexcept.ignore") #[[ATTR0]]
-; CHECK-NEXT:    ret <2 x float> [[RET]]
+; CHECK-NEXT:    ret <2 x float> [[A:%.*]]
 ;
   %ret = call <2 x float> @llvm.experimental.constrained.fadd.v2f32(<2 x float> %a, <2 x float><float -0.0, float -0.0>, metadata !"round.towardzero", metadata !"fpexcept.ignore") #0
   ret <2 x float> %ret
 }
 
 define float @fadd_nnan_x_n0_ebmaytrap(float %a) #0 {
 ; CHECK-LABEL: @fadd_nnan_x_n0_ebmaytrap(
-; CHECK-NEXT:    [[RET:%.*]] = call nnan float @llvm.experimental.constrained.fadd.f32(float [[A:%.*]], float -0.000000e+00, metadata !"round.tonearest", metadata !"fpexcept.maytrap") #[[ATTR0]]
-; CHECK-NEXT:    ret float [[RET]]
+; CHECK-NEXT:    ret float [[A:%.*]]
 ;
   %ret = call nnan float @llvm.experimental.constrained.fadd.f32(float %a, float -0.0, metadata !"round.tonearest", metadata !"fpexcept.maytrap") #0
   ret float %ret
 }
 
 define <2 x float> @fadd_vec_nnan_x_n0_ebmaytrap(<2 x float> %a) #0 {
 ; CHECK-LABEL: @fadd_vec_nnan_x_n0_ebmaytrap(
-; CHECK-NEXT:    [[RET:%.*]] = call nnan <2 x float> @llvm.experimental.constrained.fadd.v2f32(<2 x float> [[A:%.*]], <2 x float> <float -0.000000e+00, float -0.000000e+00>, metadata !"round.tonearest", metadata !"fpexcept.maytrap") #[[ATTR0]]
-; CHECK-NEXT:    ret <2 x float> [[RET]]
+; CHECK-NEXT:    ret <2 x float> [[A:%.*]]
 ;
   %ret = call nnan <2 x float> @llvm.experimental.constrained.fadd.v2f32(<2 x float> %a, <2 x float><float -0.0, float -0.0>, metadata !"round.tonearest", metadata !"fpexcept.maytrap") #0
   ret <2 x float> %ret
 }
 
 define float @fadd_nnan_x_n0_ebstrict(float %a) #0 {
 ; CHECK-LABEL: @fadd_nnan_x_n0_ebstrict(
 ; CHECK-NEXT:    [[RET:%.*]] = call nnan float @llvm.experimental.constrained.fadd.f32(float [[A:%.*]], float -0.000000e+00, metadata !"round.tonearest", metadata !"fpexcept.strict") #[[ATTR0]]
-; CHECK-NEXT:    ret float [[RET]]
+; CHECK-NEXT:    ret float [[A]]
 ;
   %ret = call nnan float @llvm.experimental.constrained.fadd.f32(float %a, float -0.0, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
   ret float %ret
 }
 
 define <2 x float> @fadd_vec_nnan_x_n0_ebstrict(<2 x float> %a) #0 {
 ; CHECK-LABEL: @fadd_vec_nnan_x_n0_ebstrict(
 ; CHECK-NEXT:    [[RET:%.*]] = call nnan <2 x float> @llvm.experimental.constrained.fadd.v2f32(<2 x float> [[A:%.*]], <2 x float> <float -0.000000e+00, float -0.000000e+00>, metadata !"round.tonearest", metadata !"fpexcept.strict") #[[ATTR0]]
-; CHECK-NEXT:    ret <2 x float> [[RET]]
+; CHECK-NEXT:    ret <2 x float> [[A]]
 ;
   %ret = call nnan <2 x float> @llvm.experimental.constrained.fadd.v2f32(<2 x float> %a, <2 x float><float -0.0, float -0.0>, metadata !"round.tonearest", metadata !"fpexcept.strict") #0
   ret <2 x float> %ret
 }
 
 ; Test with a fast math flag set but that flag is not "nnan".
 define float @fadd_ninf_x_n0_ebstrict(float %a) #0 {
 ; CHECK-LABEL: @fadd_ninf_x_n0_ebstrict(