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

[Melanie Blower via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbc5b5ea037db: [clang][patch][FPEnv] Make initialization of 
C++ globals strictfp aware (authored by mibintc).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102343

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGen/fp-floatcontrol-class.cpp
  clang/test/CodeGen/fp-floatcontrol-stack.cpp
  clang/unittests/Sema/ExternalSemaSourceTest.cpp

Index: clang/unittests/Sema/ExternalSemaSourceTest.cpp
===
--- clang/unittests/Sema/ExternalSemaSourceTest.cpp
+++ clang/unittests/Sema/ExternalSemaSourceTest.cpp
@@ -163,7 +163,8 @@
   CurrentSema->getPreprocessor().getIdentifierInfo(CorrectTo);
   auto *NewFunction = FunctionDecl::Create(
   Context, DestContext, SourceLocation(), SourceLocation(), ToIdent,
-  Context.getFunctionType(Context.VoidTy, {}, {}), nullptr, SC_Static);
+  Context.getFunctionType(Context.VoidTy, {}, {}), nullptr, SC_Static,
+  /*UsesFPIntrin*/ false);
   DestContext->addDecl(NewFunction);
   TypoCorrection Correction(ToIdent);
   Correction.addCorrectionDecl(NewFunction);
Index: clang/test/CodeGen/fp-floatcontrol-stack.cpp
===
--- clang/test/CodeGen/fp-floatcontrol-stack.cpp
+++ clang/test/CodeGen/fp-floatcontrol-stack.cpp
@@ -6,6 +6,10 @@
 #define FUN(n) \
   (float z) { return n * z + n; }
 
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone mustprogress
 float fun_default FUN(1)
 //CHECK-LABEL: define {{.*}} @_Z11fun_defaultf{{.*}}
 #if DEFAULT
@@ -28,6 +32,10 @@
 // Rule: precise must be enabled
 #pragma float_control(except, on)
 #endif
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone strictfp mustprogress
 float exc_on FUN(2)
 //CHECK-LABEL: define {{.*}} @_Z6exc_onf{{.*}}
 #if DEFAULT
@@ -46,7 +54,11 @@
 #endif
 
 #pragma float_control(pop)
-float exc_pop FUN(5)
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone mustprogress
+float exc_pop FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_popf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
@@ -63,7 +75,7 @@
 #endif
 
 #pragma float_control(except, off)
-float exc_off FUN(5)
+float exc_off FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_offf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
@@ -80,7 +92,7 @@
 #endif
 
 #pragma float_control(precise, on, push)
-float precise_on FUN(3)
+float precise_on FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z10precise_onf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
@@ -97,7 +109,7 @@
 #endif
 
 #pragma float_control(pop)
-float precise_pop FUN(3)
+float precise_pop FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z11precise_popf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
@@ -113,7 +125,7 @@
 //CHECK-FAST: fadd fast float
 #endif
 #pragma float_control(precise, off)
-float precise_off FUN(4)
+float precise_off FUN(4)
 //CHECK-LABEL: define {{.*}} @_Z11precise_offf{{.*}}
 #if DEFAULT
 // Note: precise_off enables fp_contract=fast and the instructions
@@ -137,7 +149,7 @@
 #endif
 
 #pragma float_control(precise, on)
