[PATCH] Adding support for NoDuplicate function attribute in CLang
Marcello Maggioni
marcello at codeplay.com
Wed Nov 20 09:16:04 PST 2013
Here is a patch with the modification Aaron suggested
Cheers,
Marcello
On 20/11/13 14:13, Marcello Maggioni 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)
>>> 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!
>>> +}
>>> +
>>> 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.
>>> + 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang_noduplicatev2.patch
Type: text/x-patch
Size: 4937 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131120/571a0021/attachment.bin>
More information about the cfe-commits
mailing list