[PATCH] Adding support for NoDuplicate function attribute in CLang
Marcello Maggioni
marcello at codeplay.com
Wed Nov 20 11:25:13 PST 2013
Sorry, I missed the mail.
Hmm, well, ideally if the call through the pointer is duplicated ,and
the function call shouldn't be duplicated (like in some implementations
of "barrier()" in OpenCL) the result might be wrong (in the case of a
barrier the result is a dead-lock).
Of course, OpenCL itself doesn't have this problem because it doesn't
have function pointers ...
So yeah, maybe it makes sense to also have the attribute applicable to
function-types in C, so that you can define noduplicate
function-pointers and being able to make safe calls to functions from
pointers.
Cheers,
Marcello
On 20/11/13 14:54, Aaron Ballman wrote:
> 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
--
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
More information about the cfe-commits
mailing list