Set FMF for -ffp-contract=fast

[anemet]

Summary

With this, FMF(contract) becomes an alternative way to express the request to
contract.

This is handled similarly to ApplyDebugLocation using an RAII to propagate
FPOptions from the BinaryOperator to the IRBuilder. Note that
CompoundAssignOperator (+=) can be an lvalue so it needs to be handled as such
as well.

This is toward fixing PR25721.

[ahatanak]

I think you can add a test case in this patch (after the llvm changes are checked in)?

lib/CodeGen/CGExprScalar.cpp

262 ↗

(On Diff #92424)

Is it necessary to check CXXOperatorCallExpr too here?

Since CXXOperatorCallExpr has an FPFeatures field too, I was wondering how CodeGen was going to use that information.

[anemet]

Thanks, Akira. You're right on both accounts. I actually had a test written but failed to git add it.

[anemet]

Address Akira's comments

[anemet]

Also add 'contract' for CompoundAssignOperator (+= and -=). For l-values,
these don't go through the expr-visitor in ScalarExprEmitter::Visit. This
again piggybacks on how debug-locations are added.

[anemet]

Ping. This is the only patch in the series now that wasn't approved.

[rjmccall]

I may have missed earlier steps in this patch series. Why is this being done statefully and contextually in the IRBuilder instead of just applying the flag from the BinaryOperator to the instruction when building it? It's not like ScalarExprEmitter doesn't know that it's building an FMul.

[anemet]

In D31168#716153, @rjmccall wrote:

I may have missed earlier steps in this patch series. Why is this being done statefully and contextually in the IRBuilder instead of just applying the flag from the BinaryOperator to the instruction when building it? It's not like ScalarExprEmitter doesn't know that it's building an FMul.

The main reason is that the other FMFlags are currently maintained in the IRBuilder (see CodeGenFunction.cpp:87). That said as we move those over to the operators as well, it makes more sense to move away from using the IRBuilder for this. See updated patch and thanks for the suggestion!

Also let me know if you have post-commit comments on the patches in the series. You can find them either on the cfe-dev thread or in the dependencies of D31276

[anemet]

Address John's comment.

[aaron.ballman]

LGTM. but you should wait for someone more familiar with this part of codegen before committing.

[anemet]

Thanks, Aaron! @rjmccall, does it look good to you too?

[rjmccall]

Yes, thank you, that's great.

cfe/trunk/lib/CodeGen/CGExprScalar.cpp

static bool CanElideOverflowCheck(const ASTContext &Ctx, const BinOpInfo &Op) {
 
   // The overflow check can be skipped if either one of the unpromoted types
   // are less than half the size of the promoted type.
   unsigned PromotedSize = Ctx.getTypeSize(Op.E->getType());
   return (2 * Ctx.getTypeSize(LHSTy)) < PromotedSize ||
          (2 * Ctx.getTypeSize(RHSTy)) < PromotedSize;
 }
 
+/// Update the FastMathFlags of LLVM IR from the FPOptions in LangOptions.
+static void updateFastMathFlags(llvm::FastMathFlags &FMF,
+                                FPOptions FPFeatures) {
+  FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement());
+}
+
+/// Propagate fast-math flags from \p Op to the instruction in \p V.
+static Value *propagateFMFlags(Value *V, const BinOpInfo &Op) {
+  if (auto *I = dyn_cast<llvm::Instruction>(V)) {
+    llvm::FastMathFlags FMF = I->getFastMathFlags();
+    updateFastMathFlags(FMF, Op.FPFeatures);
+    I->setFastMathFlags(FMF);
+  }
+  return V;
+}
+
 class ScalarExprEmitter
   : public StmtVisitor<ScalarExprEmitter, Value*> {
   CodeGenFunction &CGF;
   CGBuilderTy &Builder;
   bool IgnoreResultAssign;
   llvm::LLVMContext &VMContext;
 public:
 
if (Ops.Ty->isSignedIntegerOrEnumerationType()) {
       }
     }
 
     if (Ops.Ty->isUnsignedIntegerType() &&
         CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
         !CanElideOverflowCheck(CGF.getContext(), Ops))
       return EmitOverflowCheckedBinOp(Ops);
 
-    if (Ops.LHS->getType()->isFPOrFPVectorTy())
-      return Builder.CreateFMul(Ops.LHS, Ops.RHS, "mul");
+    if (Ops.LHS->getType()->isFPOrFPVectorTy()) {
+      Value *V = Builder.CreateFMul(Ops.LHS, Ops.RHS, "mul");
+      return propagateFMFlags(V, Ops);
+    }
     return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul");
   }
   /// Create a binary op that checks for overflow.
   /// Currently only supports +, - and *.
   Value *EmitOverflowCheckedBinOp(const BinOpInfo &Ops);
 
   // Check for undefined division and modulus behaviors.
   void EmitUndefinedBehaviorIntegerDivAndRemCheck(const BinOpInfo &Ops,
if (op.Ty->isUnsignedIntegerType() &&
       !CanElideOverflowCheck(CGF.getContext(), op))
     return EmitOverflowCheckedBinOp(op);
 
   if (op.LHS->getType()->isFPOrFPVectorTy()) {
     // Try to form an fmuladd.
     if (Value *FMulAdd = tryEmitFMulAdd(op, CGF, Builder))
       return FMulAdd;
 
-    return Builder.CreateFAdd(op.LHS, op.RHS, "add");
+    Value *V = Builder.CreateFAdd(op.LHS, op.RHS, "add");
+    return propagateFMFlags(V, op);
   }
 
   return Builder.CreateAdd(op.LHS, op.RHS, "add");
 }
 
 Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) {
   // The LHS is always a pointer if either side is.
   if (!op.LHS->getType()->isPointerTy()) {
if (op.Ty->isUnsignedIntegerType() &&
         CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) &&
         !CanElideOverflowCheck(CGF.getContext(), op))
       return EmitOverflowCheckedBinOp(op);
 
     if (op.LHS->getType()->isFPOrFPVectorTy()) {
       // Try to form an fmuladd.
       if (Value *FMulAdd = tryEmitFMulAdd(op, CGF, Builder, true))
         return FMulAdd;
-      return Builder.CreateFSub(op.LHS, op.RHS, "sub");
+      Value *V = Builder.CreateFSub(op.LHS, op.RHS, "sub");
+      return propagateFMFlags(V, op);
     }
 
     return Builder.CreateSub(op.LHS, op.RHS, "sub");
   }
 
   // If the RHS is not a pointer, then we have normal pointer
   // arithmetic.
   if (!op.RHS->getType()->isPointerTy())

cfe/trunk/test/CodeGen/ffp-contract-fast-option.cpp

+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+float fp_contract_1(float a, float b, float c) {
+  // CHECK-LABEL: fp_contract_1fff(
+  // CHECK: fmul contract float
+  // CHECK: fadd contract float
+  return a * b + c;
+}
+
+float fp_contract_2(float a, float b, float c) {
+  // CHECK-LABEL: fp_contract_2fff(
+  // CHECK: fmul contract float
+  // CHECK: fsub contract float
+  return a * b - c;
+}
+
+void fp_contract_3(float *a, float b, float c) {
+  // CHECK-LABEL: fp_contract_3Pfff(
+  // CHECK: fmul contract float
+  // CHECK: fadd contract float
+  a[0] += b * c;
+}
+
+void fp_contract_4(float *a, float b, float c) {
+  // CHECK-LABEL: fp_contract_4Pfff(
+  // CHECK: fmul contract float
+  // CHECK: fsub contract float
+  a[0] -= b * c;
+}