[PATCH] D149446: [Pipelines] Don't skip GlobalDCE in ThinLTO pre-link

Tue May 2 23:30:45 PDT 2023

mingmingl added inline comments.

================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1116
-  // Remove dead code, except in the ThinLTO pre-link pipeline where we may want
-  // to keep available_externally functions.
-  if (Phase != ThinOrFullLTOPhase::ThinLTOPreLink)
----------------
mingmingl wrote:
> mingmingl wrote:
> > This comment reminds me of another comment https://github.com/llvm/llvm-project/blob/4b80a8c72d3d0dba4ee2469471dfccb7e257b319/llvm/lib/Passes/PassBuilderPipelines.cpp#L983-l986, which points out in postlink (after function import) available_externally functions may appear unreferenced, if the function is referenced as virtual table indirect calls, so ICP pass runs before global-opt pass.
> > 
> > I don't know how functions become `available_externally` symbols in prelink (e.g., a small c++ example) stage, but wonder if the same 'seemingly unreferenced' but indeed referenced pattern is relevant here.
> Update:
> 
> By flag-gate this pass pipeline change in an application binary (with thinlto and ifdo) and compare the remarks, the counter of remark lines is different (10-ish out of ~30,000 lines removed with prelink global-dce) for ICP. Repeat the flag gate method with WPD remarks gives a small (<0.5%?) counter difference (ignore hash difference in imported function names)
> 
> For ICP, the removed remarks is about a function defined in cpp header `a.h` takes a function pointer from `b.cpp`. Need to chase down a little bit to get a reduced test case.
s/in an application binary/ flag gate the change in compiler to build an application binary with the flag on and off.

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

https://reviews.llvm.org/D149446