[Clang] New attribute optsize

[xbolva00]

Summary

Clang exposes attribute 'minsize' which can be used to perform optimizations to reduce code size as much as possible. Like optimization flag -Oz.

This patch exposes attribute 'optsize' which can be used to perform optimizations to reduce code size and at the same time the impact on performance stays minimal. Like optimization flag -Os.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

2263

I would like to hold off on this until we have more agreement on https://reviews.llvm.org/D126984 because I would like clang::optsize to be expressed as one of the spellings of the attribute proposed there instead of introducing a separate semantic attribute for it. Then we can also shift clang::optnone as well. (Though I'm guessing we may have to figure out something for the documentation so that we don't cram all of the docs into one place -- that leaks an implementation detail.)

[xbolva00]

clang/include/clang/Basic/Attr.td

2263

Do you plan to rework also minsize that way? and others.. ?

Documentation is gonna be a huge pain, yes.

In my opinion this is an useless blocker.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

2263

Do you plan to rework also minsize that way? and others.. ?

Eventually, yes, but I wasn't going to try to nerd snipe you into doing them (or doing them all at once) as part of this work.

Documentation is gonna be a huge pain, yes.

Yup, but it's something we should be solving anyway (we already have other attributes in similar circumstances and we've mostly just punted by saying the attributes all do similar enough things that they should be documented under one group).

In my opinion this is an useless blocker.

I'm willing to be flexible, but we have > 400 semantic attributes already and the support matrix for their combinations is pretty bad. Keeping just with the optimization attributes, optnone is a warning when misapplied, but minsize is an error. You've duplicated the logic of minsize here with your patch (including identical documentation with no hope of a user figuring out which attribute to use or how they're different), and there's no consideration for what happens when both attributes are used yet there is for combining with optnone. Etc.

So I don't see this as a useless blocker at all. I intend it as an attempt to rein in our existing maintenance burden in an area that we're now talking about extending even further. If you have other ideas on how to ensure we stop making the support matrix for the attribute combinations worse, I'm definitely willing to consider alternative approaches.

[xbolva00]

clang/include/clang/Basic/Attr.td

2263

Understood.

including identical documentation

Not so true :) As mentioned, optsize reduces size but still promises +- same performance. Minsize does not care about performance at all.

I'm willing to be flexible, but we have > 400 semantic attributes already and the support matrix for their combinations is pretty bad.

I agree. This sounds a stronger motivation than optimise attribute/pragma.

If you have other ideas on how to ensure we stop making the support matrix for the attribute combinations worse, I'm definitely willing to consider alternative approaches.

I like your OptimiseAttr idea (but yeah, a lot of work).

RFC at Discourse would be great as well (or maybe I am blind...)

[aaron.ballman]

clang/include/clang/Basic/Attr.td

2263

Not so true :) As mentioned, optsize reduces size but still promises +- same performance. Minsize does not care about performance at all.

:: reading skills intensify :: you're right, sorry for that!

It sounds like D126984 may be temporarily abandoned in favor of an even easier approach for #pragma optimize, which also throws a wrinkle in my "please use that other functionality once it lands" idea. The underlying concerns that led to me suggesting that still remain.

Do you have a strong need for optsize right now or was it more of a "since I'm in the area, let's complete the set" kind of feature? (I'm trying to see how interested you are in continuing the work there to get us to a common semantic attribute interface for these.)

[xbolva00]

clang/include/clang/Basic/Attr.td

2263

No problem :)

---

I have no strong need. I read some time ago on stackoverflow that someone wanted to optimise function for size and for gcc they recommended optimise attribute and for clang minsize. (Well, minsize could be a bit stronger) Then I found out that optsize is missing.

And when D126984 came up, I was recommending minsize as alternative for optimise(-Oz) and then I again realized that optsize is missing, so no good solution for somebody who wants -Os.

If you create something new, I have no problem to "port" this patch to your new interface/architecture.

[aaron.ballman]

clang/include/clang/Basic/Attr.td

2263

Okay, thanks for the info! I don't have the time to do the refactoring work myself currently, though.

