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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:675
+Expected<bool> parseGlobalDCEPassOptions(StringRef Params) {
+  return parseSinglePassOption(Params, "in-lto-post-link", "InLTOPostLink");
+}
----------------
arsenm wrote:
> 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'
Ok thanks. After considering these and thinking about it some more, I've settled on 'vfe-linkage-unit-visibility' (since we are telling VFE that it can assume it has visibility over the full linkage unit).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153655