Extended FPOptions with new attributes

[sepavloff]

Summary

This change added two new attributes, rounding mode and exception
behavior to the structure FPOptions. These attributes allow more
flexible treatment of specific floating point environment than it is
provided by fenv_access.

[aaron.ballman]

In general, this seems reasonable, but is missing test code.

clang/include/clang/Basic/LangOptions.h

200

Can you give these an FPR prefix?

214

And these an FPE prefix?

[kpn]

In D65994#1622840, @aaron.ballman wrote:

In general, this seems reasonable, but is missing test code.

The IRBuilder does have a strict FP mode setting now. When strict mode is enabled the (implemented) constrained intrinsics automatically replace the normal FP instructions. I wonder if that would be right for testing of this patch?

[rjmccall]

Since this setting changes IR output, you should write a test that emits IR for source code and tests that you get the right IR output.

[sepavloff]

Updated patch

[sepavloff]

In D65994#1625457, @rjmccall wrote:

Since this setting changes IR output, you should write a test that emits IR for source code and tests that you get the right IR output.

This patch is a part of patch chain, it extends FPOptions with new options. In D65997 pragma clang fp is extended to manipulate these options. Finally D66092 modifies code generator so that it emit IR depending on the new options in FPOptions, it makes possible to write IR tests.

In D65994#1623088, @kpn wrote:

In D65994#1622840, @aaron.ballman wrote:

In general, this seems reasonable, but is missing test code.

The IRBuilder does have a strict FP mode setting now. When strict mode is enabled the (implemented) constrained intrinsics automatically replace the normal FP instructions. I wonder if that would be right for testing of this patch?

It is just what D66092 does.

[rjmccall]

Oh, sorry, I confused several patches, then.

I don't actually think you need to break down the patches this finely; it would be fine to take all three steps to implement the feature in one pass. It's only important to break down the patches into more incremental components if there's significant refactoring required to prepare for the patch or if each step is substantially complex on its own. And it's nice for feature work to always be testable *somehow*.

[sepavloff]

Rebased the patch

[aaron.ballman]

LGTM, but I'll echo what @rjmccall said about this level of granularity being a bit too fine for review for future patches (we expect changes to be testable, which this is, but only if you also review other patches and notice the tests and how they relate to this patch, which can be easy for reviewers to miss).

[sammccall]

This patch increased the used size of BinaryOperator by 5 bits.
Those bits were all padding, but now BinaryOperatorBitfields is completely full at 32 bits and we can't add any new bits to Stmt without increasing BinaryOperator by 8 bytes. (See D75443 and D54526 for the optimization this would revert).

To squeeze in the new bit I'm planning to suggest squeezing getInt() to 7 bits (it encodes 3x2x5x3 = 90 states, so this is possible) but I'm not really familiar with this domain - if many of the 90 states are not possible it'd probably be useful to have some more bits back :-)

clang/include/clang/AST/Stmt.h

class BinaryOperatorBitfields {
     friend class BinaryOperator;
 
     unsigned : NumExprBits;
 
     unsigned Opc : 6;
 
     /// This is only meaningful for operations on floating point
     /// types and 0 otherwise.
-    unsigned FPFeatures : 3;
+    unsigned FPFeatures : 8;
 
