[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

[Oliver Stannard (Linaro) via Phabricator via llvm-commits]

ostannard added a comment.

In that example, with everything having default ELF visibility, all of the vtables will get vcall_visibility public, which can't be optimised by VFE, and won't ever be relaxed to one of the tighter visibilities.

The cases which can be optimised are (assuming "visibility" means visibility of the most-visible dynamic base class):

- Classes in an anonymous namespace, which aren't visible outside their translation unit. Probably doesn't happen very often, but can be optimised without LTO.
- Classes visible outside the translation unit, but not outside the LTO unit. This generally means hidden ELF visibility, unless the lto_visibility_public attribute is used. These can't be optimised without LTO, but can be with it.

I implemented the second case by adding the LinkageUnit visibility, which can't be optimised by VFE, but can be reduced to TranslationUnit when LTO internalisation happens. This could also have been done by not changing the visibility at LTO time, and instead leting GlobalDCE know if it is running post-LTO. Both of these should give the same results, but the way I used represents the visibility fully in the IR, without having the extra state of "are we doing LTO?".

I also don't think it matters here whether the DSO is loaded by dlopen or by being linked against it. Is there something I've missed here, or was dlopen not relevant to your example.

> Have you checked the assembly to see whether an unused CFI check is being emitted?

Not yet, I wanted to make sure that this optimisation was valid and correctly implemented before tracking down the performance regressions.



================
Comment at: llvm/lib/Transforms/IPO/GlobalDCE.cpp:183
+    // unit, we know that we can see all virtual functions which might use it,
+    // so VFE is safe.
+    if (auto GO = dyn_cast<GlobalObject>(&GV)) {
----------------
pcc wrote:
> Not necessarily, at least in this implementation, because "vtable symbol can be internalized" doesn't imply "all vcalls are visible". See main response.
This is checking the vcall_visibility, which will only be VCallVisibilityTranslationUnit if all base classes which contain virtual functions are private to this translation unit, which does imply "all vcalls are visible".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63932