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

[MaskRay]

Summary

A local: version node in a version script can change the effective symbol binding
to STB_LOCAL. The linker needs to communicate the fact to enable WPD
(otherwise LTO does not know that the !vcall_visibility metadata has
effectively changed from VCallVisibilityPublic to VCallVisibilityLinkageUnit).

[tejohnson]

lld/test/ELF/lto/devirt_vcall_vis_hidden.ll

25 ↗

(On Diff #329140)

These hidden symbols should start out with Linkage Unit LTO visibility (CodeGenModule::GetVCallVisibilityLevel)

[lanza]

Thanks Fangrui! A cherry-pick and test and local run makes it seem like it works.

[lanza]

lld/test/ELF/lto/devirt_vcall_vis_hidden.ll

25 ↗

(On Diff #329140)

Yea, this shouldn't have !vcall_visibility. A version script could be used here to say that _ZTV1B is global but _ZTV1A is local.

{
    global:
    _ZTV1B;
    local:
    *;
};

[tejohnson]

lld/test/ELF/lto/devirt_vcall_vis_hidden.ll

25 ↗

(On Diff #329140)

Well right now they are marked with public visibility, which is the same thing as not having any vcall_visibility. Since they are marked in the IR as having hidden visibility, clang presumably should have marked them with a more restricted vcall_visibility (linkage unit level).

[MaskRay]

Test --version-script

[lanza]

Well right now they are marked with public visibility, which is the same thing as not having any vcall_visibility. Since they are marked in the IR as having hidden visibility, clang presumably should have marked them with a more restricted vcall_visibility (linkage unit level).

Hmm? You first said "they are marked with public visibility" and then said "they are marked in the IR as having hidden visibility." I don't follow. A __attribute__(visibility("hidden"))) symbol gets the hidden attr.

[lanza]

lld/test/ELF/lto/devirt_vcall_vis_localize.ll

3

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.

13

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.

[tejohnson]

In D98220#2613098, @lanza wrote:

Well right now they are marked with public visibility, which is the same thing as not having any vcall_visibility. Since they are marked in the IR as having hidden visibility, clang presumably should have marked them with a more restricted vcall_visibility (linkage unit level).

Hmm? You first said "they are marked with public visibility" and then said "they are marked in the IR as having hidden visibility." I don't follow. A __attribute__(visibility("hidden"))) symbol gets the hidden attr.

An older version of the patch had the vtables marked "hidden" in the LLVM assembly. My point was that clang would have given them a non-public vcall_visibility in that case. The patch and the test case have changed though, so that now the local binding comes from the linker version script and not the input IR, which is more consistent with the case I believe you are trying to handle?

[MaskRay]

@lanza I did not know that -flto -fvisibility=hidden -fwhole-program-vtables -fvirtual-function-elimination create s`!vcall_visibility VCallVisibilityLinkageUnit`.
With VCallVisibilityLinkageUnit, WPD will be performed, no need for this change (test/ThinLTO/X86/devirt_vcall_vis_hidden.ll).
This change specifically handles localization via a local: version node in a version script, where WPD was not performed previously.

[lanza]

An older version of the patch had the vtables marked "hidden" in the LLVM assembly. My point was that clang would have given them a non-public vcall_visibility in that case.

Ahh, sorry, I missed that!

The patch and the test case have changed though, so that now the local binding comes from the linker version script and not the input IR, which is more consistent with the case I believe you are trying to handle?

Correct.

[MaskRay]

lld/test/ELF/lto/devirt_vcall_vis_localize.ll

13

!vcall_visibility is here to better reflect the clang codegen.

[tejohnson]

lgtm

lld/ELF/LTO.cpp

for (size_t i = 0, e = syms.size(); i != e; ++i) {
     // 2) Symbols that are used in regular objects.
     // 3) C named sections if we have corresponding __start_/__stop_ symbol.
     // 4) Symbols that are defined in bitcode files and used for dynamic linking.
     r.VisibleToRegularObj = config->relocatable || sym->isUsedInRegularObj ||
                             (r.Prevailing && sym->includeInDynsym()) ||
                             usedStartStop.count(objSym.getSectionName());
     // Identify symbols exported dynamically, and that therefore could be
     // referenced by a shared library not visible to the linker.
-    r.ExportDynamic = sym->isExportDynamic(sym->kind(), sym->visibility) ||
-                      sym->exportDynamic || sym->inDynamicList;
+    r.ExportDynamic = sym->computeBinding() != STB_LOCAL &&
+                      (sym->isExportDynamic(sym->kind(), sym->visibility) ||
+                       sym->exportDynamic || sym->inDynamicList);
     const auto *dr = dyn_cast<Defined>(sym);
     r.FinalDefinitionInLinkageUnit =
         (isExec || sym->visibility != STV_DEFAULT) && dr &&
         // Skip absolute symbols from ELF objects, otherwise PC-rel relocations
         // will be generated by for them, triggering linker errors.
         // Symbol section is always null for bitcode symbols, hence the check
         // for isElf(). Skip linker script defined symbols as well: they have
         // no File defined.

lld/test/ELF/lto/devirt_vcall_vis_localize.ll

+; REQUIRES: x86
+;; If the vtable symbols are localized by a version script, devirtualization
+;; can still happen.
+
+; RUN: opt --thinlto-bc -o %t1.o %s
+; RUN: opt --thinlto-bc -o %t2.o %S/Inputs/devirt_vcall_vis_shared_def.ll
+; RUN: echo '{ global: _start; local: *; };' > %t.ver
+
+; RUN: ld.lld %t1.o %t2.o -o %t.out --save-temps --lto-whole-program-visibility -shared \
+; RUN:   -mllvm -pass-remarks=. 2>&1 | count 0
+
+; 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
+; RUN: llvm-dis < %t1.o.4.opt.bc | FileCheck %s --check-prefix=CHECK-IR
+
+; REMARK: single-impl: devirtualized a call to _ZN1A1nEi
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-grtev4-linux-gnu"
+
+%struct.A = type { i32 (...)** }
+%struct.B = type { %struct.A }
+
+@_ZTV1A = available_externally unnamed_addr constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !vcall_visibility !2
+@_ZTV1B = linkonce_odr unnamed_addr constant { [4 x i8*] } { [4 x i8*] [i8* null, i8* undef, i8* bitcast (i32 (%struct.B*, i32)* @_ZN1B1fEi to i8*), i8* bitcast (i32 (%struct.A*, i32)* @_ZN1A1nEi to i8*)] }, !type !0, !type !1, !vcall_visibility !2
+
+;; Prevent the vtables from being dead code eliminated.
[e-mail] = appending global [2 x i8*] [ i8* bitcast ( { [4 x i8*] }* @_ZTV1A to i8*), i8* bitcast ( { [4 x i8*] }* @_ZTV1B to i8*)]
+
+; CHECK-IR-LABEL: @_start(
+define i32 @_start(%struct.A* %obj, i32 %a) {
+entry:
+  %0 = bitcast %struct.A* %obj to i8***
+  %vtable = load i8**, i8*** %0
+  %1 = bitcast i8** %vtable to i8*
+  %p = call i1 @llvm.type.test(i8* %1, metadata !"_ZTS1A")
+  call void @llvm.assume(i1 %p)
+  %fptrptr = getelementptr i8*, i8** %vtable, i32 1
+  %2 = bitcast i8** %fptrptr to i32 (%struct.A*, i32)**
+  %fptr1 = load i32 (%struct.A*, i32)*, i32 (%struct.A*, i32)** %2, align 8
+
+  ;; Check that the call was devirtualized.
+  ; CHECK-IR: %call = tail call i32 @_ZN1A1nEi
+  %call = tail call i32 %fptr1(%struct.A* nonnull %obj, i32 %a)
+
+  ret i32 %call
+}
+; CHECK-IR-LABEL: ret i32
+; CHECK-IR-LABEL: }
+
+declare i1 @llvm.type.test(i8*, metadata)
+declare void @llvm.assume(i1)
+
+define available_externally i32 @_ZN1A1fEi(%struct.A* %this, i32 %a) #0 {
+   ret i32 0
+}
+
+define available_externally i32 @_ZN1A1nEi(%struct.A* %this, i32 %a) #0 {
+   ret i32 0
+}
+
+define linkonce_odr i32 @_ZN1B1fEi(%struct.B* %this, i32 %a) #0 {
+   ret i32 0
+}
+
+;; Make sure we don't inline or otherwise optimize out the direct calls.
+attributes #0 = { noinline optnone }
+
+!0 = !{i64 16, !"_ZTS1A"}
+!1 = !{i64 16, !"_ZTS1B"}
+!2 = !{i64 0}