[PATCH] D141970: [Clang][LLD] Add --lto-CGO[0-3] option

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lld/ELF/Driver.cpp:1143
+  config->ltocgo =
+      *CodeGenOpt::getLevel(args::getInteger(args, OPT_lto_CGO, config->ltoo));
   config->ltoObjPath = args.getLastArgValue(OPT_lto_obj_path_eq);
----------------
pcc wrote:
> scott.linder wrote:
> > pcc wrote:
> > > It looks like this is setting the CG opt level based on the LTO opt level, which is undesirable for the reasons mentioned in D57422. Let's keep the existing logic for determining the default CG opt level.
> > Right, but the existing `lld::args::getCGOptLevel` is also setting the CG opt level based on the LTO opt level, just in a surprising manner (i.e. not 1:1). I saw this same concern in a comment on D57422 by @mehdi_amini but didn't see any response.
> > 
> > I find it confusing that a new user of LTO would start with a command-line like this and observe that they got `CodeGenOpt::None`:
> > 
> > ```
> > $ echo 'int main() {}' | build/bin/clang -### -x c -O0 -
> > "clang" "-cc1" "-emit-obj" "-O0" ...
> > "ld.lld" ...
> > ```
> > 
> > And that by adding just `-flto` they would still see command-lines for CC1 and the linker that would indicate the same would be true:
> > 
> > ```
> > $ echo 'int main() {}' | build/bin/clang -### -x c -O0 - -flto
> > "clang" "-cc1" "-emit-llvm-bc" "-O0" ...
> > "ld.lld" "-plugin-opt=O0" ...
> > ```
> > 
> > And yet somehow they get `CodeGenOpt::Default` behavior (and before this patch would have no recourse to correct this).
> > 
> > It isn't obvious to me (and I assume a typical user of Clang) that the internal behavior when adding "-flto" is that the CG opt level cannot ever fall below `Default`.
> > 
> > If we really need to retain the special behavior in the linkers themselves, could we instead teach Clang to start passing the new option, so the above becomes:
> > 
> > ```
> > $ echo 'int main() {}' | build/bin/clang -### -x c -O0 - -flto
> > "clang" "-cc1" "-emit-llvm-bc" "-O0" ...
> > "ld.lld" "-plugin-opt=O0" "--lto-CGO0" ...
> > ```
> > 
> > ?
> I think that the default is a separate issue from making the CG opt level configurable. Let's just keep this patch about making it configurable for now.
> 
> I agree that the default CG opt level is somewhat unintuitive, but the goal was to match the opt level used at compile time. Ideally it would be based on the information in the .bc file, but in the absence of such information, we have to "guess" which opt level was used, and the logic that was implemented was the most reasonable guess that we could come up with at the time.
I maintain that the optimization level of a translation unit is not semantics and should not matter. The optimization level for the final codegen invocation is what should matter 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141970