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