     SourceLocation OpLoc;
   };
 
   class InitListExprBitfields {
     friend class InitListExpr;
 
     unsigned : NumExprBits;
class CXXOperatorCallExprBitfields {
 
     unsigned : NumCallExprBits;
 
     /// The kind of this overloaded operator. One of the enumerator
     /// value of OverloadedOperatorKind.
     unsigned OperatorKind : 6;
 
     // Only meaningful for floating point types.
-    unsigned FPFeatures : 3;
+    unsigned FPFeatures : 8;
   };
 
   class CXXRewrittenBinaryOperatorBitfields {
     friend class ASTStmtReader;
     friend class CXXRewrittenBinaryOperator;
 
     unsigned : NumCallExprBits;
 

clang/include/clang/Basic/LangOptions.h

public:
 
   // Values of the following enumerations correspond to metadata arguments
   // specified for constrained floating-point intrinsics:
   // http://llvm.org/docs/LangRef.html#constrained-floating-point-intrinsics.
 
   /// Possible rounding modes.
   enum FPRoundingModeKind {
     /// Rounding to nearest, corresponds to "round.tonearest".
     FPR_ToNearest,
     /// Rounding toward -Inf, corresponds to "round.downward".
     FPR_Downward,
     /// Rounding toward +Inf, corresponds to "round.upward".
     FPR_Upward,
     /// Rounding toward zero, corresponds to "round.towardzero".
     FPR_TowardZero,
     /// Is determined by runtime environment, corresponds to "round.dynamic".
     FPR_Dynamic
   };
 
   /// Possible floating point exception behavior.
   enum FPExceptionModeKind {
     /// Assume that floating-point exceptions are masked.
     FPE_Ignore,
     /// Transformations do not cause new exceptions but may hide some.
     FPE_MayTrap,
     /// Strictly preserve the floating-point exception semantics.
     FPE_Strict
   };
 
   enum class LaxVectorConversionKind {
     /// Permit no implicit vector bitcasts.
#include "clang/Basic/LangOptions.def"
   /// Return the OpenCL C or C++ version as a VersionTuple.
   VersionTuple getOpenCLVersionTuple() const;
 };
 
 /// Floating point control options
 class FPOptions {
 public:
   FPOptions() : fp_contract(LangOptions::FPC_Off),
-                fenv_access(LangOptions::FEA_Off) {}
+                fenv_access(LangOptions::FEA_Off),
+                rounding(LangOptions::FPR_ToNearest),
+                exceptions(LangOptions::FPE_Ignore)
+        {}
 
   // Used for serializing.
   explicit FPOptions(unsigned I)
       : fp_contract(static_cast<LangOptions::FPContractModeKind>(I & 3)),
-        fenv_access(static_cast<LangOptions::FEnvAccessModeKind>((I >> 2) & 1))
+        fenv_access(static_cast<LangOptions::FEnvAccessModeKind>((I >> 2) & 1)),
+        rounding(static_cast<LangOptions::FPRoundingModeKind>((I >> 3) & 7)),
+        exceptions(static_cast<LangOptions::FPExceptionModeKind>((I >> 6) & 3))
         {}
 
   explicit FPOptions(const LangOptions &LangOpts)
       : fp_contract(LangOpts.getDefaultFPContractMode()),
-        fenv_access(LangOptions::FEA_Off) {}
+        fenv_access(LangOptions::FEA_Off),
+        rounding(LangOptions::FPR_ToNearest),
+        exceptions(LangOptions::FPE_Ignore)
+        {}
   // FIXME: Use getDefaultFEnvAccessMode() when available.
 
   bool allowFPContractWithinStatement() const {
     return fp_contract == LangOptions::FPC_On;
   }
 
   bool allowFPContractAcrossStatement() const {
     return fp_contract == LangOptions::FPC_Fast;
public:
   }
 
   void setAllowFEnvAccess() {
     fenv_access = LangOptions::FEA_On;
   }
 
   void setDisallowFEnvAccess() { fenv_access = LangOptions::FEA_Off; }
 
+  LangOptions::FPRoundingModeKind getRoundingMode() const {
+    return static_cast<LangOptions::FPRoundingModeKind>(rounding);
+  }
+
+  void setRoundingMode(LangOptions::FPRoundingModeKind RM) {
+    rounding = RM;
+  }
+
+  LangOptions::FPExceptionModeKind getExceptionMode() const {
+    return static_cast<LangOptions::FPExceptionModeKind>(exceptions);
+  }
+
+  void setExceptionMode(LangOptions::FPExceptionModeKind EM) {
+    exceptions = EM;
+  }
+
+  bool isFPConstrained() const {
+    return getRoundingMode() != LangOptions::FPR_ToNearest ||
+           getExceptionMode() != LangOptions::FPE_Ignore ||
+           allowFEnvAccess();
+  }
+
   /// Used to serialize this.
-  unsigned getInt() const { return fp_contract | (fenv_access << 2); }
+  unsigned getInt() const {
+    return fp_contract | (fenv_access << 2) | (rounding << 3)
+        | (exceptions << 6);
+  }
 
 private:
-  /// Adjust BinaryOperator::FPFeatures to match the total bit-field size
-  /// of these two.
+  /// Adjust BinaryOperatorBitfields::FPFeatures and
+  /// CXXOperatorCallExprBitfields::FPFeatures to match the total bit-field size
+  /// of these fields.
   unsigned fp_contract : 2;
   unsigned fenv_access : 1;
+  unsigned rounding : 3;
+  unsigned exceptions : 2;
 };
 
 /// Describes the kind of translation unit being processed.
 enum TranslationUnitKind {
   /// The translation unit is a complete translation unit.
   TU_Complete,
 
   /// The translation unit is a prefix to a translation unit, and is

clang/include/clang/Sema/Sema.h

public:
   bool isSelfExpr(Expr *RExpr);
   bool isSelfExpr(Expr *RExpr, const ObjCMethodDecl *Method);
 
   /// Cause the active diagnostic on the DiagosticsEngine to be
   /// emitted. This is closely coupled to the SemaDiagnosticBuilder class and
   /// should not be used elsewhere.
   void EmitCurrentDiagnostic(unsigned DiagID);
 
-  /// Records and restores the FP_CONTRACT state on entry/exit of compound
+  /// Records and restores the FPFeatures state on entry/exit of compound
   /// statements.
-  class FPContractStateRAII {
+  class FPFeaturesStateRAII {
   public:
-    FPContractStateRAII(Sema &S) : S(S), OldFPFeaturesState(S.FPFeatures) {}
-    ~FPContractStateRAII() { S.FPFeatures = OldFPFeaturesState; }
+    FPFeaturesStateRAII(Sema &S) : S(S), OldFPFeaturesState(S.FPFeatures) {}
+    ~FPFeaturesStateRAII() { S.FPFeatures = OldFPFeaturesState; }
 
   private:
     Sema& S;
     FPOptions OldFPFeaturesState;
   };
 
   void addImplicitTypedef(StringRef Name, QualType T);
 

   /// \#pragma {STDC,OPENCL} FP_CONTRACT and
   /// \#pragma clang fp contract
   void ActOnPragmaFPContract(LangOptions::FPContractModeKind FPC);
 
   /// ActOnPragmaFenvAccess - Called on well formed
   /// \#pragma STDC FENV_ACCESS
   void ActOnPragmaFEnvAccess(LangOptions::FEnvAccessModeKind FPC);
 
+  /// Called to set rounding mode for floating point operations.
+  void setRoundingMode(LangOptions::FPRoundingModeKind);
+
+  /// Called to set exception behavior for floating point operations.
+  void setExceptionMode(LangOptions::FPExceptionModeKind);
+
   /// AddAlignmentAttributesForRecord - Adds any needed alignment attributes to
   /// a the record decl, to handle '\#pragma pack' and '\#pragma options align'.
   void AddAlignmentAttributesForRecord(RecordDecl *RD);
 
   /// AddMsStructLayoutForRecord - Adds ms_struct layout attribute to record.
   void AddMsStructLayoutForRecord(RecordDecl *RD);
 
   /// FreePackedContext - Deallocate and null out PackContext.

clang/lib/Parse/ParseStmt.cpp

 /// ActOnCompoundStmt action.  This expects the '{' to be the current token, and
 /// consume the '}' at the end of the block.  It does not manipulate the scope
 /// stack.
 StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
   PrettyStackTraceLoc CrashInfo(PP.getSourceManager(),
                                 Tok.getLocation(),
                                 "in compound statement ('{}')");
 
-  // Record the state of the FP_CONTRACT pragma, restore on leaving the
+  // Record the state of the FPFeatures, restore on leaving the
   // compound statement.
-  Sema::FPContractStateRAII SaveFPContractState(Actions);
+  Sema::FPFeaturesStateRAII SaveFPContractState(Actions);
 
   InMessageExpressionRAIIObject InMessage(*this, false);
   BalancedDelimiterTracker T(*this, tok::l_brace);
   if (T.consumeOpen())
     return StmtError();
 
   Sema::CompoundScopeRAII CompoundScope(Actions, isStmtExpr);
 

clang/lib/Sema/SemaAttr.cpp

case LangOptions::FPC_Fast:
     FPFeatures.setAllowFPContractAcrossStatement();
     break;
   case LangOptions::FPC_Off:
     FPFeatures.setDisallowFPContract();
     break;
   }
 }
 
+void Sema::setRoundingMode(LangOptions::FPRoundingModeKind FPR) {
+  FPFeatures.setRoundingMode(FPR);
+}
+
+void Sema::setExceptionMode(LangOptions::FPExceptionModeKind FPE) {
+  FPFeatures.setExceptionMode(FPE);
+}
+
 void Sema::ActOnPragmaFEnvAccess(LangOptions::FEnvAccessModeKind FPC) {
   switch (FPC) {
   case LangOptions::FEA_On:
     FPFeatures.setAllowFEnvAccess();
     break;
   case LangOptions::FEA_Off:
     FPFeatures.setDisallowFEnvAccess();
     break;

clang/lib/Sema/TreeTransform.h

TreeTransform<Derived>::TransformBinaryOperator(BinaryOperator *E) {
   if (RHS.isInvalid())
     return ExprError();
 
   if (!getDerived().AlwaysRebuild() &&
       LHS.get() == E->getLHS() &&
       RHS.get() == E->getRHS())
     return E;
 
-  Sema::FPContractStateRAII FPContractState(getSema());
+  Sema::FPFeaturesStateRAII FPFeaturesState(getSema());
   getSema().FPFeatures = E->getFPFeatures();
 
   return getDerived().RebuildBinaryOperator(E->getOperatorLoc(), E->getOpcode(),
                                             LHS.get(), RHS.get());
 }
 
 template <typename Derived>
 ExprResult TreeTransform<Derived>::TransformCXXRewrittenBinaryOperator(
#include "clang/Basic/OperatorKinds.def"
   }
 
   if (!getDerived().AlwaysRebuild() &&
       Callee.get() == E->getCallee() &&
       First.get() == E->getArg(0) &&
       (E->getNumArgs() != 2 || Second.get() == E->getArg(1)))
     return SemaRef.MaybeBindToTemporary(E);
 
-  Sema::FPContractStateRAII FPContractState(getSema());
+  Sema::FPFeaturesStateRAII FPFeaturesState(getSema());
   getSema().FPFeatures = E->getFPFeatures();
 
   return getDerived().RebuildCXXOperatorCallExpr(E->getOperator(),
                                                  E->getOperatorLoc(),
                                                  Callee.get(),
                                                  First.get(),
                                                  Second.get());
 }