[LTO][WPD] Suppress WPD on a class if the LTO unit doesn't have the prevailing definition of this class or any of its superclass #131721

mingmingl-llvm · 2025-03-18T03:58:03Z

Before this patch, whole program devirtualization is suppressed on a class if any superclass is visible to regular object files, by recording the class GUID in VisibleToRegularObjSymbols.

This patch suppresses whole program devirtualization on a class if the LTO unit doesn't have the prevailing definition of this class or any of its superclass (e.g., the prevailing definition is in a shared library)

Implementation summaries:

In llvm/lib/LTO/LTO.cpp, IsVisibleToRegularObj is updated to look at the global resolution's IsPrevailing bit for ThinLTO and regularLTO.
In llvm/tools/llvm-lto2/llvm-lto2.cpp,
- three command line options are added so llvm-lto2 can override Conf.HasWholeProgramVisibility, Conf.ValidateAllVtablesHaveTypeInfos and Conf.AllVtablesHaveTypeInfos.

The test case is reduced from a small C++ program (main.cc, lib.cc/h pasted below in [1]). To reproduce the program failure without this patch, compile lib.cc into a shared library, and provide it to a ThinLTO build of main.cc (commands are pasted in [2]).

[1]

lib.h

#include <cstdio>

class Derived {
 public:
  void dispatch();
  virtual void print();
  virtual void sum();
};

void Derived::dispatch() {
  static_cast<Derived*>(this)->print();
  static_cast<Derived*>(this)->sum();
}

void Derived::sum() {
  printf("Derived::sum\n");
}

__attribute__((noinline)) void* create(int i);
__attribute__((noinline)) void* getPtr(int i);

lib.cc

#include "lib.h"
#include <cstdio>
#include <iostream>

class Derived2 : public Derived {
public:
  void print() override {
    printf("DerivedSharedLib\n");
  }
  void sum() override {
    printf("DerivedSharedLib::sum\n");
  }
};


void Derived::print() {
  printf("Derived\n");
}

__attribute__((noinline)) void* create(int i) {
  if (i & 1)
    return new Derived2();
  return new Derived();
}

main.cc

cat main.cc
#include "lib.h"

class DerivedN : public Derived {
 public:
};

__attribute__((noinline)) void* getPtr(int x) {
  return new DerivedN();
}


int main() {
  Derived*b = static_cast<Derived*>(create(201));
  b->dispatch();
  delete b;

  Derived* a = static_cast<Derived*>(getPtr(202));
  a->dispatch();
  delete a;
  return 0;
}

[2]

# compile lib.o in a shared library.
$ ./bin/clang++ -O2 -fPIC -c lib.cc -o lib.o
$ ./bin/clang++ -shared -o libdata.so lib.o

# Provide the shared library in `-ldata`
$ ./bin/clang++ -v -g -ldata --save-temps -fno-discard-value-names -Wl,-mllvm,-print-before=wholeprogramdevirt -Wl,-mllvm,-wholeprogramdevirt-check=trap -Rpass=wholeprogramdevirt -Wl,--lto-whole-program-visibility -Wl,--lto-validate-all-vtables-have-type-infos -mllvm -disable-icp=true -Wl,-mllvm,-disable-icp=false -flto=thin -fwhole-program-vtables  -fno-split-lto-unit -fuse-ld=lld   main.cc  -L . -o main >/tmp/wholeprogramdevirt.ir 2>&1

# Run the program hits a segmentation fault with  `-Wl,-mllvm,-wholeprogramdevirt-check=trap`

$ LD_LIBRARY_PATH=. ./main
DerivedSharedLib
Trace/breakpoint trap (core dumped)

WPD.

llvmbot · 2025-03-18T03:58:36Z

@llvm/pr-subscribers-lto

Author: Mingming Liu (mingmingl-llvm)

Changes

Before this patch, whole program devirtualization is suppressed on a class if any superclass is visible to regular object files, by recording the class GUID in VisibleToRegularObjSymbols.

This patch suppresses whole program devirtualization on a class if any superclass is visible (or escape to shared library) for index-based WPD.

Implementation summaries:

In llvm/lib/LTO/LTO.cpp, IsVisibleToRegularObj is updated to look at the global resolution's ExportDynamic bit.
In llvm/tools/llvm-lto2/llvm-lto2.cpp,
- three command line options are added so llvm-lto2 can override Conf.HasWholeProgramVisibility, Conf.ValidateAllVtablesHaveTypeInfos and Conf.AllVtablesHaveTypeInfos.
- -r parses d as ExportDynamic

The test case is reduced from a small C++ program (main.cc, lib.cc/h pasted below in [1]). To reproduce the program failure without this patch, compile lib.cc into a shared library, and provide it to a ThinLTO build of main.cc (commands are pasted in [2]).

[1]

lib.h

#include &lt;cstdio&gt;

class Derived {
 public:
  void dispatch();
  virtual void print();
  virtual void sum();
};

void Derived::dispatch() {
  static_cast&lt;Derived*&gt;(this)-&gt;print();
  static_cast&lt;Derived*&gt;(this)-&gt;sum();
}

void Derived::sum() {
  printf("Derived::sum\n");
}

__attribute__((noinline)) void* create(int i);
__attribute__((noinline)) void* getPtr(int i);

lib.cc

#include "lib.h"
#include &lt;cstdio&gt;
#include &lt;iostream&gt;

class Derived2 : public Derived {
public:
  void print() override {
    printf("DerivedSharedLib\n");
  }
  void sum() override {
    printf("DerivedSharedLib::sum\n");
  }
};


void Derived::print() {
  printf("Derived\n");
}

__attribute__((noinline)) void* create(int i) {
  if (i &amp; 1)
    return new Derived2();
  return new Derived();
}

main.cc

cat main.cc
#include "lib.h"

class DerivedN : public Derived {
 public:
};

__attribute__((noinline)) void* getPtr(int x) {
  return new DerivedN();
}


int main() {
  Derived*b = static_cast&lt;Derived*&gt;(create(201));
  b-&gt;dispatch();
  delete b;

  Derived* a = static_cast&lt;Derived*&gt;(getPtr(202));
  a-&gt;dispatch();
  delete a;
  return 0;
}

[2]

# compile lib.o in a shared library.
$ ./bin/clang++ -O2 -fPIC -c lib.cc -o lib.o
$ ./bin/clang++ -shared -o libdata.so lib.o

# Provide the shared library in `-ldata`
$ ./bin/clang++ -v -g -ldata --save-temps -fno-discard-value-names -Wl,-mllvm,-print-before=wholeprogramdevirt -Wl,-mllvm,-wholeprogramdevirt-check=trap -Rpass=wholeprogramdevirt -Wl,--lto-whole-program-visibility -Wl,--lto-validate-all-vtables-have-type-infos -mllvm -disable-icp=true -Wl,-mllvm,-disable-icp=false -flto=thin -fwhole-program-vtables  -fno-split-lto-unit -fuse-ld=lld   main.cc  -L . -o main &gt;/tmp/wholeprogramdevirt.ir 2&gt;&amp;1

# Run the program hits a segmentation fault with  `-Wl,-mllvm,-wholeprogramdevirt-check=trap`

$ LD_LIBRARY_PATH=. ./main
DerivedSharedLib
Trace/breakpoint trap (core dumped)

Full diff: https://github.com/llvm/llvm-project/pull/131721.diff

3 Files Affected:

(modified) llvm/lib/LTO/LTO.cpp (+1-1)
(added) llvm/test/ThinLTO/X86/nodevirt_exort_dynamic.ll (+111)
(modified) llvm/tools/llvm-lto2/llvm-lto2.cpp (+21)

diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index e895a46b8cd77..6ec2dc381a932 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1905,7 +1905,7 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
     auto IsVisibleToRegularObj = [&](StringRef name) {
       auto It = GlobalResolutions->find(name);
       return (It == GlobalResolutions->end() ||
-              It->second.VisibleOutsideSummary);
+              It->second.VisibleOutsideSummary || It->second.ExportDynamic);
     };
 
     getVisibleToRegularObjVtableGUIDs(ThinLTO.CombinedIndex,
diff --git a/llvm/test/ThinLTO/X86/nodevirt_exort_dynamic.ll b/llvm/test/ThinLTO/X86/nodevirt_exort_dynamic.ll
new file mode 100644
index 0000000000000..ea33291a75161
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/nodevirt_exort_dynamic.ll
@@ -0,0 +1,111 @@
+; RUN: rm -rf %t && mkdir %t && cd %t
+
+; Generate unsplit module with summary for ThinLTO index-based WPD.
+; RUN: opt -thinlto-bc -o summary.o %s
+
+; RUN: llvm-dis -o - summary.o
+
+;; TODO: Implement the fix for WPD in regular or hybrid LTO, and add test coverage.
+
+; Index based WPD
+; For `_ZTI7Derived`, the 'llvm-lto2' resolution arguments specifies  `VisibleOutsideSummary` as false
+; and `ExportDynamic` as false. The callsite inside @_ZN4Base8dispatchEv
+; got devirtualized.
+; RUN: llvm-lto2 run summary.o -save-temps -pass-remarks=. \
+; RUN:   -o tmp \
+; RUN:   --whole-program-visibility-enabled-in-lto=true \
+; RUN:   --validate-all-vtables-have-type-infos=true \
+; RUN:   --all-vtables-have-type-infos=true \
+; RUN:   -r=summary.o,__cxa_pure_virtual, \
+; RUN:   -r=summary.o,_ZN8DerivedNC2Ev,x \
+; RUN:   -r=summary.o,_ZN4Base8dispatchEv,px \
+; RUN:   -r=summary.o,_ZN7DerivedC2Ev, \
+; RUN:   -r=summary.o,_ZN8DerivedN5printEv,px \
+; RUN:   -r=summary.o,_ZTS4Base, \
+; RUN:   -r=summary.o,_ZTV8DerivedN,p \
+; RUN:   -r=summary.o,_ZTI8DerivedN,p \
+; RUN:   -r=summary.o,_ZTI4Base, \
+; RUN:   -r=summary.o,_ZTS8DerivedN,p \
+; RUN:   -r=summary.o,_ZTI7Derived, \
+; RUN:   -r=summary.o,_ZTV4Base 2>&1 | FileCheck --allow-empty %s --check-prefix=REMARK
+
+; REMARK: single-impl: devirtualized a call to _ZN8DerivedN5printEv 
+
+; Index based WPD
+; For `_ZTI7Derived`, the 'llvm-lto2' resolution arguments specifies  `VisibleOutsideSummary` as false
+; and `ExportDynamic` as true. The callsite inside @_ZN4Base8dispatchEv won't
+; get devirtualized.
+; RUN: llvm-lto2  run summary.o -save-temps -pass-remarks=. \
+; RUN:   -o tmp \
+; RUN:   --whole-program-visibility-enabled-in-lto=true \
+; RUN:   --validate-all-vtables-have-type-infos=true \
+; RUN:   --all-vtables-have-type-infos=true \
+; RUN:   -r=summary.o,__cxa_pure_virtual, \
+; RUN:   -r=summary.o,_ZN8DerivedNC2Ev,x \
+; RUN:   -r=summary.o,_ZN4Base8dispatchEv,px \
+; RUN:   -r=summary.o,_ZN7DerivedC2Ev, \
+; RUN:   -r=summary.o,_ZN8DerivedN5printEv,px \
+; RUN:   -r=summary.o,_ZTS4Base, \
+; RUN:   -r=summary.o,_ZTV8DerivedN,p \
+; RUN:   -r=summary.o,_ZTI8DerivedN,p \
+; RUN:   -r=summary.o,_ZTI4Base, \
+; RUN:   -r=summary.o,_ZTS8DerivedN,p \
+; RUN:   -r=summary.o,_ZTI7Derived,d \
+; RUN:   -r=summary.o,_ZTV4Base 2>&1 | FileCheck %s --allow-empty --implicit-check-not='single-impl: devirtualized a call to'
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@_ZTV8DerivedN = linkonce_odr hidden constant { [3 x ptr] } { [3 x ptr] [ptr null, ptr @_ZTI8DerivedN, ptr @_ZN8DerivedN5printEv] }, !type !0, !type !1, !type !2, !type !3, !type !4, !type !5, !vcall_visibility !6
+@_ZTI8DerivedN = linkonce_odr hidden constant { ptr, ptr, ptr } { ptr null, ptr @_ZTS8DerivedN, ptr @_ZTI7Derived }
+@_ZTS8DerivedN = linkonce_odr hidden constant [10 x i8] c"8DerivedN\00", align 1
+@_ZTI7Derived = external constant ptr
+@_ZTV4Base = linkonce_odr hidden constant { [3 x ptr] } { [3 x ptr] [ptr null, ptr @_ZTI4Base, ptr @__cxa_pure_virtual] }, !type !0, !type !1, !vcall_visibility !6
+@_ZTI4Base = linkonce_odr hidden constant { ptr, ptr } { ptr null, ptr @_ZTS4Base }
+@_ZTS4Base = linkonce_odr hidden constant [6 x i8] c"4Base\00", align 1
+
+@llvm.used = appending global [1 x ptr] [ptr @_ZN8DerivedNC2Ev], section "llvm.metadata"
+
+define hidden void @_ZN4Base8dispatchEv(ptr %this) {
+entry:
+  %this.addr = alloca ptr
+  store ptr %this, ptr %this.addr
+  %this1 = load ptr, ptr %this.addr
+  %vtable = load ptr, ptr %this1
+  %0 = call i1 @llvm.type.test(ptr %vtable, metadata !"_ZTS7Derived")
+  call void @llvm.assume(i1 %0)
+  %vfn = getelementptr inbounds ptr, ptr %vtable, i64 0
+  %1 = load ptr, ptr %vfn
+  call void %1(ptr %this1)
+  ret void
+}
+
+define linkonce_odr hidden void @_ZN8DerivedNC2Ev(ptr %this) #0 {
+entry:
+  %this.addr = alloca ptr
+  store ptr %this, ptr %this.addr
+  %this1 = load ptr, ptr %this.addr
+  call void @_ZN7DerivedC2Ev(ptr %this1)
+  store ptr getelementptr inbounds inrange(-16, 8) ({ [3 x ptr] }, ptr @_ZTV8DerivedN, i32 0, i32 0, i32 2), ptr %this1
+  ret void
+}
+
+define linkonce_odr hidden void @_ZN8DerivedN5printEv(ptr %this) #0 {
+entry:
+  ret void
+}
+
+attributes #0 = { noinline optnone }
+
+declare void @__cxa_pure_virtual()
+declare i1 @llvm.type.test(ptr, metadata)
+declare void @llvm.assume(i1)
+declare void @_ZN7DerivedC2Ev(ptr)
+
+!0 = !{i64 16, !"_ZTS4Base"}
+!1 = !{i64 16, !"_ZTSM4BaseFvvE.virtual"}
+!2 = !{i64 16, !"_ZTS7Derived"}
+!3 = !{i64 16, !"_ZTSM7DerivedFvvE.virtual"}
+!4 = !{i64 16, !"_ZTS8DerivedN"}
+!5 = !{i64 16, !"_ZTSM8DerivedNFvvE.virtual"}
+!6 = !{i64 0}
diff --git a/llvm/tools/llvm-lto2/llvm-lto2.cpp b/llvm/tools/llvm-lto2/llvm-lto2.cpp
index dbb7f2f3028aa..0317a610a25a3 100644
--- a/llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ b/llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -187,6 +187,18 @@ static cl::opt<bool> EnableFreestanding(
     cl::desc("Enable Freestanding (disable builtins / TLI) during LTO"),
     cl::Hidden);
 
