[PATCH] Adding support for NoDuplicate function attribute in CLang

Aaron Ballman aaron at aaronballman.com
Wed Nov 20 05:22:49 PST 2013


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);
>    } 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()));
> +}
> +
>  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



More information about the cfe-commits mailing list