Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 62 additions & 5 deletions clang/include/clang/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions clang/include/clang/AST/Stmt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -1233,6 +1243,7 @@ class alignas(void *) Stmt {
MemberExprBitfields MemberExprBits;
CastExprBitfields CastExprBits;
BinaryOperatorBitfields BinaryOperatorBits;
ConditionalOperatorBitfields ConditionalOperatorBits;
InitListExprBitfields InitListExprBits;
ParenListExprBitfields ParenListExprBits;
GenericSelectionExprBitfields GenericSelectionExprBits;
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/AST/TextNodeDumper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/AST/ASTImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions clang/lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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),
Expand Down
5 changes: 5 additions & 0 deletions clang/lib/AST/TextNodeDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/CodeGen/CGExprScalar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down
13 changes: 7 additions & 6 deletions clang/lib/Frontend/Rewrite/RewriteObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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>();
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 6 additions & 6 deletions clang/lib/Sema/SemaOpenMP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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);
Expand Down
8 changes: 7 additions & 1 deletion clang/lib/Serialization/ASTReaderStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Serialization/ASTWriterStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
21 changes: 21 additions & 0 deletions clang/test/AST/conditional-operator.c
Original file line number Diff line number Diff line change
@@ -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
36 changes: 36 additions & 0 deletions clang/test/CodeGen/conditional-operator.c
Original file line number Diff line number Diff line change
@@ -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
Comment on lines +2 to +4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundantly using -ffast-math and all the individual flags?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These flags correspond to how the clang driver -ffast-math are expanded to the individual flags passed to -cc1.

Several tests (e.g. clang/test/Parser/fp-floatcontrol-syntax.cpp and clang/test/CodeGen/math-errno.c) use similar command lines for testing fast-path behavior and I was trying to be consistent with them.


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;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test some vector and complex cases

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For "vector case" do you mean something along the lines of

typedef float float2 __attribute__((ext_vector_type(2)));
typedef int int2 __attribute__((ext_vector_type(2)));

float2 foo(int2 c, float2 t, float2 f) {
#pragma float_control(precise, off)
    return c ? t : f;
}

In this case, the expression would be codegen-d to a sequence of bitwise operations and these are not affected by fast-math flags

; Function Attrs: noinline nounwind optnone
define <2 x float> @foo(<2 x i32> noundef %c, <2 x float> noundef %t, <2 x float> noundef %f) #0 {
entry:
  %c.addr = alloca <2 x i32>, align 8
  %t.addr = alloca <2 x float>, align 8
  %f.addr = alloca <2 x float>, align 8
  store <2 x i32> %c, ptr %c.addr, align 8
  store <2 x float> %t, ptr %t.addr, align 8
  store <2 x float> %f, ptr %f.addr, align 8
  %0 = load <2 x i32>, ptr %c.addr, align 8
  %1 = load <2 x float>, ptr %t.addr, align 8
  %2 = load <2 x float>, ptr %f.addr, align 8
  %3 = icmp slt <2 x i32> %0, zeroinitializer
  %sext = sext <2 x i1> %3 to <2 x i32>
  %4 = xor <2 x i32> %sext, <i32 -1, i32 -1>
  %5 = bitcast <2 x float> %2 to <2 x i32>
  %6 = bitcast <2 x float> %1 to <2 x i32>
  %7 = and <2 x i32> %5, %4
  %8 = and <2 x i32> %6, %sext
  %cond = or <2 x i32> %7, %8
  %9 = bitcast <2 x i32> %cond to <2 x float>
  ret <2 x float> %9
}

Not really sure what I will be testing here?

// CHECK-LABEL: test_precise_on_phi
// CHECK: phi float [ {{%.+}}, {{%.+}} ], [ {{%.+}}, {{%.+}} ]
Loading