[PATCH] Adding support for NoDuplicate function attribute in CLang
Richard Smith
richard at metafoo.co.uk
Wed Nov 20 11:51:41 PST 2013
Please include a documentation update for this attribute.
On Wed, Nov 20, 2013 at 5:22 AM, Aaron Ballman <aaron at aaronballman.com>wrote:
> On Fri, Nov 15, 2013 at 5:49 AM, Marcello Maggioni
> <marcello at codeplay.com> wrote:
> > Hi all,
> >
> > I made a patch that adds the capability of defining the NoDuplicate
> > attribute for a function using both the GNU and CXX11 attribute syntaxes
> > like in:
> >
> > __attribute__((noduplicate))
> > [[noduplicate]]
> >
> > I also added tests to test out the new addition.
> > The NoDuplicate attribute is useful to avoid the application of compiler
> > optimizations like Loop Unswitching to code containing function calls
> that
> > implement functionality like barriers in OpenCL. These functions could
> fail
> > if such an optimization is applied on the code that calls them.
>
> How is this different from the optnone attribute that was recently
> added? The attribute name here doesn't really convey much meaning
> based on this description, so perhaps there is a better name for it?
>
> >
> > I splitted the patch in two files. The "_code" file contains the part
> that
> > adds the functionality to Clang and the "_tests" part contains the added
> > tests.
> >
> > Tell me if it is interesting to add this to mainline. If there is a need
> for
> > corrections I'll be happy to make them.
> > diff --git a/test/CodeGen/noduplicate-cxx11-test.cpp
> b/test/CodeGen/noduplicate-cxx11-test.cpp
>
> This file should be in CodeGenCXX
>
> > new file mode 100644
> > index 0000000..09e3801
> > --- /dev/null
> > +++ b/test/CodeGen/noduplicate-cxx11-test.cpp
> > @@ -0,0 +1,20 @@
> > +// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - | FileCheck %s
> > +
> > +// This was a problem in Sema, but only shows up as noinline missing
> > +// in CodeGen.
>
> What was a problem in sema?
>
> > +
> > +// CHECK: define i32 @_Z15noduplicatedfuni(i32 %a) [[NI:#[0-9]+]]
> > +
> > +int noduplicatedfun [[noduplicate]] (int a) {
> > +
> > + return a+1;
> > +
> > +}
> > +
> > +int main() {
> > +
> > + return noduplicatedfun(5);
> > +
> > +}
> > +
> > +// CHECK: attributes [[NI]] = { noduplicate nounwind{{.*}} }
> > diff --git a/test/CodeGen/noduplicate-test.cpp
> b/test/CodeGen/noduplicate-test.cpp
>
> This file should be a .c file
>
> > new file mode 100644
> > index 0000000..86413a8
> > --- /dev/null
> > +++ b/test/CodeGen/noduplicate-test.cpp
> > @@ -0,0 +1,20 @@
> > +// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
> > +
> > +// This was a problem in Sema, but only shows up as noinline missing
> > +// in CodeGen.
> > +
> > +// CHECK: define i32 @_Z15noduplicatedfuni(i32 %a) [[NI:#[0-9]+]]
> > +
> > +__attribute__((noduplicate)) int noduplicatedfun(int a) {
> > +
> > + return a+1;
> > +
> > +}
> > +
> > +int main() {
> > +
> > + return noduplicatedfun(5);
> > +
> > +}
> > +
> > +// CHECK: attributes [[NI]] = { noduplicate nounwind{{.*}} }
> > diff --git a/test/Sema/attr-noduplicate.c b/test/Sema/attr-noduplicate.c
> > new file mode 100644
> > index 0000000..2a77de5
> > --- /dev/null
> > +++ b/test/Sema/attr-noduplicate.c
> > @@ -0,0 +1,8 @@
> > +// RUN: %clang_cc1 %s -verify -fsyntax-only
> > +
> > +int a __attribute__((noduplicate)); // expected-warning {{'noduplicate'
> attribute only applies to functions}}
> > +
> > +void t1() __attribute__((noduplicate));
> > +
> > +void t2() __attribute__((noduplicate(2))); // expected-error
> {{'noduplicate' attribute takes no arguments}}
> > +
> > diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
> > index a149a6a..5da256b 100644
> > --- a/include/clang/Basic/Attr.td
> > +++ b/include/clang/Basic/Attr.td
> > @@ -498,6 +498,10 @@ def NoDebug : InheritableAttr {
> > let Spellings = [GNU<"nodebug">];
> > }
> >
> > +def NoDuplicate : InheritableAttr {
> > + let Spellings = [GNU<"noduplicate">, CXX11<"", "noduplicate">];
>
> Since this isn't a C++11 attribute, this should be added to the clang
> namespace in that case. Also, there is no Subjects clause.
>
> > +}
> > +
> > def NoInline : InheritableAttr {
> > let Spellings = [GNU<"noinline">, CXX11<"gnu", "noinline">];
> > }
> > diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp
> > index 22f2467..4fc2ea4 100644
> > --- a/lib/CodeGen/CGCall.cpp
> > +++ b/lib/CodeGen/CGCall.cpp
> > @@ -1006,6 +1006,8 @@ void CodeGenModule::ConstructAttributeList(const
> CGFunctionInfo &FI,
> > FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
> > if (TargetDecl->hasAttr<NoReturnAttr>())
> > FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
> > + if (TargetDecl->hasAttr<NoDuplicateAttr>())
> > + FuncAttrs.addAttribute(llvm::Attribute::NoDuplicate);
> >
> > if (const FunctionDecl *Fn = dyn_cast<FunctionDecl>(TargetDecl)) {
> > const FunctionProtoType *FPT =
> Fn->getType()->getAs<FunctionProtoType>();
> > diff --git a/lib/CodeGen/CodeGenModule.cpp
> b/lib/CodeGen/CodeGenModule.cpp
> > index da54de0..89aa8fb 100644
> > --- a/lib/CodeGen/CodeGenModule.cpp
> > +++ b/lib/CodeGen/CodeGenModule.cpp
> > @@ -678,6 +678,8 @@ void
> CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,
> > // Naked implies noinline: we should not be inlining such functions.
> > B.addAttribute(llvm::Attribute::Naked);
> > B.addAttribute(llvm::Attribute::NoInline);
> > + } else if (D->hasAttr<NoDuplicateAttr>()) {
> > + B.addAttribute(llvm::Attribute::NoDuplicate);
>
Why does noduplicate override noinline? Does noduplicate imply noinline?
> > } else if (D->hasAttr<NoInlineAttr>()) {
> > B.addAttribute(llvm::Attribute::NoInline);
> > } else if ((D->hasAttr<AlwaysInlineAttr>() ||
> > diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
> > index 30ddd4d..016776f 100644
> > --- a/lib/Sema/SemaDeclAttr.cpp
> > +++ b/lib/Sema/SemaDeclAttr.cpp
> > @@ -3628,6 +3628,18 @@ static void handleNoDebugAttr(Sema &S, Decl *D,
> const AttributeList &Attr) {
> > Attr.getAttributeSpellingListIndex()));
> > }
> >
> > +static void handleNoDuplicateAttr(Sema &S, Decl *D, const AttributeList
> &Attr) {
> > + if (!isa<FunctionDecl>(D)) {
> > + S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
> > + << Attr.getName() << ExpectedFunction;
>
> Should this also apply to ObjC methods?
>
> > + return;
> > + }
> > +
> > + D->addAttr(::new (S.Context)
> > + NoDuplicateAttr(Attr.getRange(), S.Context,
> > + Attr.getAttributeSpellingListIndex()));
>
Should this attribute conflict with always_inline / __forceinline?
> > +}
> > +
> > static void handleNoInlineAttr(Sema &S, Decl *D, const AttributeList
> &Attr) {
> > if (!isa<FunctionDecl>(D)) {
> > S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
> > @@ -4804,6 +4816,7 @@ static void ProcessDeclAttribute(Sema &S, Scope
> *scope, Decl *D,
> > case AttributeList::AT_Pure: handlePureAttr (S, D,
> Attr); break;
> > case AttributeList::AT_Cleanup: handleCleanupAttr (S, D,
> Attr); break;
> > case AttributeList::AT_NoDebug: handleNoDebugAttr (S, D,
> Attr); break;
> > + case AttributeList::AT_NoDuplicate: handleNoDuplicateAttr (S, D,
> Attr); break;
> > case AttributeList::AT_NoInline: handleNoInlineAttr (S, D,
> Attr); break;
> > case AttributeList::AT_Regparm: handleRegparmAttr (S, D,
> Attr); break;
> > case AttributeList::IgnoredAttribute:
> >
>
>
> On Sat, Nov 16, 2013 at 10:41 AM, Marcello Maggioni
> <marcello at codeplay.com> wrote:
> > New patch that merges the two separate patches together and also adds the
> > NoDuplicate attribute to the ConstructAttributeList functions in
> CGCall.cpp
> >
> > Cheers,
> > Marcello
> >
> >
> > On 15/11/13 10:49, Marcello Maggioni wrote:
> >
> > Hi all,
> >
> > I made a patch that adds the capability of defining the NoDuplicate
> > attribute for a function using both the GNU and CXX11 attribute syntaxes
> > like in:
> >
> > __attribute__((noduplicate))
> > [[noduplicate]]
> >
> > I also added tests to test out the new addition.
> > The NoDuplicate attribute is useful to avoid the application of compiler
> > optimizations like Loop Unswitching to code containing function calls
> that
> > implement functionality like barriers in OpenCL. These functions could
> fail
> > if such an optimization is applied on the code that calls them.
> >
> > I splitted the patch in two files. The "_code" file contains the part
> that
> > adds the functionality to Clang and the "_tests" part contains the added
> > tests.
> >
> > Tell me if it is interesting to add this to mainline. If there is a need
> for
> > corrections I'll be happy to make them.
> >
> > Cheers,
> > Marcello
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
>
> ~Aaron
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131120/2b9ca3a8/attachment.html>
More information about the cfe-commits
mailing list