[Clang] Teach buildFMulAdd to peek through fneg to find fmul.

[craig.topper]

Summary

Allows us to handle expressions like -(a * b) + c

Based on the examples from D144366 that gcc seems to get.

[kpn]

clang/lib/CodeGen/CGExprScalar.cpp

3738

If I'm reading this right it looks like the assert() wasn't needed before. Do we know why it was added in the first place?

[craig.topper]

clang/lib/CodeGen/CGExprScalar.cpp

3738

I don't. I've added @lhames who wrote the code originally, but's been 10 years.

[lhames]

clang/lib/CodeGen/CGExprScalar.cpp

3738

It was probably included as a guard rail during bringup (from memory we didn't have any patterns in LLVM that matched both negations) and was never removed. I think it should be fine to remove.

[kpn]

LGTM.

clang/lib/CodeGen/CGExprScalar.cpp

 //===--- CGExprScalar.cpp - Emit LLVM Code for Scalar Exprs ---------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //

 // Construct an fmuladd intrinsic to represent a fused mul-add of MulOp and
 // Addend. Use negMul and negAdd to negate the first operand of the Mul or
 // the add operand respectively. This allows fmuladd to represent a*b-c, or
 // c-a*b. Patterns in LLVM should catch the negated forms and translate them to
 // efficient operations.
 static Value* buildFMulAdd(llvm::Instruction *MulOp, Value *Addend,
                            const CodeGenFunction &CGF, CGBuilderTy &Builder,
                            bool negMul, bool negAdd) {
-  assert(!(negMul && negAdd) && "Only one of negMul and negAdd should be set.");
-
   Value *MulOp0 = MulOp->getOperand(0);
   Value *MulOp1 = MulOp->getOperand(1);
   if (negMul)
     MulOp0 = Builder.CreateFNeg(MulOp0, "neg");
   if (negAdd)
     Addend = Builder.CreateFNeg(Addend, "neg");
 
   Value *FMulAdd = nullptr;
static Value* tryEmitFMulAdd(const BinOpInfo &op,
   assert((op.Opcode == BO_Add || op.Opcode == BO_AddAssign ||
           op.Opcode == BO_Sub || op.Opcode == BO_SubAssign) &&
          "Only fadd/fsub can be the root of an fmuladd.");
 
   // Check whether this op is marked as fusable.
   if (!op.FPFeatures.allowFPContractWithinStatement())
     return nullptr;
 
+  Value *LHS = op.LHS;
+  Value *RHS = op.RHS;
+
+  // Peek through fneg to look for fmul. Make sure fneg has no users, and that
+  // it is the only use of its operand.
+  bool NegLHS = false;
+  if (auto *LHSUnOp = dyn_cast<llvm::UnaryOperator>(LHS)) {
+    if (LHSUnOp->getOpcode() == llvm::Instruction::FNeg &&
+        LHSUnOp->use_empty() && LHSUnOp->getOperand(0)->hasOneUse()) {
+      LHS = LHSUnOp->getOperand(0);
+      NegLHS = true;
+    }
+  }
+
+  bool NegRHS = false;
+  if (auto *RHSUnOp = dyn_cast<llvm::UnaryOperator>(RHS)) {
+    if (RHSUnOp->getOpcode() == llvm::Instruction::FNeg &&
+        RHSUnOp->use_empty() && RHSUnOp->getOperand(0)->hasOneUse()) {
+      RHS = RHSUnOp->getOperand(0);
+      NegRHS = true;
+    }
+  }
+
   // We have a potentially fusable op. Look for a mul on one of the operands.
   // Also, make sure that the mul result isn't used directly. In that case,
   // there's no point creating a muladd operation.
-  if (auto *LHSBinOp = dyn_cast<llvm::BinaryOperator>(op.LHS)) {
+  if (auto *LHSBinOp = dyn_cast<llvm::BinaryOperator>(LHS)) {
     if (LHSBinOp->getOpcode() == llvm::Instruction::FMul &&
-        LHSBinOp->use_empty())
-      return buildFMulAdd(LHSBinOp, op.RHS, CGF, Builder, false, isSub);
+        (LHSBinOp->use_empty() || NegLHS)) {
+      // If we looked through fneg, erase it.
+      if (NegLHS)
+        cast<llvm::Instruction>(op.LHS)->eraseFromParent();
+      return buildFMulAdd(LHSBinOp, op.RHS, CGF, Builder, NegLHS, isSub);
+    }
   }
-  if (auto *RHSBinOp = dyn_cast<llvm::BinaryOperator>(op.RHS)) {
+  if (auto *RHSBinOp = dyn_cast<llvm::BinaryOperator>(RHS)) {
     if (RHSBinOp->getOpcode() == llvm::Instruction::FMul &&
-        RHSBinOp->use_empty())
-      return buildFMulAdd(RHSBinOp, op.LHS, CGF, Builder, isSub, false);
+        (RHSBinOp->use_empty() || NegRHS)) {
+      // If we looked through fneg, erase it.
+      if (NegRHS)
+        cast<llvm::Instruction>(op.RHS)->eraseFromParent();
+      return buildFMulAdd(RHSBinOp, op.LHS, CGF, Builder, isSub ^ NegRHS, false);
+    }
   }
 
-  if (auto *LHSBinOp = dyn_cast<llvm::CallBase>(op.LHS)) {
+  if (auto *LHSBinOp = dyn_cast<llvm::CallBase>(LHS)) {
     if (LHSBinOp->getIntrinsicID() ==
             llvm::Intrinsic::experimental_constrained_fmul &&
-        LHSBinOp->use_empty())
-      return buildFMulAdd(LHSBinOp, op.RHS, CGF, Builder, false, isSub);
+        (LHSBinOp->use_empty() || NegLHS)) {
+      // If we looked through fneg, erase it.
+      if (NegLHS)
+        cast<llvm::Instruction>(op.LHS)->eraseFromParent();
+      return buildFMulAdd(LHSBinOp, op.RHS, CGF, Builder, NegLHS, isSub);
+    }
   }
-  if (auto *RHSBinOp = dyn_cast<llvm::CallBase>(op.RHS)) {
+  if (auto *RHSBinOp = dyn_cast<llvm::CallBase>(RHS)) {
     if (RHSBinOp->getIntrinsicID() ==
             llvm::Intrinsic::experimental_constrained_fmul &&
-        RHSBinOp->use_empty())
-      return buildFMulAdd(RHSBinOp, op.LHS, CGF, Builder, isSub, false);
+        (RHSBinOp->use_empty() || NegRHS)) {
+      // If we looked through fneg, erase it.
+      if (NegRHS)
+        cast<llvm::Instruction>(op.RHS)->eraseFromParent();
+      return buildFMulAdd(RHSBinOp, op.LHS, CGF, Builder, isSub ^ NegRHS, false);
+    }
   }
 
   return nullptr;
 }
 
 Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) {
   if (op.LHS->getType()->isPointerTy() ||
       op.RHS->getType()->isPointerTy())

clang/test/CodeGen/constrained-math-builtins.c

 // CHECK: declare x86_fp80 @llvm.experimental.constrained.trunc.f80(x86_fp80, metadata)
 // CHECK: declare fp128 @llvm.experimental.constrained.trunc.f128(fp128, metadata)
 
 #pragma STDC FP_CONTRACT ON
 void bar(float f) {
   f * f + f;
   (double)f * f - f;
   (long double)-f * f + f;
+  -(f * f) - f;
+  f + -(f * f);
 
   // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: fneg
   // CHECK: call double @llvm.experimental.constrained.fmuladd.f64(double %{{.*}}, double %{{.*}}, double %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: fneg
   // CHECK: call x86_fp80 @llvm.experimental.constrained.fmuladd.f80(x86_fp80 %{{.*}}, x86_fp80 %{{.*}}, x86_fp80 %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // CHECK: fneg
+  // CHECK: fneg
+  // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // CHECK: fneg
+  // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 };

clang/test/CodeGen/fp-contract-pragma.cpp

 
 float fp_contract_9(float a, float b, float c) {
 // CHECK: _Z13fp_contract_9fff
 // CHECK: fneg float %a
 // CHECK: tail call float @llvm.fmuladd
   #pragma STDC FP_CONTRACT ON
   return c - a * b;
 }
+
+float fp_contract_10(float a, float b, float c) {
+// CHECK: _Z14fp_contract_10fff
+// CHECK: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return -(a * b) + c;
+}
+
+float fp_contract_11(float a, float b, float c) {
+// CHECK: _Z14fp_contract_11fff
+// CHECK: fneg float %a
+// CHECK: fneg float %c
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return -(a * b) - c;
+}
+
+float fp_contract_12(float a, float b, float c) {
+// CHECK: _Z14fp_contract_12fff
+// CHECK: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return c + -(a * b);
+}
+
+float fp_contract_13(float a, float b, float c) {
+// CHECK: _Z14fp_contract_13fff
+// CHECK-NOT: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return c - -(a * b);
+}
+
+float fp_contract_14(float a, float b, float c) {
+// CHECK: _Z14fp_contract_14fff
+// CHECK: %[[M:.+]] = fmul float %a, %b
+// CHECK-NEXT: %add = fsub float %c, %[[M]]
+  #pragma STDC FP_CONTRACT ON
+  float d;
+  return (d = -(a * b)) + c;
+}
+
+float fp_contract_15(float a, float b, float c) {
+// CHECK: _Z14fp_contract_15fff
+// CHECK: %[[M:.+]] = fmul float %a, %b
+// CHECK-NEXT: %add = fsub float %c, %[[M]]
+  #pragma STDC FP_CONTRACT ON
+  float d;
+  return -(d = (a * b)) + c;
+}