<div dir="ltr"><div>Please include a documentation update for this attribute.</div><div><br></div>On Wed, Nov 20, 2013 at 5:22 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Fri, Nov 15, 2013 at 5:49 AM, Marcello Maggioni<br>

<<a href="mailto:marcello@codeplay.com">marcello@codeplay.com</a>> wrote:<br>
> Hi all,<br>
><br>
> I made a patch that adds the capability of defining the NoDuplicate<br>
> attribute for a function using both the GNU and CXX11 attribute syntaxes<br>
> like in:<br>
><br>
> __attribute__((noduplicate))<br>
> [[noduplicate]]<br>
><br>
> I also added tests to test out the new addition.<br>
> The NoDuplicate attribute is useful to avoid the application of compiler<br>
> optimizations like Loop Unswitching to code containing function calls that<br>
> implement functionality like barriers in OpenCL. These functions could fail<br>
> if such an optimization is applied on the code that calls them.<br>
<br>
</div>How is this different from the optnone attribute that was recently<br>
added? The attribute name here doesn't really convey much meaning<br>
based on this description, so perhaps there is a better name for it?<br>
<div class="im"><br>
><br>
> I splitted the patch in two files. The "_code" file contains the part that<br>
> adds the functionality to Clang and the "_tests" part contains the added<br>
> tests.<br>
><br>
> Tell me if it is interesting to add this to mainline. If there is a need for<br>
> corrections I'll be happy to make them.<br>
</div>> diff --git a/test/CodeGen/noduplicate-cxx11-test.cpp b/test/CodeGen/noduplicate-cxx11-test.cpp<br>
<br>
This file should be in CodeGenCXX<br>
<br>
> new file mode 100644<br>
> index 0000000..09e3801<br>
> --- /dev/null<br>
> +++ b/test/CodeGen/noduplicate-cxx11-test.cpp<br>
> @@ -0,0 +1,20 @@<br>
> +// RUN: %clang_cc1 -std=c++11 %s  -emit-llvm -o - | FileCheck %s<br>
> +<br>
> +// This was a problem in Sema, but only shows up as noinline missing<br>
> +// in CodeGen.<br>
<br>
What was a problem in sema?<br>
<br>
> +<br>
> +// CHECK: define i32 @_Z15noduplicatedfuni(i32 %a) [[NI:#[0-9]+]]<br>
> +<br>
> +int noduplicatedfun [[noduplicate]] (int a) {<br>
> +<br>
> +  return a+1;<br>
> +<br>
> +}<br>
> +<br>
> +int main() {<br>
> +<br>
> +  return noduplicatedfun(5);<br>
> +<br>
> +}<br>
> +<br>
> +// CHECK: attributes [[NI]] = { noduplicate nounwind{{.*}} }<br>
> diff --git a/test/CodeGen/noduplicate-test.cpp b/test/CodeGen/noduplicate-test.cpp<br>
<br>
This file should be a .c file<br>
<br>
> new file mode 100644<br>
> index 0000000..86413a8<br>
> --- /dev/null<br>
> +++ b/test/CodeGen/noduplicate-test.cpp<br>
> @@ -0,0 +1,20 @@<br>
> +// RUN: %clang_cc1 %s  -emit-llvm -o - | FileCheck %s<br>
> +<br>
> +// This was a problem in Sema, but only shows up as noinline missing<br>
> +// in CodeGen.<br>
> +<br>
> +// CHECK: define i32 @_Z15noduplicatedfuni(i32 %a) [[NI:#[0-9]+]]<br>
> +<br>
> +__attribute__((noduplicate)) int noduplicatedfun(int a) {<br>
> +<br>
> +  return a+1;<br>
> +<br>
> +}<br>
> +<br>
> +int main() {<br>
> +<br>
> +  return noduplicatedfun(5);<br>
> +<br>
> +}<br>
> +<br>
> +// CHECK: attributes [[NI]] = { noduplicate nounwind{{.*}} }<br>
> diff --git a/test/Sema/attr-noduplicate.c b/test/Sema/attr-noduplicate.c<br>
> new file mode 100644<br>
> index 0000000..2a77de5<br>
> --- /dev/null<br>
> +++ b/test/Sema/attr-noduplicate.c<br>
> @@ -0,0 +1,8 @@<br>
> +// RUN: %clang_cc1 %s -verify -fsyntax-only<br>
> +<br>
> +int a __attribute__((noduplicate)); // expected-warning {{'noduplicate' attribute only applies to functions}}<br>
> +<br>
> +void t1() __attribute__((noduplicate));<br>
> +<br>
> +void t2() __attribute__((noduplicate(2))); // expected-error {{'noduplicate' attribute takes no arguments}}<br>
> +<br>
> diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td<br>
> index a149a6a..5da256b 100644<br>
> --- a/include/clang/Basic/Attr.td<br>
> +++ b/include/clang/Basic/Attr.td<br>
> @@ -498,6 +498,10 @@ def NoDebug : InheritableAttr {<br>
>    let Spellings = [GNU<"nodebug">];<br>
>  }<br>
><br>
> +def NoDuplicate : InheritableAttr {<br>
> +  let Spellings = [GNU<"noduplicate">, CXX11<"", "noduplicate">];<br>
<br>
Since this isn't a C++11 attribute, this should be added to the clang<br>
namespace in that case. Also, there is no Subjects clause.<br>
<br>
> +}<br>
> +<br>
>  def NoInline : InheritableAttr {<br>
>    let Spellings = [GNU<"noinline">, CXX11<"gnu", "noinline">];<br>
>  }<br>
> diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp<br>
> index 22f2467..4fc2ea4 100644<br>
> --- a/lib/CodeGen/CGCall.cpp<br>
> +++ b/lib/CodeGen/CGCall.cpp<br>
> @@ -1006,6 +1006,8 @@ void CodeGenModule::ConstructAttributeList(const CGFunctionInfo &FI,<br>
>        FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);<br>
>      if (TargetDecl->hasAttr<NoReturnAttr>())<br>
>        FuncAttrs.addAttribute(llvm::Attribute::NoReturn);<br>
> +    if (TargetDecl->hasAttr<NoDuplicateAttr>())<br>
> +      FuncAttrs.addAttribute(llvm::Attribute::NoDuplicate);<br>
><br>
>      if (const FunctionDecl *Fn = dyn_cast<FunctionDecl>(TargetDecl)) {<br>
>        const FunctionProtoType *FPT = Fn->getType()->getAs<FunctionProtoType>();<br>
> diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp<br>
> index da54de0..89aa8fb 100644<br>
> --- a/lib/CodeGen/CodeGenModule.cpp<br>
> +++ b/lib/CodeGen/CodeGenModule.cpp<br>
> @@ -678,6 +678,8 @@ void CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,<br>
>      // Naked implies noinline: we should not be inlining such functions.<br>
>      B.addAttribute(llvm::Attribute::Naked);<br>
>      B.addAttribute(llvm::Attribute::NoInline);<br>
> +  } else if (D->hasAttr<NoDuplicateAttr>()) {<br>
> +    B.addAttribute(llvm::Attribute::NoDuplicate);<br></blockquote><div><br></div><div>Why does noduplicate override noinline? Does noduplicate imply noinline?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

>    } else if (D->hasAttr<NoInlineAttr>()) {<br>
>      B.addAttribute(llvm::Attribute::NoInline);<br>
>    } else if ((D->hasAttr<AlwaysInlineAttr>() ||<br>
> diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp<br>
> index 30ddd4d..016776f 100644<br>
> --- a/lib/Sema/SemaDeclAttr.cpp<br>
> +++ b/lib/Sema/SemaDeclAttr.cpp<br>
> @@ -3628,6 +3628,18 @@ static void handleNoDebugAttr(Sema &S, Decl *D, const AttributeList &Attr) {<br>
>                           Attr.getAttributeSpellingListIndex()));<br>
>  }<br>
><br>
> +static void handleNoDuplicateAttr(Sema &S, Decl *D, const AttributeList &Attr) {<br>
> +  if (!isa<FunctionDecl>(D)) {<br>
> +    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)<br>
> +      << Attr.getName() << ExpectedFunction;<br>
<br>
Should this also apply to ObjC methods?<br>
<br>
> +    return;<br>
> +  }<br>
> +<br>
> +  D->addAttr(::new (S.Context)<br>
> +             NoDuplicateAttr(Attr.getRange(), S.Context,<br>
> +             Attr.getAttributeSpellingListIndex()));<br></blockquote><div><br></div><div>Should this attribute conflict with always_inline / __forceinline?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