-float precise_on2 FUN(3)
+float precise_on2 FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z11precise_on2f{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: 

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

[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.

Giving my official LGTM now that there's been a week for folks to comment with 
additional concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102343

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The changes LGTM, but I'd appreciate additional sign-off from someone with more 
expertise in this area. @rjmccall, can you take a look to see if it addresses 
your concerns?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102343

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 360225.
mibintc added a comment.

Removed use of auto, and used different capitalization for local var name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102343

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGen/fp-floatcontrol-class.cpp
  clang/test/CodeGen/fp-floatcontrol-stack.cpp
  clang/unittests/Sema/ExternalSemaSourceTest.cpp

Index: clang/unittests/Sema/ExternalSemaSourceTest.cpp
===
--- clang/unittests/Sema/ExternalSemaSourceTest.cpp
+++ clang/unittests/Sema/ExternalSemaSourceTest.cpp
@@ -163,7 +163,8 @@
   CurrentSema->getPreprocessor().getIdentifierInfo(CorrectTo);
   auto *NewFunction = FunctionDecl::Create(
   Context, DestContext, SourceLocation(), SourceLocation(), ToIdent,
-  Context.getFunctionType(Context.VoidTy, {}, {}), nullptr, SC_Static);
+  Context.getFunctionType(Context.VoidTy, {}, {}), nullptr, SC_Static,
+  /*UsesFPIntrin*/ false);
   DestContext->addDecl(NewFunction);
   TypoCorrection Correction(ToIdent);
   Correction.addCorrectionDecl(NewFunction);
Index: clang/test/CodeGen/fp-floatcontrol-stack.cpp
===
--- clang/test/CodeGen/fp-floatcontrol-stack.cpp
+++ clang/test/CodeGen/fp-floatcontrol-stack.cpp
@@ -6,6 +6,10 @@
 #define FUN(n) \
   (float z) { return n * z + n; }
 
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone mustprogress
 float fun_default FUN(1)
 //CHECK-LABEL: define {{.*}} @_Z11fun_defaultf{{.*}}
 #if DEFAULT
@@ -28,6 +32,10 @@
 // Rule: precise must be enabled
 #pragma float_control(except, on)
 #endif
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone strictfp mustprogress
 float exc_on FUN(2)
 //CHECK-LABEL: define {{.*}} @_Z6exc_onf{{.*}}
 #if DEFAULT
@@ -46,7 +54,11 @@
 #endif
 
 #pragma float_control(pop)
-float exc_pop FUN(5)
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone mustprogress
+float exc_pop FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_popf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
@@ -63,7 +75,7 @@
 #endif
 
 #pragma float_control(except, off)
-float exc_off FUN(5)
+float exc_off FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_offf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
@@ -80,7 +92,7 @@
 #endif
 
 #pragma float_control(precise, on, push)
-float precise_on FUN(3)
+float precise_on FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z10precise_onf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
@@ -97,7 +109,7 @@
 #endif
 
 #pragma float_control(pop)
-float precise_pop FUN(3)
+float precise_pop FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z11precise_popf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
@@ -113,7 +125,7 @@
 //CHECK-FAST: fadd fast float
 #endif
 #pragma float_control(precise, off)
-float precise_off FUN(4)
+float precise_off FUN(4)
 //CHECK-LABEL: define {{.*}} @_Z11precise_offf{{.*}}
 #if DEFAULT
 // Note: precise_off enables fp_contract=fast and the instructions
@@ -137,7 +149,7 @@
 #endif
 
 #pragma float_control(precise, on)
-float precise_on2 FUN(3)
+float precise_on2 FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z11precise_on2f{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: llvm.fmuladd{{.*}}
@@ -154,7 +166,7 @@
 #endif
 
 #pragma float_control(push)
-

[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 , QualType T,
-   TypeSourceInfo *TInfo, StorageClass S, bool isInlineSpecified,
-   ConstexprSpecKind ConstexprKind,
+   TypeSourceInfo *TInfo, StorageClass S, bool UsesFPIntrin,
+   bool isInlineSpecified, ConstexprSpecKind ConstexprKind,

mibintc wrote:
> aaron.ballman wrote:
> > 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`?
> I spent a few hours recoding to rearrange the parameter list, but there are 
> downstream types from FunctionDecl which would presumably also need to be 
> rearranged, and those types add more parameters and some of those parameters 
> have default values, and moving the new boolean before TrailingRequiresClause 
> doesn't work, I decided to throw away the mess I made and ask if you could 
> live with it this way. 
Yup, I can live with it this way if changing it causes trouble. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102343

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked an inline comment as done.
mibintc added a comment.

Question for Aaron




Comment at: clang/include/clang/AST/Decl.h:1993
const DeclarationNameInfo , QualType T,
-   TypeSourceInfo *TInfo, StorageClass S, bool isInlineSpecified,
-   ConstexprSpecKind ConstexprKind,
+   TypeSourceInfo *TInfo, StorageClass S, bool UsesFPIntrin,
+   bool isInlineSpecified, ConstexprSpecKind ConstexprKind,

aaron.ballman wrote:
> 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`?
I spent a few hours recoding to rearrange the parameter list, but there are 
downstream types from FunctionDecl which would presumably also need to be 
rearranged, and those types add more parameters and some of those parameters 
have default values, and moving the new boolean before TrailingRequiresClause 
doesn't work, I decided to throw away the mess I made and ask if you could live 
with it this way. 



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:939
+  llvm::RoundingMode RM = getLangOpts().getFPRoundingMode();
+  auto fpExceptionBehavior =
+  ToConstrainedExceptMD(getLangOpts().getFPExceptionMode());

aaron.ballman wrote:
> Please spell out the type rather than use `auto`, also, should probably be 
> `FP` rather than `fp` per the usual naming conventions.
oops i didn't get this one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102343

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 360222.
mibintc added a comment.

Partially respond to @aaron.ballman 's review by refactoring a change into a 
separate commit, but I'll push back on another request, I'll add that reply 
inline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102343

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGen/fp-floatcontrol-class.cpp
  clang/test/CodeGen/fp-floatcontrol-stack.cpp
  clang/unittests/Sema/ExternalSemaSourceTest.cpp

Index: clang/unittests/Sema/ExternalSemaSourceTest.cpp
===
--- clang/unittests/Sema/ExternalSemaSourceTest.cpp
+++ clang/unittests/Sema/ExternalSemaSourceTest.cpp
@@ -163,7 +163,8 @@
   CurrentSema->getPreprocessor().getIdentifierInfo(CorrectTo);
   auto *NewFunction = FunctionDecl::Create(
   Context, DestContext, SourceLocation(), SourceLocation(), ToIdent,
-  Context.getFunctionType(Context.VoidTy, {}, {}), nullptr, SC_Static);
+  Context.getFunctionType(Context.VoidTy, {}, {}), nullptr, SC_Static,
+  /*UsesFPIntrin*/ false);
   DestContext->addDecl(NewFunction);
   TypoCorrection Correction(ToIdent);
   Correction.addCorrectionDecl(NewFunction);
Index: clang/test/CodeGen/fp-floatcontrol-stack.cpp
===
--- clang/test/CodeGen/fp-floatcontrol-stack.cpp
+++ clang/test/CodeGen/fp-floatcontrol-stack.cpp
@@ -6,6 +6,10 @@
 #define FUN(n) \
   (float z) { return n * z + n; }
 
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone mustprogress
 float fun_default FUN(1)
 //CHECK-LABEL: define {{.*}} @_Z11fun_defaultf{{.*}}
 #if DEFAULT
@@ -28,6 +32,10 @@
 // Rule: precise must be enabled
 #pragma float_control(except, on)
 #endif
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone strictfp mustprogress
 float exc_on FUN(2)
 //CHECK-LABEL: define {{.*}} @_Z6exc_onf{{.*}}
 #if DEFAULT
@@ -46,7 +54,11 @@
 #endif
 
 #pragma float_control(pop)
-float exc_pop FUN(5)
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone mustprogress
+float exc_pop FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_popf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
@@ -63,7 +75,7 @@
 #endif
 
 #pragma float_control(except, off)
-float exc_off FUN(5)
+float exc_off FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_offf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
@@ -80,7 +92,7 @@
 #endif
 
 #pragma float_control(precise, on, push)
-float precise_on FUN(3)
+float precise_on FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z10precise_onf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
@@ -97,7 +109,7 @@
 #endif
 
 #pragma float_control(pop)
-float precise_pop FUN(3)
+float precise_pop FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z11precise_popf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
@@ -113,7 +125,7 @@
 //CHECK-FAST: fadd fast float
 #endif
 #pragma float_control(precise, off)
-float precise_off FUN(4)
+float precise_off FUN(4)
 //CHECK-LABEL: define {{.*}} @_Z11precise_offf{{.*}}
 #if DEFAULT
 // Note: precise_off enables fp_contract=fast and the instructions
@@ -137,7 +149,7 @@
 #endif
 
 #pragma float_control(precise, on)
-float precise_on2 FUN(3)
+float precise_on2 FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z11precise_on2f{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: llvm.fmuladd{{.*}}
@@ 

[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 , 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(D))
-if (FD->usesSEHTry())
-  CurSEHParent = FD;
+  const FunctionDecl *FD = dyn_cast_or_null(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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked an inline comment as done.
mibintc added inline comments.



Comment at: clang/lib/Sema/SemaLambda.cpp:395
   MethodNameLoc),
-  MethodType, MethodTypeInfo, SC_None,
+  MethodType, MethodTypeInfo, SC_None, /*UsesFPIntrin*/ false,
   /*isInline=*/true, ConstexprKind, EndLoc, TrailingRequiresClause);

aaron.ballman wrote:
> Should lambdas pick up the same fp constraints as the function defining the 
> lambda?
I made it use the FP settings from Sema, 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102343

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 359772.
mibintc added a reviewer: zahiraam.
mibintc added a comment.

Respond to @aaron.ballman 's review, rebased & used clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102343

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGen/fp-floatcontrol-class.cpp
  clang/test/CodeGen/fp-floatcontrol-stack.cpp
  clang/unittests/Sema/ExternalSemaSourceTest.cpp

Index: clang/unittests/Sema/ExternalSemaSourceTest.cpp
===
--- clang/unittests/Sema/ExternalSemaSourceTest.cpp
+++ clang/unittests/Sema/ExternalSemaSourceTest.cpp
@@ -163,7 +163,8 @@
   CurrentSema->getPreprocessor().getIdentifierInfo(CorrectTo);
   auto *NewFunction = FunctionDecl::Create(
   Context, DestContext, SourceLocation(), SourceLocation(), ToIdent,
-  Context.getFunctionType(Context.VoidTy, {}, {}), nullptr, SC_Static);
+  Context.getFunctionType(Context.VoidTy, {}, {}), nullptr, SC_Static,
+  /*UsesFPIntrin*/ false);
   DestContext->addDecl(NewFunction);
   TypoCorrection Correction(ToIdent);
   Correction.addCorrectionDecl(NewFunction);
Index: clang/test/CodeGen/fp-floatcontrol-stack.cpp
===
--- clang/test/CodeGen/fp-floatcontrol-stack.cpp
+++ clang/test/CodeGen/fp-floatcontrol-stack.cpp
@@ -6,6 +6,10 @@
 #define FUN(n) \
   (float z) { return n * z + n; }
 
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone mustprogress
 float fun_default FUN(1)
 //CHECK-LABEL: define {{.*}} @_Z11fun_defaultf{{.*}}
 #if DEFAULT
@@ -28,6 +32,10 @@
 // Rule: precise must be enabled
 #pragma float_control(except, on)
 #endif
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone strictfp mustprogress
 float exc_on FUN(2)
 //CHECK-LABEL: define {{.*}} @_Z6exc_onf{{.*}}
 #if DEFAULT
@@ -46,7 +54,11 @@
 #endif
 
 #pragma float_control(pop)
-float exc_pop FUN(5)
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-FAST: Function Attrs: mustprogress noinline nounwind optnone
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone mustprogress
+float exc_pop FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_popf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
@@ -63,7 +75,7 @@
 #endif
 
 #pragma float_control(except, off)
-float exc_off FUN(5)
+float exc_off FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_offf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
@@ -80,7 +92,7 @@
 #endif
 
 #pragma float_control(precise, on, push)
-float precise_on FUN(3)
+float precise_on FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z10precise_onf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
@@ -97,7 +109,7 @@
 #endif
 
 #pragma float_control(pop)
-float precise_pop FUN(3)
+float precise_pop FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z11precise_popf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
@@ -113,7 +125,7 @@
 //CHECK-FAST: fadd fast float
 #endif
 #pragma float_control(precise, off)
-float precise_off FUN(4)
+float precise_off FUN(4)
 //CHECK-LABEL: define {{.*}} @_Z11precise_offf{{.*}}
 #if DEFAULT
 // Note: precise_off enables fp_contract=fast and the instructions
@@ -137,7 +149,7 @@
 #endif
 
 #pragma float_control(precise, on)
-float precise_on2 FUN(3)
+float precise_on2 FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z11precise_on2f{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: llvm.fmuladd{{.*}}
@@ -154,7 +166,7 @@
 #endif
 
 #pragma float_control(push)
-   

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: rsmith, erichkeane.
aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:2685
+   bool UsesFPIntrin, bool isInline,
+  bool isImplicitlyDeclared,
ConstexprSpecKind ConstexprKind,

Formatting looks off here.



Comment at: clang/include/clang/AST/DeclCXX.h:2729
   : CXXMethodDecl(CXXConversion, C, RD, StartLoc, NameInfo, T, TInfo,
-  SC_None, isInline, ConstexprKind, EndLocation,
+  SC_None, false, isInline, ConstexprKind, EndLocation,
   TrailingRequiresClause),

If ctors and cxxmethods get the parameter, why not conversion operators?



Comment at: clang/lib/Sema/SemaLambda.cpp:395
   MethodNameLoc),
-  MethodType, MethodTypeInfo, SC_None,
+  MethodType, MethodTypeInfo, SC_None, /*UsesFPIntrin*/ false,
   /*isInline=*/true, ConstexprKind, EndLoc, TrailingRequiresClause);

