[PATCH] D63976: Allow clang -Os and -Oz to work with -flto and lld

Tue Jul 2 09:38:55 PDT 2019

mehdi_amini added a comment.

In D63976#1566236 <https://reviews.llvm.org/D63976#1566236>, @ruiu wrote:

> I agree with Teresa. I don't think automatically setting O3 <https://reviews.llvm.org/owners/package/3/> for Os and Oz is a good idea because these three options are different (that's why we have three different options in the first place). Adding an Os and Oz to lld's LTO option seems like a good idea, but they should be mapped to their corresponding features.

It shouldn't Matt

================
Comment at: llvm-9.0.0-20190629/clang/lib/Driver/ToolChains/CommonArgs.cpp:395
+      if(OOpt == "s" || OOpt == "z")
+        OOpt = "3";
+    }
----------------
tejohnson wrote:
> Os/Oz are closer to O2 than O3 (which allows much more aggressive code size increasing optimizations).
> 
> Better though to add a size level to the LTO::Config, teach lld to pass it through properly, then using the LTO Config to set the SizeLevel in the old PM and the PassBuilder::OptimizationLevel in the new PM when setting up the LTO backend pipelines, similar to how CodeGenLevel.OptimizeSize is handled in clang (BackendUtil.cpp).
> 
> My concern is that silently mapping Os/Oz to do something different than in the non-LTO pipeline is going to end up more confusing than the current error (which isn't good either, but at least makes it clear that it isn't supported).
Using O2 makes sense to me. 
Beyond this does it matter much? Isn't the important part of Os/Oz  carried through a function attribute?

Repository:
  rC Clang

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

https://reviews.llvm.org/D63976