[PATCH] D102343: [clang][patch][FPEnv} Initialization of C++ globals not strictfp aware

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/AST/Decl.h:1993
                const DeclarationNameInfo &NameInfo, QualType T,
-               TypeSourceInfo *TInfo, StorageClass S, bool isInlineSpecified,
-               ConstexprSpecKind ConstexprKind,
+               TypeSourceInfo *TInfo, StorageClass S, bool UsesFPIntrin,
+               bool isInlineSpecified, ConstexprSpecKind ConstexprKind,
----------------
I have no idea if others agree, but I have a slight preference for putting `UsesFPIntrin` towards the end of the parameter list (here and in the Create methods) -- this flag seems less important than the constexpr kind or whether inline is specified (and, it helpfully avoids putting two `bool` parameters next to one another).

If you agree, perhaps it could be moved to before `TrailingRequiresClause`?


================
Comment at: clang/include/clang/AST/Decl.h:2598
+  /// Determine whether the function was declared in source context
+  /// that requires constrained FP intrinsics
+  bool UsesFPIntrin() const { return FunctionDeclBits.UsesFPIntrin; }
----------------



================
Comment at: clang/include/clang/AST/Decl.h:2602
+  /// Set whether the function was declared in source context
+  /// that requires constrained FP intrinsics
+  void setUsesFPIntrin(bool I) { FunctionDeclBits.UsesFPIntrin = I; }
----------------



================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:939
+  llvm::RoundingMode RM = getLangOpts().getFPRoundingMode();
+  auto fpExceptionBehavior =
+      ToConstrainedExceptMD(getLangOpts().getFPExceptionMode());
----------------
Please spell out the type rather than use `auto`, also, should probably be `FP` rather than `fp` per the usual naming conventions.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:700
   CurCodeDecl = D;
-  if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D))
-    if (FD->usesSEHTry())
-      CurSEHParent = FD;
+  const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D);
+  if (FD && FD->usesSEHTry())
----------------
mibintc wrote:
> there's a whole bunch of places in this func that creates FD, I changed them into a single place. hope that's OK, or i could pull it out as a pre-patch if you prefer
I think it'd be better to separate these changes into an NFC patch (feel free to land it without review; I've checked the changes here and they look correct and NFC).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102343/new/

https://reviews.llvm.org/D102343