[PATCH] D31166: Encapsulate FPOptions and use it consistently

[Adam Nemet via Phabricator via cfe-commits]

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

[PATCH] D31166: Encapsulate FPOptions and use it consistently

[Aaron Ballman via Phabricator via cfe-commits]

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

[Adam Nemet via Phabricator via cfe-commits]

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 ) {
-  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,
+   

[PATCH] D31166: Encapsulate FPOptions and use it consistently

[Adam Nemet via Phabricator via cfe-commits]

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

[Aaron Ballman via Phabricator via cfe-commits]

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 ) :
-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 )
+  : 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

[Adam Nemet via Phabricator via cfe-commits]

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 ) {
-  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
+++