[llvm] Match llvm.type.checked.load.relative semantics to llvm.load.r… #129583
Conversation
This file contains hidden or 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
…elative The semantics of `llvm.type.checked.load.relative` seem to be a little different from that of `llvm.load.relative`. It looks like the semantics for `llvm.type.checked.load.relative` is `ptr + offset + *(ptr + offset)` whereas the semantics for `llvm.load.relative` is `ptr + *(ptr + offset)`. That is, the offset for the former is added to the offset address whereas the later has the offset added to the original pointer. It really feels like the checked intrinsic was meant to match the semantics of the non-checked intrinsic, but I think for all cases the checked intrinsic is used (swift being the only use I know of), the calculation just happens to be the same because swift always uses an offset of zero. Likewise, all llvm tests for this intrinsic happen to use an offset of zero. Relative vtables in clang happens to be the first time where we're using this intrinsic and using it with non-zero values. This updates the semantics of the checked intrinsic to match the non-checked one. Effectively this shouldn't change any codegen by any users of this since all current users seem to use a zero offset.
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-llvm-transforms Author: None (PiJoules) Changes…elative The semantics of It really feels like the checked intrinsic was meant to match the semantics of the non-checked intrinsic, but I think for all cases the checked intrinsic is used (swift being the only use I know of), the calculation just happens to be the same because swift always uses an offset of zero. Likewise, all llvm tests for this intrinsic happen to use an offset of zero. Relative vtables in clang happens to be the first time where we're using this intrinsic and using it with non-zero values. This updates the semantics of the checked intrinsic to match the non-checked one. Effectively this shouldn't change any codegen by any users of this since all current users seem to use a zero offset. Full diff: https://github.com/llvm/llvm-project/pull/129583.diff 5 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 33c85c7ba9d29..a4316ead0974c 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -29512,9 +29512,10 @@ The ``llvm.type.checked.load.relative`` intrinsic loads a relative pointer to a
function from a virtual table pointer using metadata. Otherwise, its semantic is
identical to the ``llvm.type.checked.load`` intrinsic.
-A relative pointer is a pointer to an offset to the pointed to value. The
-address of the underlying pointer of the relative pointer is obtained by adding
-the offset to the address of the offset value.
+A relative pointer is a pointer to an offset. This is the offset between the destination
+pointer and the original pointer. The address of the destination pointer is obtained
+by loading this offset and adding it to the original pointer. This calculation is the
+same as that of the ``llvm.load.relative`` intrinsic.
'``llvm.arithmetic.fence``' Intrinsic
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md
index f1f64f77ee71a..aeb7267ad159d 100644
--- a/llvm/docs/ReleaseNotes.md
+++ b/llvm/docs/ReleaseNotes.md
@@ -62,6 +62,9 @@ Changes to the LLVM IR
* `mul`
+* Updated semantics of `llvm.type.checked.load.relative` to match that of
+ `llvm.load.relative`.
+
Changes to LLVM infrastructure
------------------------------
diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
index 30e935ea663f3..0680b2c558807 100644
--- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -2100,12 +2100,9 @@ void DevirtModule::scanTypeCheckedLoadUsers(Function *TypeCheckedLoadFunc) {
Value *LoadedValue = nullptr;
if (TypeCheckedLoadFunc->getIntrinsicID() ==
Intrinsic::type_checked_load_relative) {
- Value *GEP = LoadB.CreatePtrAdd(Ptr, Offset);
- LoadedValue = LoadB.CreateLoad(Int32Ty, GEP);
- LoadedValue = LoadB.CreateSExt(LoadedValue, IntPtrTy);
- GEP = LoadB.CreatePtrToInt(GEP, IntPtrTy);
- LoadedValue = LoadB.CreateAdd(GEP, LoadedValue);
- LoadedValue = LoadB.CreateIntToPtr(LoadedValue, Int8PtrTy);
+ Function *LoadRelFunc = Intrinsic::getOrInsertDeclaration(
+ &M, Intrinsic::load_relative, {Int32Ty});
+ LoadedValue = LoadB.CreateCall(LoadRelFunc, {Ptr, Offset});
} else {
Value *GEP = LoadB.CreatePtrAdd(Ptr, Offset);
LoadedValue = LoadB.CreateLoad(Int8PtrTy, GEP);
diff --git a/llvm/test/Transforms/WholeProgramDevirt/devirt-single-impl-check-relative.ll b/llvm/test/Transforms/WholeProgramDevirt/devirt-single-impl-check-relative.ll
index ba8c89d950ffb..03ec48a5b8467 100644
--- a/llvm/test/Transforms/WholeProgramDevirt/devirt-single-impl-check-relative.ll
+++ b/llvm/test/Transforms/WholeProgramDevirt/devirt-single-impl-check-relative.ll
@@ -3,18 +3,19 @@
target datalayout = "e-p:64:64"
target triple = "x86_64-unknown-linux-gnu"
-; CHECK: remark: <unknown>:0:0: single-impl: devirtualized a call to vf
-; CHECK: remark: <unknown>:0:0: devirtualized vf
+; CHECK: remark: <unknown>:0:0: single-impl: devirtualized a call to vfunc1_live
+; CHECK: remark: <unknown>:0:0: single-impl: devirtualized a call to vfunc3_live
+; CHECK: remark: <unknown>:0:0: devirtualized vfunc1_live
+; CHECK: remark: <unknown>:0:0: devirtualized vfunc3_live
; CHECK-NOT: devirtualized
; A vtable with "relative pointers", slots don't contain pointers to implementations, but instead have an i32 offset from the vtable itself to the implementation.
-@vtable = internal unnamed_addr constant { [2 x i32] } { [2 x i32] [
+@vtable = internal unnamed_addr constant { [3 x i32] } { [3 x i32] [
i32 trunc (i64 sub (i64 ptrtoint (ptr @vfunc1_live to i64), i64 ptrtoint (ptr @vtable to i64)) to i32),
- i32 trunc (i64 sub (i64 ptrtoint (ptr @vfunc2_dead to i64), i64 ptrtoint (ptr @vtable to i64)) to i32)
-]}, align 8, !type !0, !type !1
-;, !vcall_visibility !{i64 2}
-!0 = !{i64 0, !"vfunc1.type"}
-!1 = !{i64 4, !"vfunc2.type"}
+ i32 trunc (i64 sub (i64 ptrtoint (ptr @vfunc2_dead to i64), i64 ptrtoint (ptr @vtable to i64)) to i32),
+ i32 trunc (i64 sub (i64 ptrtoint (ptr @vfunc3_live to i64), i64 ptrtoint (ptr @vtable to i64)) to i32)
+]}, align 8, !type !0
+!0 = !{i64 0, !"vtable"}
define internal void @vfunc1_live() {
ret void
@@ -24,10 +25,14 @@ define internal void @vfunc2_dead() {
ret void
}
-; CHECK: define void @call
-define void @call(ptr %obj) {
+define internal void @vfunc3_live() {
+ ret void
+}
+
+; CHECK: define void @call_vf1
+define void @call_vf1(ptr %obj) {
%vtable = load ptr, ptr %obj
- %pair = call {ptr, i1} @llvm.type.checked.load.relative(ptr %vtable, i32 0, metadata !"vfunc1.type")
+ %pair = call {ptr, i1} @llvm.type.checked.load.relative(ptr %vtable, i32 0, metadata !"vtable")
%fptr = extractvalue {ptr, i1} %pair, 0
%p = extractvalue {ptr, i1} %pair, 1
; CHECK: br i1 true,
@@ -43,5 +48,24 @@ trap:
unreachable
}
+; CHECK: define void @call_vf3
+define void @call_vf3(ptr %obj) {
+ %vtable = load ptr, ptr %obj
+ %pair = call {ptr, i1} @llvm.type.checked.load.relative(ptr %vtable, i32 8, metadata !"vtable")
+ %fptr = extractvalue {ptr, i1} %pair, 0
+ %p = extractvalue {ptr, i1} %pair, 1
+ ; CHECK: br i1 true,
+ br i1 %p, label %cont, label %trap
+
+cont:
+ ; CHECK: call void @vfunc3_live(
+ call void %fptr()
+ ret void
+
+trap:
+ call void @llvm.trap()
+ unreachable
+}
+
declare {ptr, i1} @llvm.type.checked.load.relative(ptr, i32, metadata)
declare void @llvm.trap()
diff --git a/llvm/test/Transforms/WholeProgramDevirt/expand-check-relative.ll b/llvm/test/Transforms/WholeProgramDevirt/expand-check-relative.ll
index d67458952ba72..275b188f848cd 100644
--- a/llvm/test/Transforms/WholeProgramDevirt/expand-check-relative.ll
+++ b/llvm/test/Transforms/WholeProgramDevirt/expand-check-relative.ll
@@ -9,11 +9,9 @@ target triple = "x86_64-unknown-linux-gnu"
@vt1 = constant { [2 x i32] } { [2 x i32] [
i32 trunc (i64 sub (i64 ptrtoint (ptr @vf1 to i64), i64 ptrtoint (ptr @vt1 to i64)) to i32),
i32 trunc (i64 sub (i64 ptrtoint (ptr @vf2 to i64), i64 ptrtoint (ptr @vt1 to i64)) to i32)
-]}, align 8, !type !0, !type !1
-
-!0 = !{i64 0, !"vfunc1.type"}
-!1 = !{i64 4, !"vfunc2.type"}
+]}, align 8, !type !0
+!0 = !{i64 0, !"vtable"}
define void @vf1(ptr %this) {
ret void
@@ -23,24 +21,42 @@ define void @vf2(ptr %this) {
ret void
}
-; CHECK: define void @call
-; CHECK: [[TT:%.*]] = call i1 @llvm.type.test(ptr [[VT:%.*]], metadata !"vfunc1.type")
+; CHECK: define void @call_vf1
+; CHECK: [[TT:%.*]] = call i1 @llvm.type.test(ptr [[VT:%.*]], metadata !"vtable")
; CHECK: br i1 [[TT]]
-; Relative pointer computation at the address of the i32 value to the i32 value
+; Relative pointer computation at the vtable to the i32 value
; to get to the pointer value.
-; CHECK: [[T0:%.*]] = getelementptr i8, ptr [[VT]], i32 0
-; CHECK: [[T1:%.*]] = load i32, ptr [[T0]]
-; CHECK: [[T2:%.*]] = sext i32 [[T1]] to i64
-; CHECK: [[T3:%.*]] = ptrtoint ptr [[T0]] to i64
-; CHECK: [[T4:%.*]] = add i64 [[T3]], [[T2]]
-; CHECK: [[F:%.*]] = inttoptr i64 [[T4]] to ptr
+; CHECK: [[F:%.*]] = call ptr @llvm.load.relative.i32(ptr [[VT]], i32 0)
+; CHECK: call void [[F]](ptr
+
+define void @call_vf1(ptr %obj) {
+ %vtable = load ptr, ptr %obj
+ %pair = call {ptr, i1} @llvm.type.checked.load.relative(ptr %vtable, i32 0, metadata !"vtable")
+ %p = extractvalue {ptr, i1} %pair, 1
+ br i1 %p, label %cont, label %trap
+
+cont:
+ %fptr = extractvalue {ptr, i1} %pair, 0
+ call void %fptr(ptr %obj)
+ ret void
+
+trap:
+ call void @llvm.trap()
+ unreachable
+}
+
+; CHECK: define void @call_vf2
+; CHECK: [[TT:%.*]] = call i1 @llvm.type.test(ptr [[VT:%.*]], metadata !"vtable")
+; CHECK: br i1 [[TT]]
+
+; CHECK: [[F:%.*]] = call ptr @llvm.load.relative.i32(ptr [[VT]], i32 4)
; CHECK: call void [[F]](ptr
-define void @call(ptr %obj) {
+define void @call_vf2(ptr %obj) {
%vtable = load ptr, ptr %obj
- %pair = call {ptr, i1} @llvm.type.checked.load.relative(ptr %vtable, i32 0, metadata !"vfunc1.type")
+ %pair = call {ptr, i1} @llvm.type.checked.load.relative(ptr %vtable, i32 4, metadata !"vtable")
%p = extractvalue {ptr, i1} %pair, 1
br i1 %p, label %cont, label %trap
|
|
ping for reviews |
ilovepi
approved these changes
Mar 12, 2025
|
Thanks. I'll let this sit for a day or so in case others want to chime in before landing. |
frederik-h
pushed a commit
to frederik-h/llvm-project
that referenced
this pull request
Mar 18, 2025
llvm#129583) …elative The semantics of `llvm.type.checked.load.relative` seem to be a little different from that of `llvm.load.relative`. It looks like the semantics for `llvm.type.checked.load.relative` is `ptr + offset + *(ptr + offset)` whereas the semantics for `llvm.load.relative` is `ptr + *(ptr + offset)`. That is, the offset for the former is added to the offset address whereas the later has the offset added to the original pointer. It really feels like the checked intrinsic was meant to match the semantics of the non-checked intrinsic, but I think for all cases the checked intrinsic is used (swift being the only use I know of), the calculation just happens to be the same because swift always uses an offset of zero. Likewise, all llvm tests for this intrinsic happen to use an offset of zero. Relative vtables in clang happens to be the first time where we're using this intrinsic and using it with non-zero values. This updates the semantics of the checked intrinsic to match the non-checked one. Effectively this shouldn't change any codegen by any users of this since all current users seem to use a zero offset. This PR also updates some tests with non-zero offsets.
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.
…elative
The semantics of
llvm.type.checked.load.relativeseem to be a little different from that ofllvm.load.relative. It looks like the semantics forllvm.type.checked.load.relativeisptr + offset + *(ptr + offset)whereas the semantics forllvm.load.relativeisptr + *(ptr + offset). That is, the offset for the former is added to the offset address whereas the later has the offset added to the original pointer.It really feels like the checked intrinsic was meant to match the semantics of the non-checked intrinsic, but I think for all cases the checked intrinsic is used (swift being the only use I know of), the calculation just happens to be the same because swift always uses an offset of zero. Likewise, all llvm tests for this intrinsic happen to use an offset of zero.
Relative vtables in clang happens to be the first time where we're using this intrinsic and using it with non-zero values. This updates the semantics of the checked intrinsic to match the non-checked one. Effectively this shouldn't change any codegen by any users of this since all current users seem to use a zero offset.
This PR also updates some tests with non-zero offsets.