[PATCH] Adding support for NoDuplicate function attribute in CLang
Aaron Ballman
aaron at aaronballman.com
Wed Nov 20 06:54:54 PST 2013
On Wed, Nov 20, 2013 at 9:13 AM, Marcello Maggioni
<marcello at codeplay.com> wrote:
> On 20/11/13 13:22, Aaron Ballman 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?
>
>
> The noduplicate attribute is an attribute in LLVM that stops optimizations
> that duplicate a function call, like Loop Unswitching for example. OptNone
> and NoDuplicate are very different. OptNone specifies that the function must
> not be optimized, while NoDuplicate specifies that function calls to the
> function must not be duplicated by optimizations (optimization inside the
> function itself is not concerned by noduplicate at all)
Thank you for the explanation -- is attribute specific to OpenCL, or
are there use cases outside of that? Is this attribute supported by
any other compilers, or just clang?
Your explanation makes me wonder -- what should happen in this case?
void foo(void) __attribute__((noduplicate));
typedef void (*fp)(void);
fp f = foo;
f();
Should the calls to fp be noduplicate as well? If so, then perhaps the
attribute be applicable to function-like things (typedefs, fields,
etc)?
>
>>> 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?
>
>
> Hmmm, I took this test and readapted for my case from a similar "noinline"
> test and forgot to remove the comments inside ... I'll address that and move
> it to the right directory
>
>>> +
>>> +// 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
>
> Make sense!
>
>>> 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.
>
>
> Ok!
Might want to hold off on the namespace for a moment. It definitely
doesn't belong in the global namespace, but it may belong in clang, or
gnu, or perhaps even an opencl-specific namespace. This is why I am
asking the questions I'm asking. :-)
>
>>> +}
>>> +
>>> 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?
>
>
> I haven't specifically thought about use cases in ObjC for this, but I guess
> it could be applicable to ObjC also.
If it's OpenCL-specific functionality, then perhaps ObjC methods don't
make sense.
>
>>> + 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
>
>
> Marcello
>
> --
> Marcello Maggioni
>
> Compiler Engineer
>
>
> Codeplay Software Ltd
>
> 45 York Place, Edinburgh, EH1 3HP
>
> Tel: 0131 466 0503
>
> Fax: 0131 557 6600
>
> Website: http://www.codeplay.com
>
> Twitter: https://twitter.com/@codeplaybiz
>
>
> This email and any attachments may contain confidential and /or privileged
> information and is for use by the addressee only. If you are not the
> intended recipient, please notify Codeplay Software Ltd immediately and
> delete the message from your computer. You may not copy or forward it,or use
> or disclose its contents to any other person. Any views or other information
> in this message which do not relate to our business are not authorized by
> Codeplay software Ltd, nor does this message form part of any contract
> unless so stated.
>
> As internet communications are capable of data corruption Codeplay Software
> Ltd does not accept any responsibility for any changes made to this message
> after it was sent. Please note that Codeplay Software Ltd does not accept
> any liability or responsibility for viruses and it is your responsibility to
> scan any attachments.
>
> Company registered in England and Wales, number: 04567874
>
> Registered office: 81 Linkfield Street, Redhill RH1 6BY
>
~Aaron
More information about the cfe-commits
mailing list