[PATCH] D153655: [LTO][GlobalDCE] Use pass parameter instead of module flag for LTO phase

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:675
+Expected<bool> parseGlobalDCEPassOptions(StringRef Params) {
+  return parseSinglePassOption(Params, "in-lto-post-link", "InLTOPostLink");
+}
----------------
tejohnson wrote:
> arsenm wrote:
> > tejohnson wrote:
> > > arsenm wrote:
> > > > aeubanks wrote:
> > > > > `GlobalDCE`
> > > > Can you use a name descriptive of the behavior change, rather than the why/where?
> > > Looks like it is supposed to be the pass name, fixed.
> > I mean "in-lto-post-link" doesn't tell me how the pass is going to behave differently. I mean something like handle-vtable-something or skip-something
> Oh hmm, I was thought this should be the same as the parameter name, which I was trying to keep close to the previous module flag.
> 
> I'm not sure what to rename this to that is descriptive yet not too long. The behavior is basically: when in the LTO post link, VFE can be applied to vtables with linkage unit vcall visibility (by default it is just those with translation unit vcall visibility). Open to suggestions.
After skimming the code and barely knowing anything about this optimization, my suggestions:

'assume-visible-vcall-table'
'assume-vcall-visibility-translation-unit'
'assume-module-visibility'
'whole-module-vfe'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153655