Should lambdas pick up the same fp constraints as the function defining the 
lambda?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102343

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:700
   CurCodeDecl = D;
-  if (const auto *FD = dyn_cast_or_null(D))
-if (FD->usesSEHTry())
-  CurSEHParent = FD;
+  const FunctionDecl *FD = dyn_cast_or_null(D);
+  if (FD && FD->usesSEHTry())

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102343

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Melanie Blower via Phabricator via cfe-commits]

mibintc created this revision.
mibintc added reviewers: kpn, rjmccall, aaron.ballman, pcc.
Herald added a subscriber: martong.
Herald added a reviewer: shafik.
mibintc requested review of this revision.
Herald added a project: clang.

In the proposed patch https://reviews.llvm.org/D81178 @kpn pointed out that the 
global variable initialization functions didn't have the "strictfp" metadata 
set correctly, and @rjmccall said that there was buggy code in SetFPModel and 
StartFunction, this patch is to solve those problems. When Sema creates a 
FunctionDecl, it sets the FunctionDeclBits.UsesFPIntrin to "true" if the 
lexical FP settings (i.e. a combination of command line options and #pragma 
float_control settings) correspond to ConstrainedFP mode.  That bit is used 
when CodeGen starts codegen for a llvm function, and it translates into the 
"strictfp" function attribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102343

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGNonTrivialStruct.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGen/fp-floatcontrol-class.cpp
  clang/test/CodeGen/fp-floatcontrol-stack.cpp
  clang/unittests/Sema/ExternalSemaSourceTest.cpp

