[PATCH] D31166: Encapsulate FPOptions and use it consistently
anemet created this revision. Sema holds the current FPOptions which is adjusted by 'pragma STDC FP_CONTRACT'. This then gets propagated into expression nodes as they are built. This encapsulates FPOptions so that this propagation happens opaquely rather than directly with the fp_contractable on/off bit. This allows controlled transitioning of fp_contractable to a ternary value (off, on, fast). It will also allow adding more fast-math flags later. This is toward moving fp-contraction=fast from an LLVM TargetOption to a FastMathFlag in order to fix PR25721. https://reviews.llvm.org/D31166 Files: include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/Basic/LangOptions.h include/clang/Sema/Sema.h lib/AST/ASTImporter.cpp lib/Analysis/BodyFarm.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CGObjC.cpp lib/CodeGen/CGStmtOpenMP.cpp lib/Frontend/Rewrite/RewriteModernObjC.cpp lib/Frontend/Rewrite/RewriteObjC.cpp lib/Sema/SemaAttr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaOverload.cpp lib/Sema/SemaPseudoObject.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReader.cpp lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterStmt.cpp Index: lib/Serialization/ASTWriterStmt.cpp === --- lib/Serialization/ASTWriterStmt.cpp +++ lib/Serialization/ASTWriterStmt.cpp @@ -650,7 +650,7 @@ Record.AddStmt(E->getRHS()); Record.push_back(E->getOpcode()); // FIXME: stable encoding Record.AddSourceLocation(E->getOperatorLoc()); - Record.push_back(E->isFPContractable()); + Record.push_back(E->getFPFeatures().getInt()); Code = serialization::EXPR_BINARY_OPERATOR; } @@ -1218,7 +1218,7 @@ VisitCallExpr(E); Record.push_back(E->getOperator()); Record.AddSourceRange(E->Range); - Record.push_back(E->isFPContractable()); + Record.push_back(E->getFPFeatures().getInt()); Code = serialization::EXPR_CXX_OPERATOR_CALL; } Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -3975,7 +3975,7 @@ /// \brief Write an FP_PRAGMA_OPTIONS block for the given FPOptions. void ASTWriter::WriteFPPragmaOptions(const FPOptions &Opts) { - RecordData::value_type Record[] = {Opts.fp_contract}; + RecordData::value_type Record[] = {Opts.getInt()}; Stream.EmitRecord(FP_PRAGMA_OPTIONS, Record); } Index: lib/Serialization/ASTReaderStmt.cpp === --- lib/Serialization/ASTReaderStmt.cpp +++ lib/Serialization/ASTReaderStmt.cpp @@ -670,7 +670,7 @@ E->setRHS(Record.readSubExpr()); E->setOpcode((BinaryOperator::Opcode)Record.readInt()); E->setOperatorLoc(ReadSourceLocation()); - E->setFPContractable((bool)Record.readInt()); + E->setFPFeatures(FPOptions(Record.readInt())); } void ASTStmtReader::VisitCompoundAssignOperator(CompoundAssignOperator *E) { @@ -1225,7 +1225,7 @@ VisitCallExpr(E); E->Operator = (OverloadedOperatorKind)Record.readInt(); E->Range = Record.readSourceRange(); - E->setFPContractable((bool)Record.readInt()); + E->setFPFeatures(FPOptions(Record.readInt())); } void ASTStmtReader::VisitCXXConstructExpr(CXXConstructExpr *E) { Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -7215,7 +7215,7 @@ // FIXME: What happens if these are changed by a module import? if (!FPPragmaOptions.empty()) { assert(FPPragmaOptions.size() == 1 && "Wrong number of FP_PRAGMA_OPTIONS"); -SemaObj->FPFeatures.fp_contract = FPPragmaOptions[0]; +SemaObj->FPFeatures = FPOptions(FPPragmaOptions[0]); } SemaObj->OpenCLFeatures.copy(OpenCLExtensions); Index: lib/Sema/TreeTransform.h === --- lib/Sema/TreeTransform.h +++ lib/Sema/TreeTransform.h @@ -9146,7 +9146,7 @@ return E; Sema::FPContractStateRAII FPContractState(getSema()); - getSema().FPFeatures.fp_contract = E->isFPContractable(); + getSema().FPFeatures = E->getFPFeatures(); return getDerived().RebuildBinaryOperator(E->getOperatorLoc(), E->getOpcode(), LHS.get(), RHS.get()); @@ -9626,7 +9626,7 @@ return SemaRef.MaybeBindToTemporary(E); Sema::FPContractStateRAII FPContractState(getSema()); - getSema().FPFeatures.fp_contract = E->isFPContractable(); + getSema().FPFeatures = E->getFPFeatures(); return getDerived().RebuildCXXOperatorCallExpr(E->getOperator(), E->getOperatorLoc(), Index: lib/Sema/SemaPseudoObject.cpp === --- lib/Sema/SemaPseudoObject.cpp +++ lib/Se
[PATCH] D31166: Encapsulate FPOptions and use it consistently
aaron.ballman added inline comments. Comment at: include/clang/AST/ExprCXX.h:58 + // Only meaningful for floating point types. For other types this value can be // set to false. + FPOptions FPFeatures; Setting a class to false sounds a bit strange. May want to reword. Comment at: include/clang/Basic/LangOptions.h:180 - FPOptions(const LangOptions &LangOpts) : -fp_contract(LangOpts.DefaultFPContract) {} + explicit FPOptions(uint64_t I) : fp_contract(I) {} + Why a `uint64_t` rather than `unsigned`? Same for the accessor. Comment at: include/clang/Basic/LangOptions.h:182 + + FPOptions(const LangOptions &LangOpts) + : fp_contract(LangOpts.DefaultFPContract) {} Might as well make this one explicit as well. Comment at: lib/CodeGen/CGExprScalar.cpp:1712 BinOp.Opcode = IsInc ? BO_Add : BO_Sub; - BinOp.FPContractable = false; + // FIXME: once UnaryOperator carries FPFeatures, copy it here. BinOp.E = E; Why not make UnaryOperator carry this information now, since it's needed? https://reviews.llvm.org/D31166 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31166: Encapsulate FPOptions and use it consistently
anemet marked 3 inline comments as done. anemet added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1712 BinOp.Opcode = IsInc ? BO_Add : BO_Sub; - BinOp.FPContractable = false; + // FIXME: once UnaryOperator carries FPFeatures, copy it here. BinOp.E = E; aaron.ballman wrote: > Why not make UnaryOperator carry this information now, since it's needed? The trouble is that currently it's not needed. I'd rather wait for a fast-math flag that actually needs it so that we can write tests. https://reviews.llvm.org/D31166 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31166: Encapsulate FPOptions and use it consistently
anemet updated this revision to Diff 92898. anemet added a comment. Address Aaron's comments. https://reviews.llvm.org/D31166 Files: include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/Basic/LangOptions.h include/clang/Sema/Sema.h lib/AST/ASTImporter.cpp lib/Analysis/BodyFarm.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CGObjC.cpp lib/CodeGen/CGStmtOpenMP.cpp lib/Frontend/Rewrite/RewriteModernObjC.cpp lib/Frontend/Rewrite/RewriteObjC.cpp lib/Sema/SemaAttr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaOverload.cpp lib/Sema/SemaPseudoObject.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReader.cpp lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriter.cpp lib/Serialization/ASTWriterStmt.cpp Index: lib/Serialization/ASTWriterStmt.cpp === --- lib/Serialization/ASTWriterStmt.cpp +++ lib/Serialization/ASTWriterStmt.cpp @@ -650,7 +650,7 @@ Record.AddStmt(E->getRHS()); Record.push_back(E->getOpcode()); // FIXME: stable encoding Record.AddSourceLocation(E->getOperatorLoc()); - Record.push_back(E->isFPContractable()); + Record.push_back(E->getFPFeatures().getInt()); Code = serialization::EXPR_BINARY_OPERATOR; } @@ -1218,7 +1218,7 @@ VisitCallExpr(E); Record.push_back(E->getOperator()); Record.AddSourceRange(E->Range); - Record.push_back(E->isFPContractable()); + Record.push_back(E->getFPFeatures().getInt()); Code = serialization::EXPR_CXX_OPERATOR_CALL; } Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -4013,7 +4013,7 @@ /// \brief Write an FP_PRAGMA_OPTIONS block for the given FPOptions. void ASTWriter::WriteFPPragmaOptions(const FPOptions &Opts) { - RecordData::value_type Record[] = {Opts.fp_contract}; + RecordData::value_type Record[] = {Opts.getInt()}; Stream.EmitRecord(FP_PRAGMA_OPTIONS, Record); } Index: lib/Serialization/ASTReaderStmt.cpp === --- lib/Serialization/ASTReaderStmt.cpp +++ lib/Serialization/ASTReaderStmt.cpp @@ -670,7 +670,7 @@ E->setRHS(Record.readSubExpr()); E->setOpcode((BinaryOperator::Opcode)Record.readInt()); E->setOperatorLoc(ReadSourceLocation()); - E->setFPContractable((bool)Record.readInt()); + E->setFPFeatures(FPOptions(Record.readInt())); } void ASTStmtReader::VisitCompoundAssignOperator(CompoundAssignOperator *E) { @@ -1225,7 +1225,7 @@ VisitCallExpr(E); E->Operator = (OverloadedOperatorKind)Record.readInt(); E->Range = Record.readSourceRange(); - E->setFPContractable((bool)Record.readInt()); + E->setFPFeatures(FPOptions(Record.readInt())); } void ASTStmtReader::VisitCXXConstructExpr(CXXConstructExpr *E) { Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -7367,7 +7367,7 @@ // FIXME: What happens if these are changed by a module import? if (!FPPragmaOptions.empty()) { assert(FPPragmaOptions.size() == 1 && "Wrong number of FP_PRAGMA_OPTIONS"); -SemaObj->FPFeatures.fp_contract = FPPragmaOptions[0]; +SemaObj->FPFeatures = FPOptions(FPPragmaOptions[0]); } SemaObj->OpenCLFeatures.copy(OpenCLExtensions); Index: lib/Sema/TreeTransform.h === --- lib/Sema/TreeTransform.h +++ lib/Sema/TreeTransform.h @@ -9146,7 +9146,7 @@ return E; Sema::FPContractStateRAII FPContractState(getSema()); - getSema().FPFeatures.fp_contract = E->isFPContractable(); + getSema().FPFeatures = E->getFPFeatures(); return getDerived().RebuildBinaryOperator(E->getOperatorLoc(), E->getOpcode(), LHS.get(), RHS.get()); @@ -9626,7 +9626,7 @@ return SemaRef.MaybeBindToTemporary(E); Sema::FPContractStateRAII FPContractState(getSema()); - getSema().FPFeatures.fp_contract = E->isFPContractable(); + getSema().FPFeatures = E->getFPFeatures(); return getDerived().RebuildCXXOperatorCallExpr(E->getOperator(), E->getOperatorLoc(), Index: lib/Sema/SemaPseudoObject.cpp === --- lib/Sema/SemaPseudoObject.cpp +++ lib/Sema/SemaPseudoObject.cpp @@ -447,7 +447,8 @@ syntactic = new (S.Context) BinaryOperator(syntacticLHS, capturedRHS, opcode, capturedRHS->getType(), capturedRHS->getValueKind(), - OK_Ordinary, opcLoc, false); + OK_Ordinary, opcLoc, + FPOp
[PATCH] D31166: Encapsulate FPOptions and use it consistently
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lib/CodeGen/CGExprScalar.cpp:1712 BinOp.Opcode = IsInc ? BO_Add : BO_Sub; - BinOp.FPContractable = false; + // FIXME: once UnaryOperator carries FPFeatures, copy it here. BinOp.E = E; anemet wrote: > aaron.ballman wrote: > > Why not make UnaryOperator carry this information now, since it's needed? > The trouble is that currently it's not needed. I'd rather wait for a > fast-math flag that actually needs it so that we can write tests. That seems sensible to me, thank you for the explanation. https://reviews.llvm.org/D31166 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31166: Encapsulate FPOptions and use it consistently
This revision was automatically updated to reflect the committed changes. Closed by commit rL298877: Encapsulate FPOptions and use it consistently (authored by anemet). Changed prior to commit: https://reviews.llvm.org/D31166?vs=92898&id=93166#toc Repository: rL LLVM https://reviews.llvm.org/D31166 Files: cfe/trunk/include/clang/AST/Expr.h cfe/trunk/include/clang/AST/ExprCXX.h cfe/trunk/include/clang/Basic/LangOptions.h cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/lib/Analysis/BodyFarm.cpp cfe/trunk/lib/CodeGen/CGExprScalar.cpp cfe/trunk/lib/CodeGen/CGObjC.cpp cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp cfe/trunk/lib/Sema/SemaAttr.cpp cfe/trunk/lib/Sema/SemaDeclCXX.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/lib/Sema/SemaExprCXX.cpp cfe/trunk/lib/Sema/SemaOverload.cpp cfe/trunk/lib/Sema/SemaPseudoObject.cpp cfe/trunk/lib/Sema/TreeTransform.h cfe/trunk/lib/Serialization/ASTReader.cpp cfe/trunk/lib/Serialization/ASTReaderStmt.cpp cfe/trunk/lib/Serialization/ASTWriter.cpp cfe/trunk/lib/Serialization/ASTWriterStmt.cpp Index: cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp === --- cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp +++ cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp @@ -7500,7 +7500,7 @@ BinaryOperator *addExpr = new (Context) BinaryOperator(castExpr, DRE, BO_Add, Context->getPointerType(Context->CharTy), - VK_RValue, OK_Ordinary, SourceLocation(), false); + VK_RValue, OK_Ordinary, SourceLocation(), FPOptions()); // Don't forget the parens to enforce the proper binding. ParenExpr *PE = new (Context) ParenExpr(SourceLocation(), SourceLocation(), Index: cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp === --- cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp +++ cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp @@ -2992,7 +2992,7 @@ BinaryOperator *lessThanExpr = new (Context) BinaryOperator(sizeofExpr, limit, BO_LE, Context->IntTy, VK_RValue, OK_Ordinary, SourceLocation(), - false); + FPOptions()); // (sizeof(returnType) <= 8 ? objc_msgSend(...) : objc_msgSend_stret(...)) ConditionalOperator *CondExpr = new (Context) ConditionalOperator(lessThanExpr, Index: cfe/trunk/lib/Serialization/ASTReaderStmt.cpp === --- cfe/trunk/lib/Serialization/ASTReaderStmt.cpp +++ cfe/trunk/lib/Serialization/ASTReaderStmt.cpp @@ -670,7 +670,7 @@ E->setRHS(Record.readSubExpr()); E->setOpcode((BinaryOperator::Opcode)Record.readInt()); E->setOperatorLoc(ReadSourceLocation()); - E->setFPContractable((bool)Record.readInt()); + E->setFPFeatures(FPOptions(Record.readInt())); } void ASTStmtReader::VisitCompoundAssignOperator(CompoundAssignOperator *E) { @@ -1225,7 +1225,7 @@ VisitCallExpr(E); E->Operator = (OverloadedOperatorKind)Record.readInt(); E->Range = Record.readSourceRange(); - E->setFPContractable((bool)Record.readInt()); + E->setFPFeatures(FPOptions(Record.readInt())); } void ASTStmtReader::VisitCXXConstructExpr(CXXConstructExpr *E) { Index: cfe/trunk/lib/Serialization/ASTReader.cpp === --- cfe/trunk/lib/Serialization/ASTReader.cpp +++ cfe/trunk/lib/Serialization/ASTReader.cpp @@ -7378,7 +7378,7 @@ // FIXME: What happens if these are changed by a module import? if (!FPPragmaOptions.empty()) { assert(FPPragmaOptions.size() == 1 && "Wrong number of FP_PRAGMA_OPTIONS"); -SemaObj->FPFeatures.fp_contract = FPPragmaOptions[0]; +SemaObj->FPFeatures = FPOptions(FPPragmaOptions[0]); } SemaObj->OpenCLFeatures.copy(OpenCLExtensions); Index: cfe/trunk/lib/Serialization/ASTWriterStmt.cpp === --- cfe/trunk/lib/Serialization/ASTWriterStmt.cpp +++ cfe/trunk/lib/Serialization/ASTWriterStmt.cpp @@ -650,7 +650,7 @@ Record.AddStmt(E->getRHS()); Record.push_back(E->getOpcode()); // FIXME: stable encoding Record.AddSourceLocation(E->getOperatorLoc()); - Record.push_back(E->isFPContractable()); + Record.push_back(E->getFPFeatures().getInt()); Code = serialization::EXPR_BINARY_OPERATOR; } @@ -1218,7 +1218,7 @@ VisitCallExpr(E); Record.push_back(E->getOperator()); Record.AddSourceRange(E->Range); - Record.push_back(E->isFPContractable()); + Record.push_back(E->getFPFeatures().getInt()); Code = serialization::EXPR_CXX_OPERATOR_CALL; }