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

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.

lib/CodeGen/CGExprScalar.cpp

Value *EmitIntToBoolConversion(Value *V) {
 
     return Builder.CreateIsNotNull(V, "tobool");
   }
 
   //===--------------------------------------------------------------------===//
   //                            Visitor Methods
   //===--------------------------------------------------------------------===//
 
+  /// A RAII to apply the FP features from the expression to the IRBuilder.
+  struct ApplyFPFeatures : public CGBuilderTy::FastMathFlagGuard {
+    ApplyFPFeatures(llvm::IRBuilderBase &Builder, Expr *E)
+        : CGBuilderTy::FastMathFlagGuard(Builder) {
+      if (const auto *BO = dyn_cast<BinaryOperator>(E)) {
+        llvm::FastMathFlags FMF = Builder.getFastMathFlags();
+        FMF.setAllowContract(
+            BO->getFPFeatures().allowFPContractAcrossStatement());
+        Builder.setFastMathFlags(FMF);
+      }
+    }
+  };
+
   Value *Visit(Expr *E) {
     ApplyDebugLocation DL(CGF, E);
+    ApplyFPFeatures FPF(Builder, E);
     return StmtVisitor<ScalarExprEmitter, Value*>::Visit(E);
   }
 
   Value *VisitStmt(Stmt *S) {
     S->dump(CGF.getContext().getSourceManager());
     llvm_unreachable("Stmt can't have complex result type!");
   }
   Value *VisitExpr(Expr *S);