+static cl::opt<bool> WholeProgramVisibilityEnabledInLTO(
+    "whole-program-visibility-enabled-in-lto",
+    cl::desc("Enable whole program visibility during LTO"), cl::Hidden);
+
+static cl::opt<bool> ValidateAllVtablesHaveTypeInfos(
+    "validate-all-vtables-have-type-infos",
+    cl::desc("Validate that all vtables have type infos in LTO"), cl::Hidden);
+
+static cl::opt<bool>
+    AllVtablesHaveTypeInfos("all-vtables-have-type-infos", cl::Hidden,
+                            cl::desc("All vtables have type infos"));
+
 extern cl::opt<cl::boolOrDefault> LoadBitcodeIntoNewDbgInfoFormat;
 extern cl::opt<cl::boolOrDefault> PreserveInputDbgFormat;
 
@@ -257,6 +269,8 @@ static int run(int argc, char **argv) {
         Res.VisibleToRegularObj = true;
       else if (C == 'r')
         Res.LinkerRedefined = true;
+      else if (C == 'd')
+        Res.ExportDynamic = true;
       else {
         llvm::errs() << "invalid character " << C << " in resolution: " << R
                      << '\n';
@@ -332,6 +346,13 @@ static int run(int argc, char **argv) {
   Conf.PTO.LoopVectorization = Conf.OptLevel > 1;
   Conf.PTO.SLPVectorization = Conf.OptLevel > 1;
 
+  if (WholeProgramVisibilityEnabledInLTO.getNumOccurrences() > 0)
+    Conf.HasWholeProgramVisibility = WholeProgramVisibilityEnabledInLTO;
+  if (ValidateAllVtablesHaveTypeInfos.getNumOccurrences() > 0)
+    Conf.ValidateAllVtablesHaveTypeInfos = ValidateAllVtablesHaveTypeInfos;
+  if (AllVtablesHaveTypeInfos.getNumOccurrences() > 0)
+    Conf.AllVtablesHaveTypeInfos = AllVtablesHaveTypeInfos;
+
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
     Backend = createWriteIndexesThinBackend(llvm::hardware_concurrency(Threads),

teresajohnson

The test name has a typo (missing 'p' in nodevirt_exort_dynamic.ll)

teresajohnson · 2025-03-18T04:30:54Z

llvm/lib/LTO/LTO.cpp

      auto It = GlobalResolutions->find(name);
      return (It == GlobalResolutions->end() ||
-              It->second.VisibleOutsideSummary);
+              It->second.VisibleOutsideSummary || It->second.ExportDynamic);


The same fix should presumably go in for regular LTO (see the other def of IsVisibleToRegularObj in this file).

done, and added test coverage for hybrid LTO. Regular LTO test coverage is not very meaningful, since without this change, the VisibleOutsideSummary bit suppresses devirtualization with -Wl,--lto-whole-program-visibility -Wl,--lto-validate-all-vtables-have-type-infos in lld

Per offline discussion, opt -module-summary is the way to create LTO input for regular LTO. Now the regression test has coverage for {thin, hybrid, regular} LTO.

teresajohnson · 2025-03-18T04:44:01Z

llvm/test/ThinLTO/X86/nodevirt_exort_dynamic.ll

+@_ZTV8DerivedN = linkonce_odr hidden constant { [3 x ptr] } { [3 x ptr] [ptr null, ptr @_ZTI8DerivedN, ptr @_ZN8DerivedN5printEv] }, !type !0, !type !1, !type !2, !type !3, !type !4, !type !5, !vcall_visibility !6
+@_ZTI8DerivedN = linkonce_odr hidden constant { ptr, ptr, ptr } { ptr null, ptr @_ZTS8DerivedN, ptr @_ZTI7Derived }
+@_ZTS8DerivedN = linkonce_odr hidden constant [10 x i8] c"8DerivedN\00", align 1
+@_ZTI7Derived = external constant ptr


Thinking more about this...presumably the GlobalResolution Prevailing flag and isPrevailingIRSymbol are false for _ZTI7Derived. Should we more broadly be returning true to IsVisibleToRegularObj for that case?

Good question about the prevailing bit. Let me think about it more and implement the change if needed as a separate patch.

As discussed offline, I'll look into whether checking Prevailing bit is a better alternative to ExportDynamic.

I updated LTO.cpp to use IsPrevailing bit since it's semantically correct to do that.

mingmingl-llvm · 2025-03-18T18:19:21Z

The test name has a typo (missing 'p' in nodevirt_exort_dynamic.ll)

Whoops! Fixed the test name.

regression test

teresajohnson

A few minor comments on reducing the test a bit more. Also, I don't think the llvm-lto2 changes are needed, as noted in the comments there.

teresajohnson · 2025-03-20T03:32:02Z

llvm/test/ThinLTO/X86/devirt_prevailing.ll

+; RUN: rm -rf %t && mkdir %t && cd %t
+
+; Tests that devirtualization is suppressed on a class when the LTO unit doesn't
+; have the prevailing definition of the class.


"of the base class" maybe? Here it is suppressing devirt on DerivedN because base class Derived isn't prevailing in the LTO unit I think?

teresajohnson · 2025-03-20T03:33:58Z

llvm/test/ThinLTO/X86/devirt_prevailing.ll

+!1 = !{i64 16, !"_ZTSM7DerivedFvvE.virtual"}
+!2 = !{i64 16, !"_ZTS8DerivedN"}
+!3 = !{i64 16, !"_ZTSM8DerivedNFvvE.virtual"}
+;!4 = !{i64 16, !"_ZTS8DerivedN"}


Remove the commented out metadata?

teresajohnson · 2025-03-20T03:34:58Z

llvm/test/ThinLTO/X86/devirt_prevailing.ll

+
+attributes #0 = { noinline optnone }
+
+declare void @__cxa_pure_virtual()


Remove this decl?

teresajohnson · 2025-03-20T03:35:10Z

llvm/test/ThinLTO/X86/devirt_prevailing.ll

+declare void @__cxa_pure_virtual()
+declare i1 @llvm.type.test(ptr, metadata)
+declare void @llvm.assume(i1)
+declare void @_ZN7DerivedC2Ev(ptr)


Remove this decl once the reference above is removed?

The purpose of adding this function to llvm.used is to keep vtable live.

Upon this comment, it's simpler to adding the vtable itself to llvm.used for this purpose.

So I removed this function and the other function decl to simplify the test case.

teresajohnson · 2025-03-20T03:38:21Z

llvm/tools/llvm-lto2/llvm-lto2.cpp

  Conf.PTO.SLPVectorization = Conf.OptLevel > 1;

+  if (WholeProgramVisibilityEnabledInLTO.getNumOccurrences() > 0)
+    Conf.HasWholeProgramVisibility = WholeProgramVisibilityEnabledInLTO;


I think the test can just use the existing -whole-program-visibility flag (defined in WholeProgramDevirt.cpp) which is OR'ed with this config value, instead of adding a new llvm-lto2 flag.

Yeah I spent some time looking into reusing -whole-program-visibility flag before introducing new options in llvm-lto2, but -whole-program-visibility itself doesn't initialize the VisibleToRegularObjSymbols set, which are used to skip vtable visibility upgrade.

Basically, the condition (

llvm-project/llvm/lib/LTO/LTO.cpp

Lines 1901 to 1902 in d0d33d2

if (WholeProgramVisibilityEnabledInLTO &&

Conf.ValidateAllVtablesHaveTypeInfos) {

) needs both WholeProgramVisibilityEnabledInLTO and Conf.ValidateAllVtablesHaveTypeInfos to get DenseSet<GlobalValue::GUID> VisibleToRegularObjSymbols, and WholeProgramVisibilityEnabledInLTO value is based on configuration value (

llvm-project/llvm/lib/LTO/LTO.cpp

Lines 1889 to 1893 in d0d33d2

bool WholeProgramVisibilityEnabledInLTO =

Conf.HasWholeProgramVisibility &&

// If validation is enabled, upgrade visibility only when all vtables

// have typeinfos.

(!Conf.ValidateAllVtablesHaveTypeInfos || Conf.AllVtablesHaveTypeInfos);

)

I tried to use -whole-program-visibility [1] flag in the second llvm-lto2 RUN command (which is expected to see no devirtualization), and the test failed..

[1]

--- a/llvm/test/ThinLTO/X86/devirt_prevailing.ll +++ b/llvm/test/ThinLTO/X86/devirt_prevailing.ll @@ -32,9 +32,7 @@ ; RUN: llvm-lto2 run summary.o -save-temps -pass-remarks=. \ ; RUN: -thinlto-threads=1 \ ; RUN: -o tmp \ -; RUN: --whole-program-visibility-enabled-in-lto=true \ -; RUN: --validate-all-vtables-have-type-infos=true \ -; RUN: --all-vtables-have-type-infos=true \ +; RUN: -whole-program-visibility \ ; RUN: -r=summary.o,_ZN4Base8dispatchEv,px \ ; RUN: -r=summary.o,_ZN8DerivedN5printEv,px \ ; RUN: -r=summary.o,_ZTV8DerivedN,p \

teresajohnson · 2025-03-20T03:40:42Z

llvm/tools/llvm-lto2/llvm-lto2.cpp

+  if (WholeProgramVisibilityEnabledInLTO.getNumOccurrences() > 0)
+    Conf.HasWholeProgramVisibility = WholeProgramVisibilityEnabledInLTO;
+  if (ValidateAllVtablesHaveTypeInfos.getNumOccurrences() > 0)
+    Conf.ValidateAllVtablesHaveTypeInfos = ValidateAllVtablesHaveTypeInfos;


I don't think either of these are needed for the test as ValidateAllVtablesHaveTypeInfos defaults to false, meaning we don't do this validation, and AllVtablesHaveTypeInfos is only checked if ValidateAllVtablesHaveTypeInfos is true.

teresajohnson · 2025-03-20T03:41:00Z

llvm/test/ThinLTO/X86/devirt_prevailing.ll

+  ret void
+}
+
+define linkonce_odr hidden void @_ZN8DerivedNC2Ev(ptr %this) #0 {


Can this function be removed from the test? It doesn't seem to be referenced other than in the @llvm.used (which could then presumably also be removed).

teresajohnson

lgtm

Propagate ExportDynamic from base type to compatible derived types in

40f913c

WPD.

llvmbot added the LTO Link time optimization (regular/full LTO or ThinLTO) label Mar 18, 2025

mingmingl-llvm requested a review from teresajohnson March 18, 2025 03:58

teresajohnson reviewed Mar 18, 2025

View reviewed changes

implement the fix for hybrid LTO, and update test cases

6be4464

mingmingl-llvm changed the title ~~[ThinLTO][WPD] Suppress WPD on a class if any superclass is visible in a shared library for index-based WPD~~ [ThinLTO][WPD] Suppress WPD on a class if any superclass is visible in a shared library for index-based and hybrid WPD Mar 18, 2025

mingmingl-llvm requested a review from teresajohnson March 18, 2025 18:19

Look at the Prevailing bit as opposed to ExportDynamic bit, update

dfe5970

regression test

mingmingl-llvm changed the title ~~[ThinLTO][WPD] Suppress WPD on a class if any superclass is visible in a shared library for index-based and hybrid WPD~~ [ThinLTO][WPD] Suppress WPD on a class if the LTO unit doesn't have the prevailing bit Mar 19, 2025

add test coverage for regular LTO

27b1e88

mingmingl-llvm changed the title ~~[ThinLTO][WPD] Suppress WPD on a class if the LTO unit doesn't have the prevailing bit~~ [LTO][WPD] Suppress WPD on a class if the LTO unit doesn't have the prevailing definition of this class Mar 19, 2025

teresajohnson reviewed Mar 20, 2025

View reviewed changes

resolve comments

f12edd1

mingmingl-llvm requested a review from teresajohnson March 20, 2025 04:25

teresajohnson approved these changes Mar 20, 2025

View reviewed changes

mingmingl-llvm merged commit d99033e into llvm:main Mar 20, 2025
8 of 10 checks passed

mingmingl-llvm changed the title ~~[LTO][WPD] Suppress WPD on a class if the LTO unit doesn't have the prevailing definition of this class~~ [LTO][WPD] Suppress WPD on a class if the LTO unit doesn't have the prevailing definition of this class or any of its superclass Mar 20, 2025

mingmingl-llvm deleted the shared-lib-wpd branch March 20, 2025 05:12


		attributes #0 = { noinline optnone }

		declare void @__cxa_pure_virtual()

	if (WholeProgramVisibilityEnabledInLTO &&
	Conf.ValidateAllVtablesHaveTypeInfos) {

	bool WholeProgramVisibilityEnabledInLTO =
	Conf.HasWholeProgramVisibility &&
	// If validation is enabled, upgrade visibility only when all vtables
	// have typeinfos.
	(!Conf.ValidateAllVtablesHaveTypeInfos \|\| Conf.AllVtablesHaveTypeInfos);

[LTO][WPD] Suppress WPD on a class if the LTO unit doesn't have the prevailing definition of this class or any of its superclass #131721

[LTO][WPD] Suppress WPD on a class if the LTO unit doesn't have the prevailing definition of this class or any of its superclass #131721

Uh oh!

Conversation

mingmingl-llvm commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 18, 2025

Uh oh!

teresajohnson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mingmingl-llvm commented Mar 18, 2025

Uh oh!

teresajohnson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teresajohnson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mingmingl-llvm commented Mar 18, 2025 •

edited

Loading