[PATCH] D127779: [LTO][ELF] Add --lto-exit-on option

Fri Jun 24 10:49:44 PDT 2022

tejohnson added inline comments.

================
Comment at: lld/test/ELF/lto/lto-exit-on.ll:23
+; RUN: ls %t/*.1.promote*
+; RUN: ls %t/*.2.internalize* | count 1
+; RUN: not ls %t/*.3.import*
----------------
Northbadge wrote:
> Northbadge wrote:
> > tejohnson wrote:
> > > Why are we getting one when exiting on postpromote?
> > the hook is called from [[ https://github.com/llvm/llvm-project/blob/5e9aed1be5a5608d97c2ec3ac87698a8257b0ea7/llvm/lib/LTO/LTOBackend.cpp#L585 | lto::thinBackend ]] AND also from [[ https://github.com/llvm/llvm-project/blob/5e9aed1be5a5608d97c2ec3ac87698a8257b0ea7/llvm/lib/LTO/LTO.cpp#L1134 | LTO::runRegularLTO ]]. When running the test case, LTO::runRegularLTO is called before lto::thinBackend from [[ https://github.com/llvm/llvm-project/blob/5e9aed1be5a5608d97c2ec3ac87698a8257b0ea7/llvm/lib/LTO/LTO.cpp#L1038-L1040 | here ]]. With postpromote being a ThinLTO exclusive hook, the postinternalize hook (from regularLTO) manages to run first before the postpromote hook is hit, thus creating 1 file. (I believe this is unavoidable without changes to that last snippet of code)
> > 
> > Now that you've pointed it out though, I realize that specifying `--lto-exit-on=postinternalize` would be ambiguous due to the two callsites, this is also the case for the `preopt` hook as I've just checked. The quick fix would be to make these two hooks invalid arguments for the option. Would this be an ok solution?
> > 
> > 
> I've removed it for now, let me know if I should try doing something else
I think it would be ok to leave the other cases in and just document it. 

================
Comment at: llvm/lib/LTO/LTOBackend.cpp:187
+  else if (ltoExitOn == "precodegen")
+    setHook(PreCodeGenModuleHook);
+
----------------
Probably want to issue an error here if it is something else.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127779