[InstSimplify] (X * Y) / Y --> X for relaxed floating-point ops

[spatel]

Summary

This is the FP counterpart that was mentioned in PR35709:
https://bugs.llvm.org/show_bug.cgi?id=35709

I suspect we're missing other similar basic folds for FP reassociations.

[grandinj]

lib/Analysis/InstructionSimplify.cpp

4300

for these kinds of identities, should we not be matching

(X * A * B * ....) / X

?

i.e. whereever we are searching inside a group, we should scan until we hit a non-associative operator or an operator of different precedence ?

[spatel]

lib/Analysis/InstructionSimplify.cpp

4300

It's correct that this patch is too simple to handle that case, but I don't think we want to add more non-constant-time transforms to instsimplify. We should let the reassociation pass handle the bigger jobs...of course, reassociation could handle this case too, but solving the simple cases here means that other passes that use instsimplify as an analysis (GVN, etc) become stronger.

[spatel]

Ping.

[efriedma]

LGTM

include/llvm/IR/PatternMatch.h

 }
 /// Matches a UMax with LHS and RHS in either order.
 template <typename LHS, typename RHS>
 inline MaxMin_match<ICmpInst, LHS, RHS, umax_pred_ty, true>
 m_c_UMax(const LHS &L, const RHS &R) {
   return MaxMin_match<ICmpInst, LHS, RHS, umax_pred_ty, true>(L, R);
 }
 
+/// Matches FAdd with LHS and RHS in either order.
+template <typename LHS, typename RHS>
+inline BinaryOp_match<LHS, RHS, Instruction::FAdd, true>
+m_c_FAdd(const LHS &L, const RHS &R) {
+  return BinaryOp_match<LHS, RHS, Instruction::FAdd, true>(L, R);
+}
+
+/// Matches FMul with LHS and RHS in either order.
+template <typename LHS, typename RHS>
+inline BinaryOp_match<LHS, RHS, Instruction::FMul, true>
+m_c_FMul(const LHS &L, const RHS &R) {
+  return BinaryOp_match<LHS, RHS, Instruction::FMul, true>(L, R);
+}
+
 } // end namespace PatternMatch
 } // end namespace llvm
 
 #endif // LLVM_IR_PATTERNMATCH_H

lib/Analysis/InstructionSimplify.cpp

static Value *SimplifyFDivInst(Value *Op0, Value *Op1, FastMathFlags FMF,
   // 0 / X -> 0
   // Requires that NaNs are off (X could be zero) and signed zeroes are
   // ignored (X could be positive or negative, so the output sign is unknown).
   if (FMF.noNaNs() && FMF.noSignedZeros() && match(Op0, m_AnyZero()))
     return Op0;
 
   if (FMF.noNaNs()) {
     // X / X -> 1.0 is legal when NaNs are ignored.
+    // We can ignore infinities because INF/INF is NaN.
     if (Op0 == Op1)
       return ConstantFP::get(Op0->getType(), 1.0);
 
+    // (X * Y) / Y --> X if we can reassociate to the above form.
+    Value *X;
+    if (FMF.allowReassoc() && match(Op0, m_c_FMul(m_Value(X), m_Specific(Op1))))
+      return X;
+
     // -X /  X -> -1.0 and
     //  X / -X -> -1.0 are legal when NaNs are ignored.
     // We can ignore signed zeros because +-0.0/+-0.0 is NaN and ignored.
     if ((BinaryOperator::isFNeg(Op0, /*IgnoreZeroSign=*/true) &&
          BinaryOperator::getFNegArgument(Op0) == Op1) ||
         (BinaryOperator::isFNeg(Op1, /*IgnoreZeroSign=*/true) &&
          BinaryOperator::getFNegArgument(Op1) == Op0))
       return ConstantFP::get(Op0->getType(), -1.0);

test/Transforms/InstSimplify/fdiv.ll

 define double @fdiv_by_undef(double %X) {
 ; CHECK-LABEL: @fdiv_by_undef(
 ; CHECK-NEXT:    ret double undef
 ;
 ; X / undef -> undef
   %r = fdiv double %X, undef
   ret double %r
 }
+
+define double @fmul_fdiv_common_operand(double %x, double %y) {
+; CHECK-LABEL: @fmul_fdiv_common_operand(
+; CHECK-NEXT:    ret double %x
+;
+  %m = fmul double %x, %y
+  %d = fdiv reassoc nnan double %m, %y
+  ret double %d
+}
+
+; Negative test - the fdiv must be reassociative and not allow NaNs.
+
+define double @fmul_fdiv_common_operand_too_strict(double %x, double %y) {
+; CHECK-LABEL: @fmul_fdiv_common_operand_too_strict(
+; CHECK-NEXT:    [[M:%.*]] = fmul fast double %x, %y
+; CHECK-NEXT:    [[D:%.*]] = fdiv reassoc double [[M]], %y
+; CHECK-NEXT:    ret double [[D]]
+;
+  %m = fmul fast double %x, %y
+  %d = fdiv reassoc double %m, %y
+  ret double %d
+}
+
+; Commute the fmul operands. Use a vector type to verify that works too.
+
+define <2 x float> @fmul_fdiv_common_operand_commute_vec(<2 x float> %x, <2 x float> %y) {
+; CHECK-LABEL: @fmul_fdiv_common_operand_commute_vec(
+; CHECK-NEXT:    ret <2 x float> %x
+;
+  %m = fmul <2 x float> %y, %x
+  %d = fdiv fast <2 x float> %m, %y
+  ret <2 x float> %d
+}
+