-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[llvm] Re-use original global name in RelLookupTableConverter #93626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-llvm-transforms Author: None (PiJoules) ChangesPrior, the reltable we create was "reltable." + FuncName which can result in multiple tables named "reltable." + FuncName + ".{number}" if we substitute multiple tables in a function. Since we replace the original global, it makes it easier to just take over the original global's name. Functionally, this doesn't change the IR emitted, just global names. This is a subset of PR 93355 that I'm breaking into multiple patches. Full diff: https://github.com/llvm/llvm-project/pull/93626.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp b/llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp
index ea628d7c3d7d6..e6663bb69715c 100644
--- a/llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp
+++ b/llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp
@@ -100,10 +100,10 @@ static GlobalVariable *createRelLookupTable(Function &Func,
ArrayType::get(Type::getInt32Ty(M.getContext()), NumElts);
GlobalVariable *RelLookupTable = new GlobalVariable(
- M, IntArrayTy, LookupTable.isConstant(), LookupTable.getLinkage(),
- nullptr, "reltable." + Func.getName(), &LookupTable,
- LookupTable.getThreadLocalMode(), LookupTable.getAddressSpace(),
- LookupTable.isExternallyInitialized());
+ M, IntArrayTy, LookupTable.isConstant(), LookupTable.getLinkage(),
+ nullptr, /*Name=*/"", &LookupTable, LookupTable.getThreadLocalMode(),
+ LookupTable.getAddressSpace(), LookupTable.isExternallyInitialized());
+ RelLookupTable->takeName(&LookupTable);
uint64_t Idx = 0;
SmallVector<Constant *, 64> RelLookupTableContents(NumElts);
diff --git a/llvm/test/Transforms/RelLookupTableConverter/X86/opaque-ptr.ll b/llvm/test/Transforms/RelLookupTableConverter/X86/opaque-ptr.ll
index b60f447a56774..f067feaa0ea85 100644
--- a/llvm/test/Transforms/RelLookupTableConverter/X86/opaque-ptr.ll
+++ b/llvm/test/Transforms/RelLookupTableConverter/X86/opaque-ptr.ll
@@ -15,7 +15,7 @@ target triple = "x86_64-unknown-linux-gnu"
define ptr @test(i32 %cond) {
; CHECK-LABEL: @test(
; CHECK-NEXT: [[RELTABLE_SHIFT:%.*]] = shl i32 [[COND:%.*]], 2
-; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @reltable.test, i32 [[RELTABLE_SHIFT]])
+; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @table1, i32 [[RELTABLE_SHIFT]])
; CHECK-NEXT: ret ptr [[RELTABLE_INTRINSIC]]
;
%switch.gep = getelementptr inbounds [3 x ptr], ptr @table1, i32 0, i32 %cond
diff --git a/llvm/test/Transforms/RelLookupTableConverter/X86/relative_lookup_table.ll b/llvm/test/Transforms/RelLookupTableConverter/X86/relative_lookup_table.ll
index 9e433e9a90355..566cc494d18e3 100644
--- a/llvm/test/Transforms/RelLookupTableConverter/X86/relative_lookup_table.ll
+++ b/llvm/test/Transforms/RelLookupTableConverter/X86/relative_lookup_table.ll
@@ -73,51 +73,51 @@ target triple = "x86_64-unknown-linux-gnu"
; CHECK: @switch.table.external_linkage = private unnamed_addr constant [3 x ptr] [ptr @a1, ptr @b1, ptr @c1], align
; Lookup table check for integer pointers that have internal linkage
-; CHECK: @reltable.internal_linkage = private unnamed_addr constant [3 x i32]
+; CHECK: @switch.table.internal_linkage = private unnamed_addr constant [3 x i32]
; CHECK-SAME: [
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @a2 to i64), i64 ptrtoint (ptr @reltable.internal_linkage to i64)) to i32),
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @b2 to i64), i64 ptrtoint (ptr @reltable.internal_linkage to i64)) to i32),
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @c2 to i64), i64 ptrtoint (ptr @reltable.internal_linkage to i64)) to i32)
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @a2 to i64), i64 ptrtoint (ptr @switch.table.internal_linkage to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @b2 to i64), i64 ptrtoint (ptr @switch.table.internal_linkage to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @c2 to i64), i64 ptrtoint (ptr @switch.table.internal_linkage to i64)) to i32)
; CHECK-SAME: ], align 4
; Relative switch lookup table for strings
-; CHECK: @reltable.string_table = private unnamed_addr constant [3 x i32]
+; CHECK: @switch.table.string_table = private unnamed_addr constant [3 x i32]
; CHECK-SAME: [
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str to i64), i64 ptrtoint (ptr @reltable.string_table to i64)) to i32),
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.1 to i64), i64 ptrtoint (ptr @reltable.string_table to i64)) to i32),
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.2 to i64), i64 ptrtoint (ptr @reltable.string_table to i64)) to i32)
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str to i64), i64 ptrtoint (ptr @switch.table.string_table to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.1 to i64), i64 ptrtoint (ptr @switch.table.string_table to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.2 to i64), i64 ptrtoint (ptr @switch.table.string_table to i64)) to i32)
; CHECK-SAME: ], align 4
; Relative switch lookup table for strings with holes, where holes are filled with relative offset to default values
-; CHECK: @reltable.string_table_holes = private unnamed_addr constant [4 x i32]
+; CHECK: @switch.table.string_table_holes = private unnamed_addr constant [4 x i32]
; CHECK-SAME: [
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str to i64), i64 ptrtoint (ptr @reltable.string_table_holes to i64)) to i32),
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.3 to i64), i64 ptrtoint (ptr @reltable.string_table_holes to i64)) to i32),
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.2 to i64), i64 ptrtoint (ptr @reltable.string_table_holes to i64)) to i32),
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.4 to i64), i64 ptrtoint (ptr @reltable.string_table_holes to i64)) to i32)
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str to i64), i64 ptrtoint (ptr @switch.table.string_table_holes to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.3 to i64), i64 ptrtoint (ptr @switch.table.string_table_holes to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.2 to i64), i64 ptrtoint (ptr @switch.table.string_table_holes to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.4 to i64), i64 ptrtoint (ptr @switch.table.string_table_holes to i64)) to i32)
; CHECK-SAME: ], align 4
; Single value check
-; CHECK: @reltable.single_value = private unnamed_addr constant [3 x i32]
+; CHECK: @switch.table.single_value = private unnamed_addr constant [3 x i32]
; CHECK-SAME: [
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str to i64), i64 ptrtoint (ptr @reltable.single_value to i64)) to i32),
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.1 to i64), i64 ptrtoint (ptr @reltable.single_value to i64)) to i32),
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.2 to i64), i64 ptrtoint (ptr @reltable.single_value to i64)) to i32)
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str to i64), i64 ptrtoint (ptr @switch.table.single_value to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.1 to i64), i64 ptrtoint (ptr @switch.table.single_value to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.2 to i64), i64 ptrtoint (ptr @switch.table.single_value to i64)) to i32)
; CHECK-SAME: ], align 4
;
; Relative lookup table for the loop hoist check test
-; CHECK: @reltable.loop_hoist = internal unnamed_addr constant [2 x i32]
+; CHECK: @table = internal unnamed_addr constant [2 x i32]
; CHECK-SAME: [
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.8 to i64), i64 ptrtoint (ptr @reltable.loop_hoist to i64)) to i32),
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.9 to i64), i64 ptrtoint (ptr @reltable.loop_hoist to i64)) to i32)
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.8 to i64), i64 ptrtoint (ptr @table to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.9 to i64), i64 ptrtoint (ptr @table to i64)) to i32)
; CHECK-SAME: ], align 4
; Relative look up table for the test where gep is not immediately followed by a load check
-; CHECK: @reltable.gep_is_not_imm_followed_by_load = internal unnamed_addr constant [2 x i32]
+; CHECK: @table2 = internal unnamed_addr constant [2 x i32]
; CHECK-SAME: [
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.8 to i64), i64 ptrtoint (ptr @reltable.gep_is_not_imm_followed_by_load to i64)) to i32),
-; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.9 to i64), i64 ptrtoint (ptr @reltable.gep_is_not_imm_followed_by_load to i64)) to i32)
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.8 to i64), i64 ptrtoint (ptr @table2 to i64)) to i32),
+; CHECK-SAME: i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.9 to i64), i64 ptrtoint (ptr @table2 to i64)) to i32)
; CHECK-SAME: ], align 4
; Lookup table check for integer pointers that have external linkage
@@ -154,7 +154,7 @@ define ptr @internal_linkage(i32 %cond) {
; CHECK-NEXT: br i1 [[TMP0]], label [[SWITCH_LOOKUP:%.*]], label [[RETURN:%.*]]
; CHECK: switch.lookup:
; CHECK-NEXT: [[RELTABLE_SHIFT:%.*]] = shl i32 %cond, 2
-; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @reltable.internal_linkage, i32 [[RELTABLE_SHIFT]])
+; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @switch.table.internal_linkage, i32 [[RELTABLE_SHIFT]])
; CHECK-NEXT: ret ptr [[RELTABLE_INTRINSIC]]
; CHECK: return:
; CHECK-NEXT: ret ptr @d2
@@ -180,7 +180,7 @@ define ptr @string_table(i32 %cond) {
; CHECK-NEXT: br i1 [[TMP0]], label [[SWITCH_LOOKUP:%.*]], label [[RETURN:%.*]]
; CHECK: switch.lookup:
; CHECK-NEXT: [[RELTABLE_SHIFT:%.*]] = shl i32 %cond, 2
- ; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @reltable.string_table, i32 [[RELTABLE_SHIFT]])
+ ; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @switch.table.string_table, i32 [[RELTABLE_SHIFT]])
; CHECK-NEXT: ret ptr [[RELTABLE_INTRINSIC]]
; CHECK: return:
; CHECK-NEXT: ret ptr @.str.3
@@ -206,7 +206,7 @@ define ptr @string_table_holes(i32 %cond) {
; CHECK-NEXT: br i1 [[TMP0]], label [[SWITCH_LOOKUP:%.*]], label [[RETURN:%.*]]
; CHECK: switch.lookup:
; CHECK-NEXT: [[RELTABLE_SHIFT:%.*]] = shl i32 [[COND]], 2
-; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @reltable.string_table_holes, i32 [[RELTABLE_SHIFT]])
+; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @switch.table.string_table_holes, i32 [[RELTABLE_SHIFT]])
; CHECK-NEXT: ret ptr [[RELTABLE_INTRINSIC]]
; CHECK: return:
; CHECK-NEXT: ret ptr @.str.3
@@ -235,7 +235,7 @@ define void @single_value(i32 %cond) {
; CHECK-NEXT: br i1 [[TMP0]], label [[SWITCH_LOOKUP:%.*]], label [[RETURN:%.*]]
; CHECK: switch.lookup:
; CHECK-NEXT: [[RELTABLE_SHIFT:%.*]] = shl i32 [[COND]], 2
-; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @reltable.single_value, i32 [[RELTABLE_SHIFT]])
+; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @switch.table.single_value, i32 [[RELTABLE_SHIFT]])
; CHECK: sw.epilog:
; CHECK-NEXT: [[STR1:%.*]] = phi ptr [ @.str.5, %entry ], [ @.str.7, %switch.lookup ]
; CHECK-NEXT: [[STR2:%.*]] = phi ptr [ @.str.6, %entry ], [ [[RELTABLE_INTRINSIC]], [[SWITCH_LOOKUP]] ]
@@ -265,7 +265,7 @@ define ptr @user_defined_lookup_table(i32 %cond) {
; CHECK: cond.false:
; CHECK-NEXT: [[IDX_PROM:%.*]] = sext i32 [[COND]] to i64
; CHECK-NEXT: [[RELTABLE_SHIFT:%.*]] = shl i64 [[IDX_PROM]], 2
-; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i64(ptr @reltable.user_defined_lookup_table, i64 [[RELTABLE_SHIFT]])
+; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i64(ptr @user_defined_lookup_table.table, i64 [[RELTABLE_SHIFT]])
; CHECK-NEXT: br label %cond.end
; CHECK: cond.end:
; CHECK-NEXT: [[COND1:%.*]] = phi ptr [ [[RELTABLE_INTRINSIC]], %cond.false ], [ @.str.3, %entry ]
@@ -296,7 +296,7 @@ define ptr @loop_hoist(i32 %x) {
; CHECK-NEXT: [[RELTABLE_SHIFT:%.*]] = shl i32 [[X:%.*]], 2
; CHECK-NEXT: br i1 [[TMP0]], label %if.done, label %if.false
; CHECK: if.false:
-; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @reltable.loop_hoist, i32 [[RELTABLE_SHIFT]])
+; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @table, i32 [[RELTABLE_SHIFT]])
; CHECK-NEXT: br label %if.done
; CHECK: if.done:
; CHECK-NEXT: [[TMP2:%.*]] = phi ptr [ @.str.10, %entry ], [ [[RELTABLE_INTRINSIC]], %if.false ]
@@ -327,7 +327,7 @@ define ptr @gep_is_not_imm_followed_by_load(i32 %x) {
; CHECK: entry:
; CHECK-NEXT: [[RELTABLE_SHIFT:%.*]] = shl i32 [[X:%.*]], 2
; CHECK-NEXT: call void @may_not_return()
-; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @reltable.gep_is_not_imm_followed_by_load, i32 [[RELTABLE_SHIFT]])
+; CHECK-NEXT: [[RELTABLE_INTRINSIC:%.*]] = call ptr @llvm.load.relative.i32(ptr @table2, i32 [[RELTABLE_SHIFT]])
; CHECK-NEXT: ret ptr [[RELTABLE_INTRINSIC]]
;
entry:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming the original was deleted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd consider still adding a .rel
suffix, as the contents of the table change substantially, and this makes it more obvious.
Prior, the reltable we create was "reltable." + FuncName which can result in multiple tables named "reltable." + FuncName + ".{number}" if we substitute multiple tables in a function. Since we replace the original global, it makes it easier to just take over the original global's name + ".rel". Functionally, this doesn't change the IR emitted, just global names. This is a subset of PR 93355 that I'm breaking into multiple patches.
632f68d
to
e9b3950
Compare
Yup. The old one is erased |
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Prior, the reltable we create was "reltable." + FuncName which can result in multiple tables named "reltable." + FuncName + ".{number}" if we substitute multiple tables in a function. Since we replace the original global, it makes it easier to just take over the original global's name. Functionally, this doesn't change the IR emitted, just global names.
This is a subset of PR 93355 that I'm breaking into multiple patches.