In the interests of not blocking useful work: if that refactoring doesn't take place in the "near future" (let's say ~6 months?) then it's fine to proceed with this patch as-is?

[xbolva00]

clang/include/clang/Basic/Attr.td

2263

Yes, no problem

clang/docs/ReleaseNotes.rst

 
 - Attribute ``no_builtin`` should now affect the generated code. It now disables
   builtins (corresponding to the specific names listed in the attribute) in the
   body of the function the attribute is on.
 
 - When the ``weak`` attribute is applied to a const qualified variable clang no longer
   tells the backend it is allowed to optimize based on initializer value.
 
+- New function attribute ``optsize`` can be used to perform optimizations designed to reduce
+  code size as long as they do not significantly impact runtime performance.
+
 Windows Support
 ---------------
 
 - Add support for MSVC-compatible ``/JMC``/``/JMC-`` flag in clang-cl (supports
   X86/X64/ARM/ARM64). ``/JMC`` could only be used when ``/Zi`` or ``/Z7`` is
   turned on. With this addition, clang-cl can be used in Visual Studio for the
   JustMyCode feature. Note, you may need to manually add ``/JMC`` as additional
   compile options in the Visual Studio since it currently assumes clang-cl does not support ``/JMC``.

clang/include/clang/Basic/Attr.td

 }
 
 def OptimizeNone : InheritableAttr {
   let Spellings = [Clang<"optnone">];
   let Subjects = SubjectList<[Function, ObjCMethod]>;
   let Documentation = [OptnoneDocs];
 }
 
+def OptSize : InheritableAttr {
+  let Spellings = [Clang<"optsize">];
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
+  let Documentation = [OptSizeDocs];
+}
+
 def Overloadable : Attr {
   let Spellings = [Clang<"overloadable">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [OverloadableDocs];
   let SimpleHandler = 1;
 }
 
 def Override : InheritableAttr {

clang/include/clang/Basic/AttrDocs.td

 specified function can improve the quality of the debugging information
 for that function.
 
 This attribute is incompatible with the ``always_inline`` and ``minsize``
 attributes.
   }];
 }
 
