[PATCH] D72404: [ThinLTO/FullLTO] Support Os and Oz

[Ayke via Phabricator via cfe-commits]

aykevl added a comment.

Apparently there is also another patch that tries to do something very similar: D81223 <https://reviews.llvm.org/D81223>.

In D72404#3305275 <https://reviews.llvm.org/D72404#3305275>, @mehdi_amini wrote:

> Why can't you build the object files with `-Os` in the first place instead of changing the optimization level between the compilation and the linking phases?

I'm not changing the optimization level. The bitcode files are built with an equivalent of `-Oz` and have the `optsize` and `minsize` attributes. It's the optimization passes in the linker (ThinLTO) that don't respect these attributes.

In D72404#3305267 <https://reviews.llvm.org/D72404#3305267>, @steven_wu wrote:

> Before I say yes or no to this patch, have you figured out what are the passes that causes the most size regression? Ideally, with function attributes on the function, it shouldn't be much size impact on the output.

Unfortunately, there is an impact. I did a quick test with a small program (around 4.7kB compiled code) and it looks like the LoopRotate pass is the main culprit. If `MaxHeaderSize` is set to 0 instead of -1, the code size regression is avoided. The following hacky patch avoids the code size increase:

  diff --git a/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp b/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  index aa916345954d..99be1926cf34 100644
  --- a/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  +++ b/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  @@ -450,7 +450,7 @@ void PassManagerBuilder::addFunctionSimplificationPasses(
     // TODO: Investigate promotion cap for O1.
     MPM.add(createLICMPass(LicmMssaOptCap, LicmMssaNoAccForPromotionCap));
     // Rotate Loop - disable header duplication at -Oz
  -  MPM.add(createLoopRotatePass(SizeLevel == 2 ? 0 : -1, PrepareForLTO));
  +  MPM.add(createLoopRotatePass(0/*SizeLevel == 2 ? 0 : -1*/, PrepareForLTO));
     // TODO: Investigate promotion cap for O1.
     MPM.add(createLICMPass(LicmMssaOptCap, LicmMssaNoAccForPromotionCap));
     if (EnableSimpleLoopUnswitch)
  @@ -917,7 +917,7 @@ void PassManagerBuilder::populateModulePassManager(
     // Re-rotate loops in all our loop nests. These may have fallout out of
     // rotated form due to GVN or other transformations, and the vectorizer relies
     // on the rotated form. Disable header duplication at -Oz.
  -  MPM.add(createLoopRotatePass(SizeLevel == 2 ? 0 : -1, PrepareForLTO));
  +  MPM.add(createLoopRotatePass(0 /*SizeLevel == 2 ? 0 : -1*/, PrepareForLTO));
   
     // Distribute loops to allow partial vectorization.  I.e. isolate dependences
     // into separate loop that would otherwise inhibit vectorization.  This is

So, should all passes just look at the `optsize` and `minsize` attributes instead of the `SizeLevel`? In other words, should `PassManagerBuilder.SizeLevel` be removed and should passes only look at function attributes instead of `SizeLevel`? Because at the moment, it's a weird mix of both. IMHO size level should either all go via function attributes or via a flag, not something in between as it is now.
Also, if size level is done via function attributes, why not optimization level? There is already `optnone`. I'm not saying that's better, but right now I don't see the logic in this whole system.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72404/new/

https://reviews.llvm.org/D72404