Index: clang/unittests/Sema/ExternalSemaSourceTest.cpp
===
--- clang/unittests/Sema/ExternalSemaSourceTest.cpp
+++ clang/unittests/Sema/ExternalSemaSourceTest.cpp
@@ -163,7 +163,8 @@
   CurrentSema->getPreprocessor().getIdentifierInfo(CorrectTo);
   auto *NewFunction = FunctionDecl::Create(
   Context, DestContext, SourceLocation(), SourceLocation(), ToIdent,
-  Context.getFunctionType(Context.VoidTy, {}, {}), nullptr, SC_Static);
+  Context.getFunctionType(Context.VoidTy, {}, {}), nullptr, SC_Static,
+  /*UsesFPIntrin*/ false);
   DestContext->addDecl(NewFunction);
   TypoCorrection Correction(ToIdent);
   Correction.addCorrectionDecl(NewFunction);
Index: clang/test/CodeGen/fp-floatcontrol-stack.cpp
===
--- clang/test/CodeGen/fp-floatcontrol-stack.cpp
+++ clang/test/CodeGen/fp-floatcontrol-stack.cpp
@@ -6,6 +6,10 @@
 #define FUN(n) \
   (float z) { return n * z + n; }
 
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-FAST: Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone mustprogress
 float fun_default FUN(1)
 //CHECK-LABEL: define {{.*}} @_Z11fun_defaultf{{.*}}
 #if DEFAULT
