[PATCH] Add C/C++ attribute 'optnone'

Richard Smith richard at metafoo.co.uk
Wed Nov 20 11:53:18 PST 2013


Please also update the Clang documentation to cover this attribute.


On Wed, Nov 20, 2013 at 5:29 AM, Aaron Ballman <aaron at aaronballman.com>wrote:

> Patch LGTM! Thanks!
>
> ~Aaron
>
> On Tue, Nov 19, 2013 at 8:56 PM, Robinson, Paul
> <Paul_Robinson at playstation.sony.com> wrote:
> > Revised patch with requested changes to allow 'optnone' on an
> > ObjC method.
> > --paulr
> >
> >> -----Original Message-----
> >> From: aaron.ballman at gmail.com [mailto:aaron.ballman at gmail.com] On
> Behalf
> >> Of Aaron Ballman
> >> Sent: Tuesday, November 19, 2013 5:11 PM
> >> To: Robinson, Paul
> >> Cc: cfe-commits at cs.uiuc.edu cfe
> >> Subject: Re: [PATCH] Add C/C++ attribute 'optnone'
> >>
> >> On Tue, Nov 19, 2013 at 6:33 PM, Robinson, Paul
> >> <Paul_Robinson at playstation.sony.com> wrote:
> >> >> -----Original Message-----
> >> >> From: aaron.ballman at gmail.com [mailto:aaron.ballman at gmail.com] On
> >> Behalf
> >> >> Of Aaron Ballman
> >> >> Sent: Tuesday, November 19, 2013 2:36 PM
> >> >> To: Robinson, Paul
> >> >> Cc: cfe-commits at cs.uiuc.edu 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
> >> >> <Paul_Robinson at playstation.sony.com> wrote:
> >> >> > Adds a new ‘optnone’ function attribute that maps directly to the
> >> LLVM
> >> >> IR
> >> >> > ‘optnone’ attribute.
> >> >> >
> >> >> > --paulr
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >> > _______________________________________________
> >> >> > cfe-commits mailing list
> >> >> > cfe-commits at cs.uiuc.edu
> >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >> >> >
> >> >
> >>
> >> ~Aaron
> >
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131120/da56583c/attachment.html>


More information about the cfe-commits mailing list