[PATCH] D98220: [WPD][ELF] Allow whole program devirtualization for version script localized symbols

Tue Mar 9 00:10:03 PST 2021

lanza added inline comments.

================
Comment at: lld/test/ELF/lto/devirt_vcall_vis_localize.ll:2
+; REQUIRES: x86
+;; If the vtable symbols are localized by a version script, devirtualization
+;; can still happen.
----------------
So this test and description works fine, but the case I was originally inquiring about is an additional usage.

This version script that you have below marks `_ZTV1A` as `local` and thus changes the `versionId` to `STB_LOCAL`. (`SymbolTable::assignWildcardVersion`)

Your change in this diff causes `r.ExportDynamic` to be false thanks to the `STB_LOCAL` which is later propagated to llvm's LTO `GlobalResolutions` directly. 

This stops the vtable from being put in the `DynamicExportSymbols` set. (`LTO::run` at the top).

This lets the `vcall_visiblity` metadata get applied to the vtable in `updateVCallVisibilityInModule` `WholeProgramDevirt.cpp`. And thus the devirtualization now happens even though the vtbl was not hidden to start.

================
Comment at: lld/test/ELF/lto/devirt_vcall_vis_localize.ll:12
+
+; RUN: ld.lld %t1.o %t2.o -o %t.out --save-temps --lto-whole-program-visibility -shared \
+; RUN:   --version-script=%t.ver -mllvm -pass-remarks=. 2>&1 | FileCheck %s --check-prefix=REMARK
----------------
You can remove the `vcall_visibility` here and the `--lto-whole-program-visibility` combined with the version script will allow the `vcall_visibility` to be applied. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98220