@@ -28,6 +32,10 @@
 // Rule: precise must be enabled
 #pragma float_control(except, on)
 #endif
+// CHECK-FAST: Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone strictfp mustprogress
 float exc_on FUN(2)
 //CHECK-LABEL: define {{.*}} @_Z6exc_onf{{.*}}
 #if DEFAULT
@@ -46,7 +54,11 @@
 #endif
 
 #pragma float_control(pop)
-float exc_pop FUN(5)
+// CHECK-DDEFAULT Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-DEBSTRICT Function Attrs: noinline nounwind optnone strictfp mustprogress
+// CHECK-FAST: Function Attrs: noinline nounwind optnone mustprogress
+// CHECK-NOHONOR Function Attrs: noinline nounwind optnone mustprogress
+float exc_pop FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_popf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
@@ -63,7 +75,7 @@
 #endif
 
 #pragma float_control(except, off)
-float exc_off FUN(5)
+float exc_off FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_offf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
@@ -80,7 +92,7 @@
 #endif
 
 #pragma float_control(precise, on, push)
-float precise_on FUN(3)
+float precise_on FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z10precise_onf{{.*}}
 #if DEFAULT
 //CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
@@ -97,7 +109,7 @@
 #endif
 
 #pragma float_control(pop)
-float precise_pop FUN(3)
+float precise_pop