[PATCH] D102343: [clang][patch][FPEnv} Initialization of C++ globals not strictfp aware
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: l
[PATCH] D102343: [clang][patch][FPEnv} Initialization of C++ globals not strictfp aware
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 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
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) -fl
[PATCH] D102343: [clang][patch][FPEnv} Initialization of C++ globals not strictfp aware
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, 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
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 &NameInfo, 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
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{{.*}} @@ -1
[PATCH] D102343: [clang][patch][FPEnv} Initialization of C++ globals not strictfp aware
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(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
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
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 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
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
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 F