[clang] Allow ConditionalOperator fast-math flags to be overridden by pragma float_control

echesakov · echesakov · commit 1231a5658bf7 · 2024-10-05T12:22:27.000-07:00
Currently, the fast-math flags set on `select` or `phi` instruction emitted by CodeGen when visiting `ConditionalOperator` take into account only global `FPOptions` and ignore `pragma float_control`. This involves storing `FPOptionsOverride` in trailing storage of `ConditionalOperator` and storing `CurFPOptionsOverride` when creating an AST node. Fixes #84648
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
@@ -4210,26 +4210,45 @@ class AbstractConditionalOperator : public Expr {
 
 /// ConditionalOperator - The ?: ternary operator.  The GNU "missing
 /// middle" extension is a BinaryConditionalOperator.
-class ConditionalOperator : public AbstractConditionalOperator {
+class ConditionalOperator final
+    : public AbstractConditionalOperator,
+      private llvm::TrailingObjects<ConditionalOperator, FPOptionsOverride> {
   enum { COND, LHS, RHS, END_EXPR };
   Stmt* SubExprs[END_EXPR]; // Left/Middle/Right hand sides.
 
   friend class ASTStmtReader;
-public:
+
   ConditionalOperator(Expr *cond, SourceLocation QLoc, Expr *lhs,
                       SourceLocation CLoc, Expr *rhs, QualType t,
-                      ExprValueKind VK, ExprObjectKind OK)
+                      ExprValueKind VK, ExprObjectKind OK,
+                      FPOptionsOverride FPFeatures)
       : AbstractConditionalOperator(ConditionalOperatorClass, t, VK, OK, QLoc,
                                     CLoc) {
     SubExprs[COND] = cond;
     SubExprs[LHS] = lhs;
     SubExprs[RHS] = rhs;
+    ConditionalOperatorBits.HasFPFeatures =
+        FPFeatures.requiresTrailingStorage();
+    if (hasStoredFPFeatures())
+      setStoredFPFeatures(FPFeatures);
     setDependence(computeDependence(this));
   }
 
   /// Build an empty conditional operator.
-  explicit ConditionalOperator(EmptyShell Empty)
-    : AbstractConditionalOperator(ConditionalOperatorClass, Empty) { }
+  ConditionalOperator(EmptyShell Empty, bool HasFPFeatures)
+      : AbstractConditionalOperator(ConditionalOperatorClass, Empty) {
+    ConditionalOperatorBits.HasFPFeatures = HasFPFeatures;
+  }
+
+public:
+  static ConditionalOperator *CreateEmpty(const ASTContext &C, EmptyShell Empty,
+                                          bool HasFPFeatures);
+
+  static ConditionalOperator *Create(const ASTContext &C, Expr *cond,
+                                     SourceLocation QLoc, Expr *lhs,
+                                     SourceLocation CLoc, Expr *rhs, QualType t,
+                                     ExprValueKind VK, ExprObjectKind OK,
+                                     FPOptionsOverride FPFeatures);
 
   /// getCond - Return the expression representing the condition for
   ///   the ?: operator.
@@ -4265,6 +4284,44 @@ class ConditionalOperator : public AbstractConditionalOperator {
   const_child_range children() const {
     return const_child_range(&SubExprs[0], &SubExprs[0] + END_EXPR);
   }
+
+  /// Is FPFeatures in trailing storage?
+  bool hasStoredFPFeatures() const {
+    return ConditionalOperatorBits.HasFPFeatures;
+  }
+  /// Get FPFeatures from trailing storage.
+  FPOptionsOverride getStoredFPFeatures() const {
+    return *getTrailingFPFeatures();
+  }
+
+  // Get the FP features status of this operator. Only meaningful for
+  // operations on floating point types.
+  FPOptions getFPFeaturesInEffect(const LangOptions &LO) const {
+    if (hasStoredFPFeatures())
+      return getStoredFPFeatures().applyOverrides(LO);
+    return FPOptions::defaultWithoutTrailingStorage(LO);
+  }
+  FPOptionsOverride getFPFeatures() const {
+    return hasStoredFPFeatures() ? getStoredFPFeatures() : FPOptionsOverride();
+  }
+
+private:
+  /// Set FPFeatures in trailing storage, used only by Serialization.
+  void setStoredFPFeatures(FPOptionsOverride F) {
+    *getTrailingFPFeatures() = F;
+  }
+
+  FPOptionsOverride *getTrailingFPFeatures() {
+    assert(ConditionalOperatorBits.HasFPFeatures);
+    return getTrailingObjects<FPOptionsOverride>();
+  }
+
+  const FPOptionsOverride *getTrailingFPFeatures() const {
+    assert(ConditionalOperatorBits.HasFPFeatures);
+    return getTrailingObjects<FPOptionsOverride>();
+  }
+
+  friend class TrailingObjects;
 };
 
 /// BinaryConditionalOperator - The GNU extension to the conditional
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
@@ -658,6 +658,16 @@ class alignas(void *) Stmt {
     SourceLocation OpLoc;
   };
 
+  class ConditionalOperatorBitfields {
+    friend class ConditionalOperator;
+
+    unsigned : NumExprBits;
+
+    /// This is only meaningful when the second and third operands have floating
+    /// point types.
+    unsigned HasFPFeatures : 1;
+  };
+
   class InitListExprBitfields {
     friend class InitListExpr;
 
@@ -1233,6 +1243,7 @@ class alignas(void *) Stmt {
     MemberExprBitfields MemberExprBits;
     CastExprBitfields CastExprBits;
     BinaryOperatorBitfields BinaryOperatorBits;
+    ConditionalOperatorBitfields ConditionalOperatorBits;
     InitListExprBitfields InitListExprBits;
     ParenListExprBitfields ParenListExprBits;
     GenericSelectionExprBitfields GenericSelectionExprBits;
diff --git a/clang/include/clang/AST/TextNodeDumper.h b/clang/include/clang/AST/TextNodeDumper.h
@@ -286,6 +286,7 @@ class TextNodeDumper
   void VisitExtVectorElementExpr(const ExtVectorElementExpr *Node);
   void VisitBinaryOperator(const BinaryOperator *Node);
   void VisitCompoundAssignOperator(const CompoundAssignOperator *Node);
+  void VisitConditionalOperator(const ConditionalOperator *Node);
   void VisitAddrLabelExpr(const AddrLabelExpr *Node);
   void VisitCXXNamedCastExpr(const CXXNamedCastExpr *Node);
   void VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *Node);
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
@@ -7827,9 +7827,9 @@ ExpectedStmt ASTNodeImporter::VisitConditionalOperator(ConditionalOperator *E) {
   if (Err)
     return std::move(Err);
 
-  return new (Importer.getToContext()) ConditionalOperator(
-      ToCond, ToQuestionLoc, ToLHS, ToColonLoc, ToRHS, ToType,
-      E->getValueKind(), E->getObjectKind());
+  return ConditionalOperator::Create(
+      Importer.getToContext(), ToCond, ToQuestionLoc, ToLHS, ToColonLoc, ToRHS,
+      ToType, E->getValueKind(), E->getObjectKind(), E->getFPFeatures());
 }
 
 ExpectedStmt
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
@@ -3870,6 +3870,8 @@ FPOptions Expr::getFPFeaturesInEffect(const LangOptions &LO) const {
     return BO->getFPFeaturesInEffect(LO);
   if (auto Cast = dyn_cast<CastExpr>(this))
     return Cast->getFPFeaturesInEffect(LO);
+  if (auto CO = dyn_cast<ConditionalOperator>(this))
+    return CO->getFPFeaturesInEffect(LO);
   return FPOptions::defaultWithoutTrailingStorage(LO);
 }
 
@@ -4892,6 +4894,26 @@ CompoundAssignOperator::Create(const ASTContext &C, Expr *lhs, Expr *rhs,
                              CompLHSType, CompResultType);
 }
 
+ConditionalOperator *
+ConditionalOperator::Create(const ASTContext &C, Expr *cond,
+                            SourceLocation QLoc, Expr *lhs, SourceLocation CLoc,
+                            Expr *rhs, QualType t, ExprValueKind VK,
+                            ExprObjectKind OK, FPOptionsOverride FPFeatures) {
+  bool HasFPFeatures = FPFeatures.requiresTrailingStorage();
+  void *Mem = C.Allocate(totalSizeToAlloc<FPOptionsOverride>(HasFPFeatures),
+                         alignof(ConditionalOperator));
+  return new (Mem)
+      ConditionalOperator(cond, QLoc, lhs, CLoc, rhs, t, VK, OK, FPFeatures);
+}
+
+ConditionalOperator *ConditionalOperator::CreateEmpty(const ASTContext &C,
+                                                      EmptyShell Empty,
+                                                      bool HasFPFeatures) {
+  void *Mem = C.Allocate(totalSizeToAlloc<FPOptionsOverride>(HasFPFeatures),
+                         alignof(ConditionalOperator));
+  return new (Mem) ConditionalOperator(EmptyShell(), HasFPFeatures);
+}
+
 UnaryOperator *UnaryOperator::CreateEmpty(const ASTContext &C,
                                           bool hasFPFeatures) {
   void *Mem = C.Allocate(totalSizeToAlloc<FPOptionsOverride>(hasFPFeatures),
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
@@ -1518,6 +1518,11 @@ void TextNodeDumper::VisitCompoundAssignOperator(
     printFPOptions(Node->getStoredFPFeatures());
 }
 
+void TextNodeDumper::VisitConditionalOperator(const ConditionalOperator *Node) {
+  if (Node->hasStoredFPFeatures())
+    printFPOptions(Node->getStoredFPFeatures());
+}
+
 void TextNodeDumper::VisitAddrLabelExpr(const AddrLabelExpr *Node) {
   OS << " " << Node->getLabel()->getName();
   dumpPointer(Node->getLabel());
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -5217,6 +5217,8 @@ VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
   Expr *lhsExpr = E->getTrueExpr();
   Expr *rhsExpr = E->getFalseExpr();
 
+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
+
   // If the condition constant folds and can be elided, try to avoid emitting
   // the condition and the dead arm.
   bool CondExprBool;
diff --git a/clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp b/clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
@@ -4568,9 +4568,10 @@ Stmt *RewriteModernObjC::SynthesizeBlockCall(CallExpr *Exp, const Expr *BlockExp
     Expr *RHSExp = CEXPR->getRHS();
     Stmt *RHSStmt = SynthesizeBlockCall(Exp, RHSExp);
     Expr *CONDExp = CEXPR->getCond();
-    ConditionalOperator *CondExpr = new (Context) ConditionalOperator(
-        CONDExp, SourceLocation(), cast<Expr>(LHSStmt), SourceLocation(),
-        cast<Expr>(RHSStmt), Exp->getType(), VK_PRValue, OK_Ordinary);
+    ConditionalOperator *CondExpr = ConditionalOperator::Create(
+        *Context, CONDExp, SourceLocation(), cast<Expr>(LHSStmt),
+        SourceLocation(), cast<Expr>(RHSStmt), Exp->getType(), VK_PRValue,
+        OK_Ordinary, FPOptionsOverride());
     return CondExpr;
   } else if (const ObjCIvarRefExpr *IRE = dyn_cast<ObjCIvarRefExpr>(BlockExp)) {
     CPT = IRE->getType()->getAs<BlockPointerType>();
diff --git a/clang/lib/Frontend/Rewrite/RewriteObjC.cpp b/clang/lib/Frontend/Rewrite/RewriteObjC.cpp
@@ -2997,9 +2997,9 @@ Stmt *RewriteObjC::SynthMessageExpr(ObjCMessageExpr *Exp,
         *Context, sizeofExpr, limit, BO_LE, Context->IntTy, VK_PRValue,
         OK_Ordinary, SourceLocation(), FPOptionsOverride());
     // (sizeof(returnType) <= 8 ? objc_msgSend(...) : objc_msgSend_stret(...))
-    ConditionalOperator *CondExpr = new (Context) ConditionalOperator(
-        lessThanExpr, SourceLocation(), CE, SourceLocation(), STCE, returnType,
-        VK_PRValue, OK_Ordinary);
+    ConditionalOperator *CondExpr = ConditionalOperator::Create(
+        *Context, lessThanExpr, SourceLocation(), CE, SourceLocation(), STCE,
+        returnType, VK_PRValue, OK_Ordinary, FPOptionsOverride());
     ReplacingStmt = new (Context) ParenExpr(SourceLocation(), SourceLocation(),
                                             CondExpr);
   }
@@ -3738,9 +3738,10 @@ Stmt *RewriteObjC::SynthesizeBlockCall(CallExpr *Exp, const Expr *BlockExp) {
     Expr *RHSExp = CEXPR->getRHS();
     Stmt *RHSStmt = SynthesizeBlockCall(Exp, RHSExp);
     Expr *CONDExp = CEXPR->getCond();
-    ConditionalOperator *CondExpr = new (Context) ConditionalOperator(
-        CONDExp, SourceLocation(), cast<Expr>(LHSStmt), SourceLocation(),
-        cast<Expr>(RHSStmt), Exp->getType(), VK_PRValue, OK_Ordinary);
+    ConditionalOperator *CondExpr = ConditionalOperator::Create(
+        *Context, CONDExp, SourceLocation(), cast<Expr>(LHSStmt),
+        SourceLocation(), cast<Expr>(RHSStmt), Exp->getType(), VK_PRValue,
+        OK_Ordinary, FPOptionsOverride());
     return CondExpr;
   } else if (const ObjCIvarRefExpr *IRE = dyn_cast<ObjCIvarRefExpr>(BlockExp)) {
     CPT = IRE->getType()->getAs<BlockPointerType>();
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
@@ -8795,9 +8795,9 @@ ExprResult Sema::ActOnConditionalOp(SourceLocation QuestionLoc,
                                          Context);
 
   if (!commonExpr)
-    return new (Context)
-        ConditionalOperator(Cond.get(), QuestionLoc, LHS.get(), ColonLoc,
-                            RHS.get(), result, VK, OK);
+    return ConditionalOperator::Create(Context, Cond.get(), QuestionLoc,
+                                       LHS.get(), ColonLoc, RHS.get(), result,
+                                       VK, OK, CurFPFeatureOverrides());
 
   return new (Context) BinaryConditionalOperator(
       commonExpr, opaqueValue, Cond.get(), LHS.get(), RHS.get(), QuestionLoc,
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
@@ -14330,10 +14330,10 @@ StmtResult SemaOpenMP::ActOnOpenMPTileDirective(ArrayRef<OMPClause *> Clauses,
     Expr *Cond = AssertSuccess(SemaRef.BuildBinOp(
         CurScope, {}, BO_LE,
         AssertSuccess(CopyTransformer.TransformExpr(DimTileSizeExpr)), Zero));
-    Expr *MinOne = new (Context) ConditionalOperator(
-        Cond, {}, One, {},
+    Expr *MinOne = ConditionalOperator::Create(
+        Context, Cond, {}, One, {},
         AssertSuccess(CopyTransformer.TransformExpr(DimTileSizeExpr)), DimTy,
-        VK_PRValue, OK_Ordinary);
+        VK_PRValue, OK_Ordinary, FPOptionsOverride());
     return MinOne;
   };
 
@@ -18858,9 +18858,9 @@ static bool actOnOMPReductionKindClause(
                 S.BuildBinOp(Stack->getCurScope(), ReductionId.getBeginLoc(),
                              BO_Assign, LHSDRE, ReductionOp.get());
           } else {
-            auto *ConditionalOp = new (Context)
-                ConditionalOperator(ReductionOp.get(), ELoc, LHSDRE, ELoc,
-                                    RHSDRE, Type, VK_LValue, OK_Ordinary);
+            auto *ConditionalOp = ConditionalOperator::Create(
+                Context, ReductionOp.get(), ELoc, LHSDRE, ELoc, RHSDRE, Type,
+                VK_LValue, OK_Ordinary, FPOptionsOverride());
             ReductionOp =
                 S.BuildBinOp(Stack->getCurScope(), ReductionId.getBeginLoc(),
                              BO_Assign, LHSDRE, ConditionalOp);
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1145,11 +1145,16 @@ void ASTStmtReader::VisitCompoundAssignOperator(CompoundAssignOperator *E) {
 
 void ASTStmtReader::VisitConditionalOperator(ConditionalOperator *E) {
   VisitExpr(E);
+  bool HasFPFeatures = Record.readInt();
+  assert(HasFPFeatures == E->hasStoredFPFeatures());
   E->SubExprs[ConditionalOperator::COND] = Record.readSubExpr();
   E->SubExprs[ConditionalOperator::LHS] = Record.readSubExpr();
   E->SubExprs[ConditionalOperator::RHS] = Record.readSubExpr();
   E->QuestionLoc = readSourceLocation();
   E->ColonLoc = readSourceLocation();
+  if (HasFPFeatures)
+    E->setStoredFPFeatures(
+        FPOptionsOverride::getFromOpaqueInt(Record.readInt()));
 }
 
 void
@@ -3182,7 +3187,8 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) {
     }
 
     case EXPR_CONDITIONAL_OPERATOR:
-      S = new (Context) ConditionalOperator(Empty);
+      S = ConditionalOperator::CreateEmpty(
+          Context, Empty, Record[ASTStmtReader::NumExprFields]);
       break;
 
     case EXPR_BINARY_CONDITIONAL_OPERATOR:
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1091,11 +1091,14 @@ void ASTStmtWriter::VisitCompoundAssignOperator(CompoundAssignOperator *E) {
 
 void ASTStmtWriter::VisitConditionalOperator(ConditionalOperator *E) {
   VisitExpr(E);
+  Record.push_back(E->hasStoredFPFeatures());
   Record.AddStmt(E->getCond());
   Record.AddStmt(E->getLHS());
   Record.AddStmt(E->getRHS());
   Record.AddSourceLocation(E->getQuestionLoc());
   Record.AddSourceLocation(E->getColonLoc());
+  if (E->hasStoredFPFeatures())
+    Record.push_back(E->getStoredFPFeatures().getAsOpaqueInt());
   Code = serialization::EXPR_CONDITIONAL_OPERATOR;
 }
 
diff --git a/clang/test/AST/conditional-operator.c b/clang/test/AST/conditional-operator.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -ast-dump %s | FileCheck %s
+// RUN: %clang_cc1 -ast-dump -menable-no-infs -fapprox-func -funsafe-math-optimizations \
+// RUN: -fno-signed-zeros -mreassociate -freciprocal-math -ffp-contract=fast -ffast-math %s | FileCheck %s
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -x c -include-pch %t -ast-dump-all /dev/null | FileCheck %s
+
+float test_precise_off(int c, float t, float f) {
+#pragma float_control(precise, off)
+  return c ? t : f;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_precise_off
+// CHECK: ConditionalOperator {{.*}} FPContractMode=2 AllowFPReassociate=1 NoHonorNaNs=1 NoHonorInfs=1 NoSignedZero=1 AllowReciprocal=1 AllowApproxFunc=1 MathErrno=0
+
+float test_precise_on(int c, float t, float f) {
+#pragma float_control(precise, on)
+  return c ? t : f;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_precise_on
+// CHECK: ConditionalOperator {{.*}} FPContractMode=1 AllowFPReassociate=0 NoHonorNaNs=0 NoHonorInfs=0 NoSignedZero=0 AllowReciprocal=0 AllowApproxFunc=0 MathErrno=1
diff --git a/clang/test/CodeGen/conditional-operator.c b/clang/test/CodeGen/conditional-operator.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -disable-llvm-passes -emit-llvm -menable-no-infs -fapprox-func\
+// RUN: -funsafe-math-optimizations -fno-signed-zeros -mreassociate -freciprocal-math\
+// RUN: -ffp-contract=fast -ffast-math %s -o - | FileCheck %s
+
+float test_precise_off_select(int c) {
+#pragma float_control(precise, off)
+  return c ? 0.0f : 1.0f;
+}
+
+// CHECK-LABEL: test_precise_off_select
+// CHECK: select fast i1 {{%.+}}, float 0.000000e+00, float 1.000000e+00
+
+float test_precise_off_phi(int c, float t, float f) {
+#pragma float_control(precise, off)
+    return c ? t : f;
+}
+
+// CHECK-LABEL: test_precise_off_phi
+// CHECK: phi fast float [ {{%.+}}, {{%.+}} ], [ {{%.+}}, {{%.+}} ]
+
+float test_precise_on_select(int c) {
+#pragma float_control(precise, on)
+    return c ? 0.0f : 1.0f;
+}
+
+// CHECK-LABEL: test_precise_on_select
+// CHECK: select i1 {{%.+}}, float 0.000000e+00, float 1.000000e+00
+
+float test_precise_on_phi(int c, float t, float f) {
+#pragma float_control(precise, on)
+  return c ? t : f;
+}
+
+// CHECK-LABEL: test_precise_on_phi
+// CHECK: phi float [ {{%.+}}, {{%.+}} ], [ {{%.+}}, {{%.+}} ]