> +}<br>
> +<br>
>  static void handleNoInlineAttr(Sema &S, Decl *D, const AttributeList &Attr) {<br>
>    if (!isa<FunctionDecl>(D)) {<br>
>      S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)<br>
> @@ -4804,6 +4816,7 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,<br>
>    case AttributeList::AT_Pure:        handlePureAttr        (S, D, Attr); break;<br>
>    case AttributeList::AT_Cleanup:     handleCleanupAttr     (S, D, Attr); break;<br>
>    case AttributeList::AT_NoDebug:     handleNoDebugAttr     (S, D, Attr); break;<br>
> +  case AttributeList::AT_NoDuplicate: handleNoDuplicateAttr (S, D, Attr); break;<br>
>    case AttributeList::AT_NoInline:    handleNoInlineAttr    (S, D, Attr); break;<br>
>    case AttributeList::AT_Regparm:     handleRegparmAttr     (S, D, Attr); break;<br>
>    case AttributeList::IgnoredAttribute:<br>
<div class="im HOEnZb">><br>
<br>
<br>
On Sat, Nov 16, 2013 at 10:41 AM, Marcello Maggioni<br>
<<a href="mailto:marcello@codeplay.com">marcello@codeplay.com</a>> wrote:<br>
> New patch that merges the two separate patches together and also adds the<br>
> NoDuplicate attribute to the ConstructAttributeList functions in CGCall.cpp<br>
><br>
> Cheers,<br>
> Marcello<br>
><br>
><br>
> On 15/11/13 10:49, Marcello Maggioni wrote:<br>
><br>
</div><div class="im HOEnZb">> Hi all,<br>
><br>
> I made a patch that adds the capability of defining the NoDuplicate<br>
> attribute for a function using both the GNU and CXX11 attribute syntaxes<br>
> like in:<br>
><br>
> __attribute__((noduplicate))<br>
> [[noduplicate]]<br>
><br>
> I also added tests to test out the new addition.<br>
> The NoDuplicate attribute is useful to avoid the application of compiler<br>
> optimizations like Loop Unswitching to code containing function calls that<br>
> implement functionality like barriers in OpenCL. These functions could fail<br>
> if such an optimization is applied on the code that calls them.<br>
><br>
> I splitted the patch in two files. The "_code" file contains the part that<br>
> adds the functionality to Clang and the "_tests" part contains the added<br>
> tests.<br>
><br>
> Tell me if it is interesting to add this to mainline. If there is a need for<br>
> corrections I'll be happy to make them.<br>
><br>
> Cheers,<br>
> Marcello<br>
><br>
><br>
><br>
</div><div class="im HOEnZb">> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
<br>
</div><span class="HOEnZb"><font color="#888888">~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5">_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>