[clang][RelativeVTables] Make the rtti_proxy LinkOnceODR instead of External linkage #67755
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen ChangesThis way, it the rtti_proxies can be candidates for being replaced altogether with GOTPCREL relocations because they are discardable. Functionally, this shouldn't change the final ELF linkage of the proxies. Full diff: https://github.com/llvm/llvm-project/pull/67755.diff 7 Files Affected:
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 23cfcdd138439f0..1eec9ab29133f5c 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -639,8 +639,16 @@ void CodeGenVTables::addRelativeComponent(ConstantArrayBuilder &builder,
// want the stub/proxy to be emitted for properly calculating the offset.
// Examples where there would be no symbol emitted are available_externally
// and private linkages.
- auto stubLinkage = vtableHasLocalLinkage ? llvm::GlobalValue::InternalLinkage
- : llvm::GlobalValue::ExternalLinkage;
+ //
+ // `internal` linkage results in LOCAL Elf linkage while still manifesting a
+ // local symbol.
+ //
+ // `linkonce_odr` linkage results in a DEFAULT Elf linkage but also allows for
+ // the rtti_proxy to be transparently replaced with a GOTPCREL reloc by a
+ // target that supports this replacement.
+ auto stubLinkage = vtableHasLocalLinkage
+ ? llvm::GlobalValue::InternalLinkage
+ : llvm::GlobalValue::LinkOnceODRLinkage;
llvm::Constant *target;
if (auto *func = dyn_cast<llvm::Function>(globalVal)) {
diff --git a/clang/test/CodeGenCXX/RelativeVTablesABI/child-vtable-in-comdat.cpp b/clang/test/CodeGenCXX/RelativeVTablesABI/child-vtable-in-comdat.cpp
index 48b1d8ed65d7e55..950921f67509f42 100644
--- a/clang/test/CodeGenCXX/RelativeVTablesABI/child-vtable-in-comdat.cpp
+++ b/clang/test/CodeGenCXX/RelativeVTablesABI/child-vtable-in-comdat.cpp
@@ -8,7 +8,6 @@
// CHECK-DAG: $_ZTS1B = comdat any
// CHECK-DAG: $_ZTI1B = comdat any
// CHECK-DAG: $_ZTI1B.rtti_proxy = comdat any
-// CHECK-DAG: $_ZTI1A.rtti_proxy = comdat any
// VTable for B is emitted here since we access it when creating an instance of B. The VTable is also linkonce_odr and in its own comdat.
// CHECK-DAG: @_ZTV1B.local = linkonce_odr hidden unnamed_addr constant { [3 x i32] } { [3 x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1B.rtti_proxy to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B.local, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @_ZN1B3fooEv to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B.local, i32 0, i32 0, i32 2) to i64)) to i32)] }, comdat($_ZTV1B), align 4
@@ -18,7 +17,7 @@
// CHECK-DAG: @_ZTS1B =
// CHECK-DAG: @_ZTI1A =
// CHECK-DAG: @_ZTI1B =
-// CHECK-DAG: @_ZTI1B.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1B, comdat
+// CHECK-DAG: @_ZTI1B.rtti_proxy = linkonce_odr hidden unnamed_addr constant ptr @_ZTI1B, comdat
// We will emit a vtable for B here, so it does have an alias, but we will not
// emit one for A.
diff --git a/clang/test/CodeGenCXX/RelativeVTablesABI/parent-vtable-in-comdat.cpp b/clang/test/CodeGenCXX/RelativeVTablesABI/parent-vtable-in-comdat.cpp
index 5f5f9edf411a809..ee710100152bfac 100644
--- a/clang/test/CodeGenCXX/RelativeVTablesABI/parent-vtable-in-comdat.cpp
+++ b/clang/test/CodeGenCXX/RelativeVTablesABI/parent-vtable-in-comdat.cpp
@@ -18,7 +18,7 @@
// CHECK: @_ZTVN10__cxxabiv117__class_type_infoE = external global [0 x ptr]
// CHECK: @_ZTS1A = linkonce_odr constant [3 x i8] c"1A\00", comdat, align 1
// CHECK: @_ZTI1A = linkonce_odr constant { ptr, ptr } { ptr getelementptr inbounds (i8, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i32 8), ptr @_ZTS1A }, comdat, align 8
-// CHECK: @_ZTI1A.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1A, comdat
+// CHECK: @_ZTI1A.rtti_proxy = linkonce_odr hidden unnamed_addr constant ptr @_ZTI1A, comdat
// CHECK: @_ZTV1A = linkonce_odr unnamed_addr alias { [3 x i32] }, ptr @_ZTV1A.local
// CHECK: define linkonce_odr void @_ZN1A3fooEv(ptr {{.*}}%this) unnamed_addr #{{[0-9]+}} comdat
diff --git a/clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp b/clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
index d0777b0f1245b65..7657a3bd0efdee4 100644
--- a/clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
+++ b/clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
@@ -6,7 +6,7 @@
/// hwasan-instrumented.
// CHECK-DAG: @_ZTV1A.local = private unnamed_addr constant { [3 x i32] } { [3 x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1A.rtti_proxy to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @_ZN1A3fooEv to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32)] }, no_sanitize_hwaddress, align 4
// CHECK-DAG: @_ZTV1A = unnamed_addr alias { [3 x i32] }, ptr @_ZTV1A.local
-// CHECK-DAG: @_ZTI1A.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1A, no_sanitize_hwaddress, comdat
+// CHECK-DAG: @_ZTI1A.rtti_proxy = linkonce_odr hidden unnamed_addr constant ptr @_ZTI1A, no_sanitize_hwaddress, comdat
class A {
public:
@@ -22,7 +22,7 @@ void A_foo(A *a) {
/// If the vtable happens to be hidden, then the alias is not needed. In this
/// case, the original vtable struct itself should be unsanitized.
// CHECK-DAG: @_ZTV1B = hidden unnamed_addr constant { [3 x i32] } { [3 x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1B.rtti_proxy to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @_ZN1B3fooEv to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32)] }, no_sanitize_hwaddress, align 4
-// CHECK-DAG: @_ZTI1B.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1B, no_sanitize_hwaddress, comdat
+// CHECK-DAG: @_ZTI1B.rtti_proxy = linkonce_odr hidden unnamed_addr constant ptr @_ZTI1B, no_sanitize_hwaddress, comdat
class __attribute__((visibility("hidden"))) B {
public:
diff --git a/clang/test/CodeGenCXX/RelativeVTablesABI/simple-vtable-definition.cpp b/clang/test/CodeGenCXX/RelativeVTablesABI/simple-vtable-definition.cpp
index 76de83d9d2cd466..5ed4745b9a0691d 100644
--- a/clang/test/CodeGenCXX/RelativeVTablesABI/simple-vtable-definition.cpp
+++ b/clang/test/CodeGenCXX/RelativeVTablesABI/simple-vtable-definition.cpp
@@ -14,7 +14,7 @@
// CHECK: @_ZTI1A ={{.*}} constant { ptr, ptr } { ptr getelementptr inbounds (i8, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i32 8), ptr @_ZTS1A }, align 8
// The rtti should be in a comdat
-// CHECK: @_ZTI1A.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1A, comdat
+// CHECK: @_ZTI1A.rtti_proxy = {{.*}}comdat
// The vtable symbol is exposed through an alias.
// @_ZTV1A = dso_local unnamed_addr alias { [3 x i32] }, ptr @_ZTV1A.local
diff --git a/clang/test/CodeGenCXX/RelativeVTablesABI/type-info.cpp b/clang/test/CodeGenCXX/RelativeVTablesABI/type-info.cpp
index 42f28a770e72770..bc10461187b758c 100644
--- a/clang/test/CodeGenCXX/RelativeVTablesABI/type-info.cpp
+++ b/clang/test/CodeGenCXX/RelativeVTablesABI/type-info.cpp
@@ -13,8 +13,8 @@
// CHECK: @_ZTVN10__cxxabiv120__si_class_type_infoE = external global [0 x ptr]
// CHECK: @_ZTS1B ={{.*}} constant [3 x i8] c"1B\00", align 1
// CHECK: @_ZTI1B ={{.*}} constant { ptr, ptr, ptr } { ptr getelementptr inbounds (i8, ptr @_ZTVN10__cxxabiv120__si_class_type_infoE, i32 8), ptr @_ZTS1B, ptr @_ZTI1A }, align 8
-// CHECK: @_ZTI1A.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1A, comdat
-// CHECK: @_ZTI1B.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1B, comdat
+// CHECK: @_ZTI1A.rtti_proxy = linkonce_odr hidden unnamed_addr constant ptr @_ZTI1A, comdat
+// CHECK: @_ZTI1B.rtti_proxy = linkonce_odr hidden unnamed_addr constant ptr @_ZTI1B, comdat
// CHECK: define {{.*}}ptr @_Z11getTypeInfov() local_unnamed_addr
// CHECK-NEXT: entry:
diff --git a/clang/test/CodeGenCXX/fixed-point-mangle.cpp b/clang/test/CodeGenCXX/fixed-point-mangle.cpp
new file mode 100644
index 000000000000000..103990a61316a9c
--- /dev/null
+++ b/clang/test/CodeGenCXX/fixed-point-mangle.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -triple=x86_64-unknown-linux-gnu | FileCheck %s
+
+// Primary fixed point types
+void func(signed short _Accum){} // CHECK: @_Z4funcDAs
+void func(signed _Accum){} // CHECK: @_Z4funcDAi
+void func(signed long _Accum){} // CHECK: @_Z4funcDAl
+void func(unsigned short _Accum){} // CHECK: @_Z4funcDAt
+void func(unsigned _Accum){} // CHECK: @_Z4funcDAj
+void func(unsigned long _Accum){} // CHECK: @_Z4funcDAm
+void func(signed short _Fract){} // CHECK: @_Z4funcDRs
+void func(signed _Fract){} // CHECK: @_Z4funcDRi
+void func(signed long _Fract){} // CHECK: @_Z4funcDRl
+void func(unsigned short _Fract){} // CHECK: @_Z4funcDRt
+void func(unsigned _Fract){} // CHECK: @_Z4funcDRj
+void func(unsigned long _Fract){} // CHECK: @_Z4funcDRm
+
+// Aliased
+void func2(short _Accum){} // CHECK: @_Z5func2DAs
+void func2(_Accum){} // CHECK: @_Z5func2DAi
+void func2(long _Accum){} // CHECK: @_Z5func2DAl
+void func2(short _Fract){} // CHECK: @_Z5func2DRs
+void func2(_Fract){} // CHECK: @_Z5func2DRi
+void func2(long _Fract){} // CHECK: @_Z5func2DRl
+
+// Primary saturated
+void func(_Sat signed short _Accum){} // CHECK: @_Z4funcDSDAs
+void func(_Sat signed _Accum){} // CHECK: @_Z4funcDSDAi
+void func(_Sat signed long _Accum){} // CHECK: @_Z4funcDSDAl
+void func(_Sat unsigned short _Accum){} // CHECK: @_Z4funcDSDAt
+void func(_Sat unsigned _Accum){} // CHECK: @_Z4funcDSDAj
+void func(_Sat unsigned long _Accum){} // CHECK: @_Z4funcDSDAm
+void func(_Sat signed short _Fract){} // CHECK: @_Z4funcDSDRs
+void func(_Sat signed _Fract){} // CHECK: @_Z4funcDSDRi
+void func(_Sat signed long _Fract){} // CHECK: @_Z4funcDSDRl
+void func(_Sat unsigned short _Fract){} // CHECK: @_Z4funcDSDRt
+void func(_Sat unsigned _Fract){} // CHECK: @_Z4funcDSDRj
+void func(_Sat unsigned long _Fract){} // CHECK: @_Z4funcDSDRm
+
+// Aliased saturated
+void func2(_Sat short _Accum){} // CHECK: @_Z5func2DSDAs
+void func2(_Sat _Accum){} // CHECK: @_Z5func2DSDAi
+void func2(_Sat long _Accum){} // CHECK: @_Z5func2DSDAl
+void func2(_Sat short _Fract){} // CHECK: @_Z5func2DSDRs
+void func2(_Sat _Fract){} // CHECK: @_Z5func2DSDRi
+void func2(_Sat long _Fract){} // CHECK: @_Z5func2DSDRl
|
|
The change to |
PiJoules
force-pushed
the
rv-linkonce_odr
branch
from
3ae7b8c
to
0e798f3
Compare
Woops. Yeah, removed. |
You can test this locally with the following command:git-clang-format --diff 427f120f60becd23d6e037ccd14104adde8a3af9 2607a99be986215fa27c053755e96c81bb2033e7 -- clang/lib/CodeGen/CGVTables.cpp clang/test/CodeGenCXX/RelativeVTablesABI/child-vtable-in-comdat.cpp clang/test/CodeGenCXX/RelativeVTablesABI/parent-vtable-in-comdat.cpp clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp clang/test/CodeGenCXX/RelativeVTablesABI/simple-vtable-definition.cpp clang/test/CodeGenCXX/RelativeVTablesABI/type-info.cppView the diff from clang-format here.diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 3cf88b1caa30..ced6a1c22553 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -200,9 +200,8 @@ CodeGenFunction::GenerateVarArgsThunk(llvm::Function *Fn,
// Find the first store of "this", which will be to the alloca associated
// with "this".
- Address ThisPtr =
- Address(&*AI, ConvertTypeForMem(MD->getThisObjectType()),
- CGM.getClassPointerAlignment(MD->getParent()));
+ Address ThisPtr = Address(&*AI, ConvertTypeForMem(MD->getThisObjectType()),
+ CGM.getClassPointerAlignment(MD->getParent()));
llvm::BasicBlock *EntryBB = &Fn->front();
llvm::BasicBlock::iterator ThisStore =
llvm::find_if(*EntryBB, [&](llvm::Instruction &I) {
@@ -640,11 +639,11 @@ void CodeGenVTables::addRelativeComponent(ConstantArrayBuilder &builder,
// Examples where there would be no symbol emitted are available_externally
// and private linkages.
//
- // `internal` linkage results in STB_LOCAL Elf binding while still manifesting a
- // local symbol.
+ // `internal` linkage results in STB_LOCAL Elf binding while still manifesting
+ // a local symbol.
//
- // `linkonce_odr` linkage results in a STB_DEFAULT Elf binding but also allows for
- // the rtti_proxy to be transparently replaced with a GOTPCREL reloc by a
+ // `linkonce_odr` linkage results in a STB_DEFAULT Elf binding but also allows
+ // for the rtti_proxy to be transparently replaced with a GOTPCREL reloc by a
// target that supports this replacement.
auto stubLinkage = vtableHasLocalLinkage
? llvm::GlobalValue::InternalLinkage
@@ -1324,30 +1323,30 @@ void CodeGenModule::EmitVTableTypeMetadata(const CXXRecordDecl *RD,
AP.second.AddressPointIndex));
// Sort the address points for determinism.
- llvm::sort(AddressPoints, [this](const AddressPoint &AP1,
- const AddressPoint &AP2) {
- if (&AP1 == &AP2)
- return false;
-
- std::string S1;
- llvm::raw_string_ostream O1(S1);
- getCXXABI().getMangleContext().mangleTypeName(
- QualType(AP1.first->getTypeForDecl(), 0), O1);
- O1.flush();
-
- std::string S2;
- llvm::raw_string_ostream O2(S2);
- getCXXABI().getMangleContext().mangleTypeName(
- QualType(AP2.first->getTypeForDecl(), 0), O2);
- O2.flush();
-
- if (S1 < S2)
- return true;
- if (S1 != S2)
- return false;
-
- return AP1.second < AP2.second;
- });
+ llvm::sort(AddressPoints,
+ [this](const AddressPoint &AP1, const AddressPoint &AP2) {
+ if (&AP1 == &AP2)
+ return false;
+
+ std::string S1;
+ llvm::raw_string_ostream O1(S1);
+ getCXXABI().getMangleContext().mangleTypeName(
+ QualType(AP1.first->getTypeForDecl(), 0), O1);
+ O1.flush();
+
+ std::string S2;
+ llvm::raw_string_ostream O2(S2);
+ getCXXABI().getMangleContext().mangleTypeName(
+ QualType(AP2.first->getTypeForDecl(), 0), O2);
+ O2.flush();
+
+ if (S1 < S2)
+ return true;
+ if (S1 != S2)
+ return false;
+
+ return AP1.second < AP2.second;
+ });
ArrayRef<VTableComponent> Comps = VTLayout.vtable_components();
for (auto AP : AddressPoints) {
|
petrhosek
approved these changes
Oct 3, 2023
petrhosek
reviewed
Oct 3, 2023
petrhosek
reviewed
Oct 3, 2023
…xternal linkage This way, it the rtti_proxies can be candidates for being replaced altogether with GOTPCREL relocations because they are discardable. Functionally, this shouldn't change the final ELF linkage of the proxies.
PiJoules
force-pushed
the
rv-linkonce_odr
branch
from
0e798f3
to
2607a99
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This way, it the rtti_proxies can be candidates for being replaced altogether with GOTPCREL relocations because they are discardable. Functionally, this shouldn't change the final ELF linkage of the proxies.