[PATCH] D91583: [lld] Allow --export-dynamic to override --lto-whole-program-visibility

Tue Nov 17 15:37:15 PST 2020

tejohnson added a comment.

In D91583#2401100 <https://reviews.llvm.org/D91583#2401100>, @MaskRay wrote:

> Hi, I am still learning the feature and I've just played a bit with the test. I've a couple of questions:
>
> - The `--export-dynamic` usage in the test seems a bit confusing. Are the `VisibleToRegularObj` bits of these `_ZTV*` symbols the important matter and `--export-dynamic` is just an approach to make them true?

Correct. It was just a shorthand way of preventing these symbols from being eliminated.

> - If yes, I guess all these `--export-dynamic` can be replaced with `-u _ZTV1B -u _ZTV1C -u _ZTV1D` in all the RUN lines. `--export-dynamic` is preferred just due to its brevity. If that is the case, I think this deserves a comment considering its subtle interaction with `noexportdynamic`.

Right, that's why for one of the tests where I'm using the new noexportdynamic value I switched to the -u sequence instead. I can add a comment.

> - When is devirtualization invalid? For example, if `_ZTV1D` is exported to the dynamic symbol table and a shared object inherits from class D and overrides the method?

Correct. If it is both exported and then overridden.

> If my last point is correct, I'd agree that we need something like a tri-state option. (I hope we can remove `--lto-whole-program-visibility` if possible)
> `on/noexportdynamic` the value names do not capture the actual meaning. The `on` actually means: devirtualization is safe as long as the used `_ZTV*` symbols are not exported.

Suggestion on the name? It's basically an assertion, i.e. that there is LTO whole program visibility (no, when --export-dynamic not specified, and yes). 'on' means the user is asserting that the _ZTV* are not going to be exported and overridden.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91583