<div dir="ltr">Please also update the Clang documentation to cover this attribute.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Nov 20, 2013 at 5:29 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Patch LGTM! Thanks!<br>
<br>
~Aaron<br>
<br>
On Tue, Nov 19, 2013 at 8:56 PM, Robinson, Paul<br>
<div class="HOEnZb"><div class="h5"><<a href="mailto:Paul_Robinson@playstation.sony.com">Paul_Robinson@playstation.sony.com</a>> wrote:<br>
> Revised patch with requested changes to allow 'optnone' on an<br>
> ObjC method.<br>
> --paulr<br>
><br>
>> -----Original Message-----<br>
>> From: <a href="mailto:aaron.ballman@gmail.com">aaron.ballman@gmail.com</a> [mailto:<a href="mailto:aaron.ballman@gmail.com">aaron.ballman@gmail.com</a>] On Behalf<br>
>> Of Aaron Ballman<br>
>> Sent: Tuesday, November 19, 2013 5:11 PM<br>
>> To: Robinson, Paul<br>
>> Cc: <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a> cfe<br>
>> Subject: Re: [PATCH] Add C/C++ attribute 'optnone'<br>
>><br>
>> On Tue, Nov 19, 2013 at 6:33 PM, Robinson, Paul<br>
>> <<a href="mailto:Paul_Robinson@playstation.sony.com">Paul_Robinson@playstation.sony.com</a>> wrote:<br>
>> >> -----Original Message-----<br>
>> >> From: <a href="mailto:aaron.ballman@gmail.com">aaron.ballman@gmail.com</a> [mailto:<a href="mailto:aaron.ballman@gmail.com">aaron.ballman@gmail.com</a>] On<br>
>> Behalf<br>
>> >> Of Aaron Ballman<br>
>> >> Sent: Tuesday, November 19, 2013 2:36 PM<br>
>> >> To: Robinson, Paul<br>
>> >> Cc: <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a> cfe<br>
>> >> Subject: Re: [PATCH] Add C/C++ attribute 'optnone'<br>
>> >><br>
>> >> > Index: include/clang/Basic/Attr.td<br>
>> >> > ===================================================================<br>
>> >> > --- include/clang/Basic/Attr.td (revision 195153)<br>
>> >> > +++ include/clang/Basic/Attr.td (working copy)<br>
>> >> > @@ -613,6 +613,11 @@<br>
>> >> > let Subjects = [ObjCInterface];<br>
>> >> > }<br>
>> >> ><br>
>> >> > +def OptimizeNone : InheritableAttr {<br>
>> >> > + let Spellings = [GNU<"optnone">, CXX11<"clang", "optnone">];<br>
>> >> > + let Subjects = [Function];<br>
>> >><br>
>> >> Can it apply to ObjC method declarations as well, or just functions?<br>
>> ><br>
>> > Um. Hadn't thought about it as ObjC isn't my use case. It can apply<br>
>> > to C++ methods, so I guess it could? What do I do to make that work?<br>
>><br>
>> In Attr.td, add ObjCMethod to the list of subjects, and in<br>
>> SemaDeclAttr.cpp, check for ObjCMethodDecl alongside of the existing<br>
>> FunctionDecl (and switch to using ExpectedFunctionOrMethod). I would<br>
>> guess this attribute does apply to ObjC methods, so this strikes me as<br>
>> a reasonable extension.<br>
>> ><br>
>> >><br>
>> >> > +}<br>
>> >> > +<br>
>> >> > def Overloadable : Attr {<br>
>> >> > let Spellings = [GNU<"overloadable">];<br>
>> >> > }<br>
>> >> > Index: lib/CodeGen/CodeGenModule.cpp<br>
>> >> > ===================================================================<br>
>> >> > --- lib/CodeGen/CodeGenModule.cpp (revision 195153)<br>
>> >> > +++ lib/CodeGen/CodeGenModule.cpp (working copy)<br>
>> >> > @@ -678,6 +678,10 @@<br>
>> >> > // Naked implies noinline: we should not be inlining such<br>
>> >> functions.<br>
>> >> > B.addAttribute(llvm::Attribute::Naked);<br>
>> >> > B.addAttribute(llvm::Attribute::NoInline);<br>
>> >> > + } else if (D->hasAttr<OptimizeNoneAttr>()) {<br>
>> >> > + // OptimizeNone implies noinline; we should not be inlining<br>
>> such<br>
>> >> functions.<br>
>> >> > + B.addAttribute(llvm::Attribute::OptimizeNone);<br>
>> >> > + B.addAttribute(llvm::Attribute::NoInline);<br>
>> >> > } else if (D->hasAttr<NoInlineAttr>()) {<br>
>> >> > B.addAttribute(llvm::Attribute::NoInline);<br>
>> >> > } else if ((D->hasAttr<AlwaysInlineAttr>() ||<br>
>> >> > @@ -696,6 +700,12 @@<br>
>> >> > if (D->hasAttr<MinSizeAttr>())<br>
>> >> > B.addAttribute(llvm::Attribute::MinSize);<br>
>> >> ><br>
>> >> > + if (D->hasAttr<OptimizeNoneAttr>()) {<br>
>> >> > + // OptimizeNone wins over OptimizeForSize and MinSize.<br>
>> >> > + B.removeAttribute(llvm::Attribute::OptimizeForSize);<br>
>> >> > + B.removeAttribute(llvm::Attribute::MinSize);<br>
>> >><br>
>> >> Is this something we should warn the user on when we encounter a<br>
>> >> minsize attribute along with an optnone attribute? Or should this be<br>
>> >> mutually exclusive like in the case of optnone and alwaysinline?<br>
>> ><br>
>> > See reply to Richard.<br>
>> ><br>
>> >><br>
>> >> > + }<br>
>> >> > +<br>
>> >> > if (LangOpts.getStackProtector() == LangOptions::SSPOn)<br>
>> >> > B.addAttribute(llvm::Attribute::StackProtect);<br>
>> >> > else if (LangOpts.getStackProtector() == LangOptions::SSPReq)<br>
>> >> > Index: lib/Sema/SemaDeclAttr.cpp<br>
>> >> > ===================================================================<br>
>> >> > --- lib/Sema/SemaDeclAttr.cpp (revision 195153)<br>
>> >> > +++ lib/Sema/SemaDeclAttr.cpp (working copy)<br>
>> >> > @@ -1764,6 +1764,12 @@<br>
>> >> > return;<br>
>> >> > }<br>
>> >> ><br>
>> >> > + if (D->hasAttr<OptimizeNoneAttr>()) {<br>
>> >> > + S.Diag(Attr.getLoc(), diag::err_attributes_are_not_compatible)<br>
>> >> > + << Attr.getName() << "'optnone'";<br>
>> >><br>
>> >> This is not a grumble for you to fix, but it would be very nice if we<br>
>> >> could get the original spelling for this diagnostic instead of<br>
>> >> hard-coding one particular spelling.<br>
>> ><br>
>> > Ayup.<br>
>> ><br>
>> >><br>
>> >> > + return;<br>
>> >> > + }<br>
>> >> > +<br>
>> >> > D->addAttr(::new (S.Context)<br>
>> >> > AlwaysInlineAttr(Attr.getRange(), S.Context,<br>
>> >> ><br>
>> Attr.getAttributeSpellingListIndex()));<br>
>> >> > @@ -3637,6 +3643,25 @@<br>
>> >> > Attr.getAttributeSpellingListIndex()));<br>
>> >> > }<br>
>> >> ><br>
>> >> > +static void handleOptimizeNoneAttr(Sema &S, Decl *D,<br>
>> >> > + const AttributeList &Attr) {<br>
>> >> > + if (!isa<FunctionDecl>(D)) {<br>
>> >> > + S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)<br>
>> >> > + << Attr.getName() << ExpectedFunction;<br>
>> >> > + return;<br>
>> >> > + }<br>
>> >> > +<br>
>> >> > + if (D->hasAttr<AlwaysInlineAttr>()) {<br>
>> >> > + S.Diag(Attr.getLoc(), diag::err_attributes_are_not_compatible)<br>
>> >> > + << Attr.getName() << "'always_inline'";<br>
>> >> > + return;<br>
>> >> > + }<br>
>> >> > +<br>
>> >> > + D->addAttr(::new (S.Context)<br>
>> >> > + OptimizeNoneAttr(Attr.getRange(), S.Context,<br>
>> >> > +<br>
>> Attr.getAttributeSpellingListIndex()));<br>
>> >> > +}<br>
>> >> > +<br>
>> >> > static void handleNoInstrumentFunctionAttr(Sema &S, Decl *D,<br>
>> >> > const AttributeList<br>
>> &Attr)<br>
>> >> {<br>
>> >> > if (!isa<FunctionDecl>(D)) {<br>
>> >> > @@ -4662,6 +4687,9 @@<br>
>> >> > case AttributeList::AT_MinSize:<br>
>> >> > handleMinSizeAttr(S, D, Attr);<br>
>> >> > break;<br>
>> >> > + case AttributeList::AT_OptimizeNone:<br>
>> >> > + handleOptimizeNoneAttr(S, D, Attr);<br>
>> >> > + break;<br>
>> >> > case AttributeList::AT_Format: handleFormatAttr (S, D,<br>
>> >> Attr); break;<br>
>> >> > case AttributeList::AT_FormatArg: handleFormatArgAttr (S, D,<br>
>> >> Attr); break;<br>
>> >> > case AttributeList::AT_CUDAGlobal: handleGlobalAttr (S, D,<br>
>> >> Attr); break;<br>
>> >> > Index: test/CodeGen/attr-optnone.c<br>
>> >> > ===================================================================<br>
>> >> > --- test/CodeGen/attr-optnone.c (revision 0)<br>
>> >> > +++ test/CodeGen/attr-optnone.c (revision 0)<br>
>> >> > @@ -0,0 +1,26 @@<br>
>> >> > +// RUN: %clang_cc1 -fms-compatibility -emit-llvm < %s > %t<br>
>> >> > +// RUN: FileCheck %s --check-prefix=PRESENT < %t<br>
>> >> > +// RUN: FileCheck %s --check-prefix=ABSENT < %t<br>
>> >> > +<br>
>> >> > +__attribute__((optnone)) __forceinline<br>
>> >> > +int test2() { return 0; }<br>
>> >> > +// PRESENT-DAG: @test2{{.*}}[[ATTR2:#[0-9]+]]<br>
>> >> > +<br>
>> >> > +__attribute__((optnone)) __attribute__((minsize))<br>
>> >> > +int test3() { return 0; }<br>
>> >> > +// PRESENT-DAG: @test3{{.*}}[[ATTR3:#[0-9]+]]<br>
>> >> > +<br>
>> >> > +__attribute__((optnone)) __attribute__((cold))<br>
>> >> > +int test4() { return test2(); }<br>
>> >> > +// PRESENT-DAG: @test4{{.*}}[[ATTR4:#[0-9]+]]<br>
>> >> > +// Also check that test2 isn't inlined into test4 (optnone trumps<br>
>> >> forceinline).<br>
>> >> > +// PRESENT-DAG: call i32 @test2<br>
>> >> > +<br>
>> >> > +// Check that we set both attributes noinline and optnone on each<br>
>> >> function.<br>
>> >> > +// PRESENT-DAG: attributes [[ATTR2]] = {<br>
>> >> {{.*}}noinline{{.*}}optnone{{.*}} }<br>
>> >> > +// PRESENT-DAG: attributes [[ATTR3]] = {<br>
>> >> {{.*}}noinline{{.*}}optnone{{.*}} }<br>
>> >> > +// PRESENT-DAG: attributes [[ATTR4]] = {<br>
>> >> {{.*}}noinline{{.*}}optnone{{.*}} }<br>
>> >> > +<br>
>> >> > +// Check that no 'optsize' or 'minsize' attributes appear.<br>
>> >> > +// ABSENT-NOT: optsize<br>
>> >> > +// ABSENT-NOT: minsize<br>
>> >> > Index: test/SemaCXX/attr-optnone.cpp<br>
>> >> > ===================================================================<br>
>> >> > --- test/SemaCXX/attr-optnone.cpp (revision 0)<br>
>> >> > +++ test/SemaCXX/attr-optnone.cpp (revision 0)<br>
>> >> > @@ -0,0 +1,44 @@<br>
>> >> > +// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -<br>
>> >> verify %s<br>
>> >> > +<br>
>> >> > +int foo() __attribute__((optnone));<br>
>> >> > +int bar() __attribute__((optnone)) __attribute__((noinline));<br>
>> >> > +<br>
>> >> > +int baz() __attribute__((always_inline)) __attribute__((optnone));<br>
>> //<br>
>> >> expected-error{{'always_inline' and 'optnone' attributes are not<br>
>> >> compatible}}<br>
>> >> > +int qux() __attribute__((optnone)) __attribute__((always_inline));<br>
>> //<br>
>> >> expected-error{{'optnone' and 'always_inline' attributes are not<br>
>> >> compatible}}<br>
>> >> > +<br>
>> >> > +int globalVar __attribute__((optnone)); // expected-<br>
>> >> warning{{'optnone' attribute only applies to functions}}<br>
>> >> > +<br>
>> >> > +int fubar(int __attribute__((optnone)), int); // expected-<br>
>> >> warning{{'optnone' attribute only applies to functions}}<br>
>> >> > +<br>
>> >> > +struct A {<br>
>> >> > + int aField __attribute__((optnone)); // expected-<br>
>> >> warning{{'optnone' attribute only applies to functions}}<br>
>> >> > +};<br>
>> >> > +<br>
>> >> > +struct B {<br>
>> >> > + void foo() __attribute__((optnone));<br>
>> >> > + static void bar() __attribute__((optnone));<br>
>> >> > +};<br>
>> >> > +<br>
>> >> > +// Verify that we can specify the [[clang::optnone]] syntax as<br>
>> well.<br>
>> >> > +<br>
>> >> > +[[clang::optnone]]<br>
>> >> > +int foo2();<br>
>> >> > +[[clang::optnone]]<br>
>> >> > +int bar2() __attribute__((noinline));<br>
>> >> > +<br>
>> >> > +[[clang::optnone]]<br>
>> >> > +int baz2() __attribute__((always_inline)); // expected-<br>
>> >> error{{'always_inline' and 'optnone' attributes are not compatible}}<br>
>> >> > +<br>
>> >> > +[[clang::optnone]] int globalVar2; //expected-warning{{'optnone'<br>
>> >> attribute only applies to functions}}<br>
>> >> > +<br>
>> >> > +struct A2 {<br>
>> >> > + [[clang::optnone]] int aField; // expected-warning{{'optnone'<br>
>> >> attribute only applies to functions}}<br>
>> >> > +};<br>
>> >> > +<br>
>> >> > +struct B2 {<br>
>> >> > + [[clang::optnone]]<br>
>> >> > + void foo();<br>
>> >> > + [[clang::optnone]]<br>
>> >> > + static void bar();<br>
>> >> > +};<br>
>> >> > +<br>
>> >> ><br>
>> >><br>
>> >> ~Aaron<br>
>> >><br>
>> >> On Tue, Nov 19, 2013 at 5:25 PM, Robinson, Paul<br>
>> >> <<a href="mailto:Paul_Robinson@playstation.sony.com">Paul_Robinson@playstation.sony.com</a>> wrote:<br>
>> >> > Adds a new ‘optnone’ function attribute that maps directly to the<br>
>> LLVM<br>
>> >> IR<br>
>> >> > ‘optnone’ attribute.<br>
>> >> ><br>
>> >> > --paulr<br>
>> >> ><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>
>><br>
>> ~Aaron<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>
</div></div></blockquote></div><br></div>