+def OptSizeDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+This function attribute indicates that optimization passes and code generator
+passes make choices that keep the function code size low. The compiler also does
+optimizations to reduce code size as long as they do not significantly impact
+runtime performance.
+  }];
+}
+
 def LoopHintDocs : Documentation {
   let Category = DocCatStmt;
   let Heading = "#pragma clang loop";
   let Content = [{
 The ``#pragma clang loop`` directive allows loop optimization hints to be
 specified for the subsequent loop. The directive allows pipelining to be
 disabled, or vectorization, vector predication, interleaving, and unrolling to
 be enabled or disabled. Vector width, vector predication, interleave count,

clang/include/clang/Sema/Sema.h

public:
   SectionAttr *mergeSectionAttr(Decl *D, const AttributeCommonInfo &CI,
                                 StringRef Name);
   CodeSegAttr *mergeCodeSegAttr(Decl *D, const AttributeCommonInfo &CI,
                                 StringRef Name);
   AlwaysInlineAttr *mergeAlwaysInlineAttr(Decl *D,
                                           const AttributeCommonInfo &CI,
                                           const IdentifierInfo *Ident);
   MinSizeAttr *mergeMinSizeAttr(Decl *D, const AttributeCommonInfo &CI);
+  OptSizeAttr *mergeOptSizeAttr(Decl *D, const AttributeCommonInfo &CI);
   SwiftNameAttr *mergeSwiftNameAttr(Decl *D, const SwiftNameAttr &SNA,
                                     StringRef Name);
   OptimizeNoneAttr *mergeOptimizeNoneAttr(Decl *D,
                                           const AttributeCommonInfo &CI);
   InternalLinkageAttr *mergeInternalLinkageAttr(Decl *D, const ParsedAttr &AL);
   InternalLinkageAttr *mergeInternalLinkageAttr(Decl *D,
                                                 const InternalLinkageAttr &AL);
   WebAssemblyImportNameAttr *mergeImportNameAttr(

clang/lib/CodeGen/CodeGenModule.cpp

void CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,
   }
 
   // Track whether we need to add the optnone LLVM attribute,
   // starting with the default for this optimization level.
   bool ShouldAddOptNone =
       !CodeGenOpts.DisableO0ImplyOptNone && CodeGenOpts.OptimizationLevel == 0;
   // We can't add optnone in the following cases, it won't pass the verifier.
   ShouldAddOptNone &= !D->hasAttr<MinSizeAttr>();
+  ShouldAddOptNone &= !D->hasAttr<OptSizeAttr>();
   ShouldAddOptNone &= !D->hasAttr<AlwaysInlineAttr>();
 
   // Add optnone, but do so only if the function isn't always_inline.
   if ((ShouldAddOptNone || D->hasAttr<OptimizeNoneAttr>()) &&
       !F->hasFnAttribute(llvm::Attribute::AlwaysInline)) {
     B.addAttribute(llvm::Attribute::OptimizeNone);
 
     // OptimizeNone implies noinline; we should not be inlining such functions.
if (D->hasAttr<ColdAttr>()) {
       if (!ShouldAddOptNone)
         B.addAttribute(llvm::Attribute::OptimizeForSize);
       B.addAttribute(llvm::Attribute::Cold);
     }
     if (D->hasAttr<HotAttr>())
       B.addAttribute(llvm::Attribute::Hot);
     if (D->hasAttr<MinSizeAttr>())
       B.addAttribute(llvm::Attribute::MinSize);
+    if (D->hasAttr<OptSizeAttr>())
+      B.addAttribute(llvm::Attribute::OptimizeForSize);
   }
 
   F->addFnAttrs(B);
 
   unsigned alignment = D->getMaxAlignment() / Context.getCharWidth();
   if (alignment)
     F->setAlignment(llvm::Align(alignment));
 

clang/lib/Sema/SemaDeclAttr.cpp

MinSizeAttr *Sema::mergeMinSizeAttr(Decl *D, const AttributeCommonInfo &CI) {
   }
 
   if (D->hasAttr<MinSizeAttr>())
     return nullptr;
 
   return ::new (Context) MinSizeAttr(Context, CI);
 }
 
+OptSizeAttr *Sema::mergeOptSizeAttr(Decl *D, const AttributeCommonInfo &CI) {
+  if (OptimizeNoneAttr *Optnone = D->getAttr<OptimizeNoneAttr>()) {
+    Diag(CI.getLoc(), diag::warn_attribute_ignored) << "'optsize'";
+    Diag(Optnone->getLocation(), diag::note_conflicting_attribute);
+    return nullptr;
+  }
+
+  if (D->hasAttr<OptSizeAttr>())
+    return nullptr;
+
+  return ::new (Context) OptSizeAttr(Context, CI);
+}
+
 SwiftNameAttr *Sema::mergeSwiftNameAttr(Decl *D, const SwiftNameAttr &SNA,
                                         StringRef Name) {
   if (const auto *PrevSNA = D->getAttr<SwiftNameAttr>()) {
     if (PrevSNA->getName() != Name && !PrevSNA->isImplicit()) {
       Diag(PrevSNA->getLocation(), diag::err_attributes_are_not_compatible)
           << PrevSNA << &SNA;
       Diag(SNA.getLoc(), diag::note_conflicting_attribute);
     }
if (AlwaysInlineAttr *Inline =
     D->addAttr(Inline);
 }
 
 static void handleMinSizeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (MinSizeAttr *MinSize = S.mergeMinSizeAttr(D, AL))
     D->addAttr(MinSize);
 }
 
+static void handleOptSizeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (OptSizeAttr *OptSize = S.mergeOptSizeAttr(D, AL))
+    D->addAttr(OptSize);
+}
+
 static void handleOptimizeNoneAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (OptimizeNoneAttr *Optnone = S.mergeOptimizeNoneAttr(D, AL))
     D->addAttr(Optnone);
 }
 
 static void handleConstantAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   const auto *VD = cast<VarDecl>(D);
   if (VD->hasLocalStorage()) {
case ParsedAttr::AT_ExternalSourceSymbol:
     handleExternalSourceSymbolAttr(S, D, AL);
     break;
   case ParsedAttr::AT_MinSize:
     handleMinSizeAttr(S, D, AL);
     break;
   case ParsedAttr::AT_OptimizeNone:
     handleOptimizeNoneAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_OptSize:
+    handleOptSizeAttr(S, D, AL);
+    break;
   case ParsedAttr::AT_EnumExtensibility:
     handleEnumExtensibilityAttr(S, D, AL);
     break;
   case ParsedAttr::AT_SYCLKernel:
     handleSYCLKernelAttr(S, D, AL);
     break;
   case ParsedAttr::AT_SYCLSpecialClass:
     handleSimpleAttribute<SYCLSpecialClassAttr>(S, D, AL);

clang/test/CodeGen/attr-optsize.cpp

+// RUN: %clang_cc1 -Os -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s -check-prefix=Os
+// RUN: %clang_cc1     -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s -check-prefix=OTHER
+// RUN: %clang_cc1 -O1 -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s -check-prefix=OTHER
+// RUN: %clang_cc1 -O2 -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s -check-prefix=OTHER
+// RUN: %clang_cc1 -O3 -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s -check-prefix=OTHER
+// Check that we set the optsize attribute on each function
+// when Os optimization level is set.
+
+__attribute__((optsize))
+int test1() {
+  return 42;
+// Os: @{{.*}}test1{{.*}}[[OPTSIZE:#[0-9]+]]
+// OTHER: @{{.*}}test1{{.*}}[[MS:#[0-9]+]]
+}
+
+int test2() {
+  return 42;
+// Os: @{{.*}}test2{{.*}}[[OPTSIZE]]
+// Os: ret
+// OTHER: @{{.*}}test2
+// OTHER-NOT: [[MS]]
+// OTHER: ret
+}
+
+int test3() {
+  return 42;
+// Os: @{{.*}}test3{{.*}}[[OPTSIZE]]
+// Os: ret
+// OTHER: @{{.*}}test3
+// OTHER-NOT: [[MS]]
+// OTHER: ret
+}
+
+// Check that the optsize attribute is well propagated through
+// template instantiation
+
+template<typename T>
+__attribute__((optsize))
+void test4(T arg) {
+  return;
+}
+
+template
+void test4<int>(int arg);
+// Os: define{{.*}}void @{{.*}}test4
+// Os: [[OPTSIZE]]
+// OTHER: define{{.*}}void @{{.*}}test4
+// OTHER: [[MS]]
+
+template
+void test4<float>(float arg);
+// Os: define{{.*}}void @{{.*}}test4
+// Os: [[OPTSIZE]]
+// OTHER: define{{.*}}void @{{.*}}test4
+// OTHER: [[MS]]
+
+template<typename T>
+void test5(T arg) {
+  return;
+}
+
+template
+void test5<int>(int arg);
+// Os: define{{.*}}void @{{.*}}test5
+// Os: [[OPTSIZE]]
+// OTHER: define{{.*}}void @{{.*}}test5
+// OTHER-NOT: define{{.*}}void @{{.*}}test5{{.*}}[[MS]]
+
+template
+void test5<float>(float arg);
+// Os: define{{.*}}void @{{.*}}test5
+// Os: [[OPTSIZE]]
+// OTHER: define{{.*}}void @{{.*}}test5
+// OTHER-NOT: define{{.*}}void @{{.*}}test5{{.*}}[[MS]]
+
+// Os: attributes [[OPTSIZE]] = { {{.*}} optsize {{.*}} }
+
+// OTHER: attributes [[MS]] = { {{.*}} optsize {{.*}} }
+

clang/test/CodeGenObjC/attr-optsize.m

+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+@interface Test
+- (void)test;
+@end
+
+@implementation Test
+- (void)test __attribute__((optsize)) {
+    // CHECK: define{{.*}}Test test
+    // CHECK: optsize
+}
+@end

clang/test/Sema/attr-optsize.c

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int foo(void) __attribute__((__optsize__));
+
+int var1 __attribute__((__optsize__)); // expected-warning{{'__optsize__' attribute only applies to functions and Objective-C methods}}