[PATCH] D73495: [CodeGen] Attach no-builtin attributes to function definitions with no Decl
gchatelet added a comment.
Thx for the fix @thegameg
Comment at: clang/lib/CodeGen/CGCall.cpp:1845
+
+ // First, handle the language options passed through -fno-builtin[-]
+ if (LangOpts.NoBuiltin) {
Can you remove the trailing `[-]`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73495/new/
https://reviews.llvm.org/D73495
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73495: [CodeGen] Attach no-builtin attributes to function definitions with no Decl
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4e799ada5860: [CodeGen] Attach no-builtin attributes to
function definitions with no Decl (authored by thegameg).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73495/new/
https://reviews.llvm.org/D73495
Files:
clang/lib/CodeGen/CGCall.cpp
clang/test/CodeGenCXX/global-init.cpp
Index: clang/test/CodeGenCXX/global-init.cpp
===
--- clang/test/CodeGenCXX/global-init.cpp
+++ clang/test/CodeGenCXX/global-init.cpp
@@ -2,6 +2,8 @@
// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm %s -o - |FileCheck -check-prefix CHECK-NOEXC %s
// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -mframe-pointer=non-leaf %s -o - \
// RUN: | FileCheck -check-prefix CHECK-FP %s
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm %s -o - -fno-builtin \
+// RUN: | FileCheck -check-prefix CHECK-NOBUILTIN %s
struct A {
A();
@@ -202,9 +204,12 @@
// rdar://problem/8090834: this should be nounwind
// CHECK-NOEXC: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUW:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
-
// CHECK-NOEXC: attributes [[NUW]] = { noinline nounwind{{.*}} }
+// Make sure we mark global initializers with the no-builtins attribute.
+// CHECK-NOBUILTIN: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUW:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
+// CHECK-NOBUILTIN: attributes [[NUW]] = { noinline nounwind{{.*}}"no-builtins"{{.*}} }
+
// PR21811: attach the appropriate attribute to the global init function
// CHECK-FP: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUX:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
// CHECK-FP: attributes [[NUX]] = { noinline nounwind {{.*}}"frame-pointer"="non-leaf"{{.*}} }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1832,6 +1832,42 @@
F.addAttributes(llvm::AttributeList::FunctionIndex, FuncAttrs);
}
+static void addNoBuiltinAttributes(llvm::AttrBuilder ,
+ const LangOptions ,
+ const NoBuiltinAttr *NBA = nullptr) {
+ auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
+SmallString<32> AttributeName;
+AttributeName += "no-builtin-";
+AttributeName += BuiltinName;
+FuncAttrs.addAttribute(AttributeName);
+ };
+
+ // First, handle the language options passed through -fno-builtin[-]
+ if (LangOpts.NoBuiltin) {
+// -fno-builtin disables them all.
+FuncAttrs.addAttribute("no-builtins");
+return;
+ }
+
+ // Then, add attributes for builtins specified through -fno-builtin-.
+ llvm::for_each(LangOpts.NoBuiltinFuncs, AddNoBuiltinAttr);
+
+ // Now, let's check the __attribute__((no_builtin("...")) attribute added to
+ // the source.
+ if (!NBA)
+return;
+
+ // If there is a wildcard in the builtin names specified through the
+ // attribute, disable them all.
+ if (llvm::is_contained(NBA->builtinNames(), "*")) {
+FuncAttrs.addAttribute("no-builtins");
+return;
+ }
+
+ // And last, add the rest of the builtin names.
+ llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
+}
+
void CodeGenModule::ConstructAttributeList(
StringRef Name, const CGFunctionInfo , CGCalleeInfo CalleeInfo,
llvm::AttributeList , unsigned , bool AttrOnCallSite) {
@@ -1850,6 +1886,8 @@
const Decl *TargetDecl = CalleeInfo.getCalleeDecl().getDecl();
bool HasOptnone = false;
+ // The NoBuiltinAttr attached to a TargetDecl (only allowed on FunctionDecls).
+ const NoBuiltinAttr *NBA = nullptr;
// FIXME: handle sseregparm someday...
if (TargetDecl) {
if (TargetDecl->hasAttr())
@@ -1875,22 +1913,7 @@
if (!(AttrOnCallSite && IsVirtualCall)) {
if (Fn->isNoReturn())
FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
-
-const auto *NBA = Fn->getAttr();
-bool HasWildcard = NBA && llvm::is_contained(NBA->builtinNames(), "*");
-if (getLangOpts().NoBuiltin || HasWildcard)
- FuncAttrs.addAttribute("no-builtins");
-else {
- auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
-SmallString<32> AttributeName;
-AttributeName += "no-builtin-";
-AttributeName += BuiltinName;
-FuncAttrs.addAttribute(AttributeName);
- };
- llvm::for_each(getLangOpts().NoBuiltinFuncs, AddNoBuiltinAttr);
- if (NBA)
-llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
-}
+NBA = Fn->getAttr();
}
}
@@ -1925,6 +1948,14 @@
}
}
+ // Attach "no-builtins" attributes to:
+ // * call sites: both `nobuiltin` and "no-builtins" or
[PATCH] D73495: [CodeGen] Attach no-builtin attributes to function definitions with no Decl
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM There's maybe some argument that we should be calling getNonClosureContext() or something like that to find the parent function, at least for some attributes. But that seems less critical, and I don't really want to think about which attributes should/should not apply right now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73495/new/ https://reviews.llvm.org/D73495 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73495: [CodeGen] Attach no-builtin attributes to function definitions with no Decl
thegameg updated this revision to Diff 240914.
thegameg added a comment.
Add the attribute to all TargetDecls.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73495/new/
https://reviews.llvm.org/D73495
Files:
clang/lib/CodeGen/CGCall.cpp
clang/test/CodeGenCXX/global-init.cpp
Index: clang/test/CodeGenCXX/global-init.cpp
===
--- clang/test/CodeGenCXX/global-init.cpp
+++ clang/test/CodeGenCXX/global-init.cpp
@@ -2,6 +2,8 @@
// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm %s -o - |FileCheck -check-prefix CHECK-NOEXC %s
// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -mframe-pointer=non-leaf %s -o - \
// RUN: | FileCheck -check-prefix CHECK-FP %s
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm %s -o - -fno-builtin \
+// RUN: | FileCheck -check-prefix CHECK-NOBUILTIN %s
struct A {
A();
@@ -202,9 +204,12 @@
// rdar://problem/8090834: this should be nounwind
// CHECK-NOEXC: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUW:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
-
// CHECK-NOEXC: attributes [[NUW]] = { noinline nounwind{{.*}} }
+// Make sure we mark global initializers with the no-builtins attribute.
+// CHECK-NOBUILTIN: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUW:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
+// CHECK-NOBUILTIN: attributes [[NUW]] = { noinline nounwind{{.*}}"no-builtins"{{.*}} }
+
// PR21811: attach the appropriate attribute to the global init function
// CHECK-FP: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUX:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
// CHECK-FP: attributes [[NUX]] = { noinline nounwind {{.*}}"frame-pointer"="non-leaf"{{.*}} }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1832,6 +1832,42 @@
F.addAttributes(llvm::AttributeList::FunctionIndex, FuncAttrs);
}
+static void addNoBuiltinAttributes(llvm::AttrBuilder ,
+ const LangOptions ,
+ const NoBuiltinAttr *NBA = nullptr) {
+ auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
+SmallString<32> AttributeName;
+AttributeName += "no-builtin-";
+AttributeName += BuiltinName;
+FuncAttrs.addAttribute(AttributeName);
+ };
+
+ // First, handle the language options passed through -fno-builtin[-]
+ if (LangOpts.NoBuiltin) {
+// -fno-builtin disables them all.
+FuncAttrs.addAttribute("no-builtins");
+return;
+ }
+
+ // Then, add attributes for builtins specified through -fno-builtin-.
+ llvm::for_each(LangOpts.NoBuiltinFuncs, AddNoBuiltinAttr);
+
+ // Now, let's check the __attribute__((no_builtin("...")) attribute added to
+ // the source.
+ if (!NBA)
+return;
+
+ // If there is a wildcard in the builtin names specified through the
+ // attribute, disable them all.
+ if (llvm::is_contained(NBA->builtinNames(), "*")) {
+FuncAttrs.addAttribute("no-builtins");
+return;
+ }
+
+ // And last, add the rest of the builtin names.
+ llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
+}
+
void CodeGenModule::ConstructAttributeList(
StringRef Name, const CGFunctionInfo , CGCalleeInfo CalleeInfo,
llvm::AttributeList , unsigned , bool AttrOnCallSite) {
@@ -1850,6 +1886,8 @@
const Decl *TargetDecl = CalleeInfo.getCalleeDecl().getDecl();
bool HasOptnone = false;
+ // The NoBuiltinAttr attached to a TargetDecl (only allowed on FunctionDecls).
+ const NoBuiltinAttr *NBA = nullptr;
// FIXME: handle sseregparm someday...
if (TargetDecl) {
if (TargetDecl->hasAttr())
@@ -1875,22 +1913,7 @@
if (!(AttrOnCallSite && IsVirtualCall)) {
if (Fn->isNoReturn())
FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
-
-const auto *NBA = Fn->getAttr();
-bool HasWildcard = NBA && llvm::is_contained(NBA->builtinNames(), "*");
-if (getLangOpts().NoBuiltin || HasWildcard)
- FuncAttrs.addAttribute("no-builtins");
-else {
- auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
-SmallString<32> AttributeName;
-AttributeName += "no-builtin-";
-AttributeName += BuiltinName;
-FuncAttrs.addAttribute(AttributeName);
- };
- llvm::for_each(getLangOpts().NoBuiltinFuncs, AddNoBuiltinAttr);
- if (NBA)
-llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
-}
+NBA = Fn->getAttr();
}
}
@@ -1925,6 +1948,14 @@
}
}
+ // Attach "no-builtins" attributes to:
+ // * call sites: both `nobuiltin` and "no-builtins" or "no-builtin-".
+ // * definitions: "no-builtins" or "no-builtin-" only.
+ // The attributes can come from:
+ // * LangOpts: -ffreestanding,
[PATCH] D73495: [CodeGen] Attach no-builtin attributes to function definitions with no Decl
thegameg marked 2 inline comments as done.
thegameg added inline comments.
Comment at: clang/lib/CodeGen/CGCall.cpp:1917
const auto *NBA = Fn->getAttr();
-bool HasWildcard = NBA && llvm::is_contained(NBA->builtinNames(), "*");
-if (getLangOpts().NoBuiltin || HasWildcard)
- FuncAttrs.addAttribute("no-builtins");
-else {
- auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
-SmallString<32> AttributeName;
-AttributeName += "no-builtin-";
-AttributeName += BuiltinName;
-FuncAttrs.addAttribute(AttributeName);
- };
- llvm::for_each(getLangOpts().NoBuiltinFuncs, AddNoBuiltinAttr);
- if (NBA)
-llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
-}
+addNonCallSiteNoBuiltinAttributes(FuncAttrs, getLangOpts(), NBA);
}
efriedma wrote:
> thegameg wrote:
> > efriedma wrote:
> > > What happens if we have a TargetDecl that isn't a FunctionDecl? (I think
> > > this happens in some cases, but not completely sure which, exactly.)
> > It looks like that can be triggered by indirect calls:
> >
> > ```
> > typedef void T(void);
> > void test3(T f) {
> > f();
> > }
> > ```
> >
> > Since this adds the attribute to definitions and not to call sites, we
> > should never need that.
> >
> > This patch is for the case where `CreateGlobalInitOrDestructFunction` ends
> > up re-using the same function to attach the attributes.
> >
> > I'll update the description to make it more clear.
> Are you sure that's the only case where the TargetDecl isn't a FunctionDecl?
> I'm afraid there might be some weird case where the TargetDecl defines
> something like a function, but isn't technically a FunctionDecl. Maybe an
> ObjC method or something like that. And then we end up in a situation where
> addNonCallSiteNoBuiltinAttributes is never called.
Right, it looks like in the test suite this triggers on:
* BlockDecl
* ObjCMethodDecl
* CapturedDecl
I think we should call `addNonCallSiteNoBuiltinAttributes` for these too.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73495/new/
https://reviews.llvm.org/D73495
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73495: [CodeGen] Attach no-builtin attributes to function definitions with no Decl
efriedma added inline comments.
Comment at: clang/lib/CodeGen/CGCall.cpp:1917
const auto *NBA = Fn->getAttr();
-bool HasWildcard = NBA && llvm::is_contained(NBA->builtinNames(), "*");
-if (getLangOpts().NoBuiltin || HasWildcard)
- FuncAttrs.addAttribute("no-builtins");
-else {
- auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
-SmallString<32> AttributeName;
-AttributeName += "no-builtin-";
-AttributeName += BuiltinName;
-FuncAttrs.addAttribute(AttributeName);
- };
- llvm::for_each(getLangOpts().NoBuiltinFuncs, AddNoBuiltinAttr);
- if (NBA)
-llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
-}
+addNonCallSiteNoBuiltinAttributes(FuncAttrs, getLangOpts(), NBA);
}
thegameg wrote:
> efriedma wrote:
> > What happens if we have a TargetDecl that isn't a FunctionDecl? (I think
> > this happens in some cases, but not completely sure which, exactly.)
> It looks like that can be triggered by indirect calls:
>
> ```
> typedef void T(void);
> void test3(T f) {
> f();
> }
> ```
>
> Since this adds the attribute to definitions and not to call sites, we should
> never need that.
>
> This patch is for the case where `CreateGlobalInitOrDestructFunction` ends up
> re-using the same function to attach the attributes.
>
> I'll update the description to make it more clear.
Are you sure that's the only case where the TargetDecl isn't a FunctionDecl?
I'm afraid there might be some weird case where the TargetDecl defines
something like a function, but isn't technically a FunctionDecl. Maybe an ObjC
method or something like that. And then we end up in a situation where
addNonCallSiteNoBuiltinAttributes is never called.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73495/new/
https://reviews.llvm.org/D73495
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
