[PATCH] D30920: Do not pass -Os and -Oz to the Gold plugin

Teresa Johnson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 14 06:33:55 PDT 2017


tejohnson added a comment.

In https://reviews.llvm.org/D30920#700133, @pcc wrote:

> In https://reviews.llvm.org/D30920#700077, @tejohnson wrote:
>
> > Until everything is converted to using size attributes, it seems like a correct fix for the bug is to accept these options in the gold-plugin and pass through the LTO API to the PassManagerBuilder.
>
>
> Not necessarily. There is no requirement (from a correctness perspective) that `-Os` at link time should exactly match the behaviour of `-Os` at compile time.


Sure, but there is certainly a perception that optimization flags affecting the non-LTO pipeline should similarly affect the LTO pipeline. LTO should be transparent to the user, so if -Os behaves one way without LTO, it seems problematic to me if it behaves a different way with LTO.

That being said, agree that the best way to enforce that is to pass the relevant flags through the IR. (On the flip side, if the user passes -O1 to the link step, it does get passed through to the plugin and affects the LTO optimization pipeline...)

Pirama - can you file a bug (and cc me on it) for the need to convert existing users of the sizeLevel flag to instead use the relevant function attributes?



================
Comment at: lib/Driver/ToolChains/CommonArgs.cpp:376
+      if (OptLevel != "s" && OptLevel != "z")
+        OOpt = OptLevel;
+    } else if (A->getOption().matches(options::OPT_O0))
----------------
Note that -Os and -Oz are supposed to map to -O2 minus some size increasing options. Not setting OOpt in that case works because the OptLevel in the gold-plugin defaults to -O2. I'd rather make it explicit here, i.e. add an else that sets OOpt to "2" under these options. You can add a check for that in your tests.


https://reviews.llvm.org/D30920





More information about the cfe-commits mailing list