[llvm] [RS4GC] Fix some test comments that got clobbered (PR #145186)
Wren [Undefined] via llvm-commits
llvm-commits at lists.llvm.org
Sat Jun 21 13:14:55 PDT 2025
https://github.com/ThePuzzlemaker created https://github.com/llvm/llvm-project/pull/145186
The commit 0407108 did some transformations on tests for RewriteStatepointsForGC that put some comments out-of-order. At least the `basics.ll` test is referred to by the current LLVM statepoints documentation, so this may have caused confusion to end-users investigating further into statepoint usage.
Additionally, some comments were outright removed that explained potentially non-trivial behaviour of the pass in the tests.
This is my first time ever submitting a patch to LLVM, so please let me know if there's anything I accidentally overlooked. I re-tested all tests to ensure no issues accidentally arose from these changes.
>From 63dc150416b390876bf6bf4fc91bec4168345132 Mon Sep 17 00:00:00 2001
From: ThePuzzlemaker <tpzker at thepuzzlemaker.info>
Date: Sat, 21 Jun 2025 15:11:05 -0500
Subject: [PATCH] [RS4GC] Fix some test comments that got clobbered
The commit 0407108 did some transformations on tests for
RewriteStatepointsForGC that put some comments out-of-order. At least
the `basics.ll` test is referred to by the current LLVM statepoints
documentation, so this may have caused confusion to end-users
investigating further into statepoint usage.
Additionally, some comments were outright removed that explained
potentially non-trivial behaviour of the pass in the tests.
---
.../RewriteStatepointsForGC/base-vector.ll | 12 ++++++------
.../RewriteStatepointsForGC/basics.ll | 13 ++++++-------
.../RewriteStatepointsForGC/codegen-cond.ll | 7 +++----
.../RewriteStatepointsForGC/constants.ll | 13 +++++++++----
.../RewriteStatepointsForGC/liveness-basics.ll | 17 ++++++++---------
.../RewriteStatepointsForGC/preprocess.ll | 11 +++++------
.../RewriteStatepointsForGC/relocation.ll | 9 ++++++---
7 files changed, 43 insertions(+), 39 deletions(-)
diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/base-vector.ll b/llvm/test/Transforms/RewriteStatepointsForGC/base-vector.ll
index b3ac71ac18db9..a6235e28a823a 100644
--- a/llvm/test/Transforms/RewriteStatepointsForGC/base-vector.ll
+++ b/llvm/test/Transforms/RewriteStatepointsForGC/base-vector.ll
@@ -92,8 +92,6 @@ entry:
}
define ptr addrspace(1) @test4(ptr addrspace(1) %ptr) gc "statepoint-example" {
-; When we can optimize an extractelement from a known
-; index and avoid introducing new base pointer instructions
; CHECK-LABEL: @test4(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[DERIVED:%.*]] = getelementptr i64, ptr addrspace(1) [[PTR:%.*]], i64 16
@@ -120,8 +118,9 @@ entry:
declare void @use(ptr addrspace(1)) "gc-leaf-function"
declare void @use_vec(<4 x ptr addrspace(1)>) "gc-leaf-function"
+; When we can optimize an extractelement from a known
+; index and avoid introducing new base pointer instructions
define void @test5(i1 %cnd, ptr addrspace(1) %obj) gc "statepoint-example" {
-; When we fundementally have to duplicate
; CHECK-LABEL: @test5(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[GEP:%.*]] = getelementptr i64, ptr addrspace(1) [[OBJ:%.*]], i64 1
@@ -144,9 +143,8 @@ entry:
ret void
}
+; When we fundementally have to duplicate
define void @test6(i1 %cnd, ptr addrspace(1) %obj, i64 %idx) gc "statepoint-example" {
-; A more complicated example involving vector and scalar bases.
-; This is derived from a failing test case when we didn't have correct
; insertelement handling.
; CHECK-LABEL: @test6(
; CHECK-NEXT: entry:
@@ -170,6 +168,8 @@ entry:
ret void
}
+; A more complicated example involving vector and scalar bases.
+; This is derived from a failing test case when we didn't have correct
define ptr addrspace(1) @test7(i1 %cnd, ptr addrspace(1) %obj, ptr addrspace(1) %obj2) gc "statepoint-example" {
; CHECK-LABEL: @test7(
; CHECK-NEXT: entry:
@@ -384,4 +384,4 @@ bb1: ; preds = %bb1, %bb
br label %bb1
}
-declare i32 @spam() gc "statepoint-example"
\ No newline at end of file
+declare i32 @spam() gc "statepoint-example"
diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/basics.ll b/llvm/test/Transforms/RewriteStatepointsForGC/basics.ll
index 644baebcca7ee..6d7746b31f056 100644
--- a/llvm/test/Transforms/RewriteStatepointsForGC/basics.ll
+++ b/llvm/test/Transforms/RewriteStatepointsForGC/basics.ll
@@ -1,21 +1,20 @@
; This is a collection of really basic tests for gc.statepoint rewriting.
; RUN: opt < %s -passes=rewrite-statepoints-for-gc -spp-rematerialization-threshold=0 -S | FileCheck %s
-; Trivial relocation over a single call
-
declare void @foo()
+; Trivial relocation over a single call
define ptr addrspace(1) @test1(ptr addrspace(1) %obj) gc "statepoint-example" {
; CHECK-LABEL: @test1
entry:
; CHECK-LABEL: entry:
; CHECK-NEXT: gc.statepoint
; CHECK-NEXT: %obj.relocated = call coldcc ptr addrspace(1)
-; Two safepoints in a row (i.e. consistent liveness)
call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
ret ptr addrspace(1) %obj
}
+; Two safepoints in a row (i.e. consistent liveness)
define ptr addrspace(1) @test2(ptr addrspace(1) %obj) gc "statepoint-example" {
; CHECK-LABEL: @test2
entry:
@@ -24,12 +23,12 @@ entry:
; CHECK-NEXT: %obj.relocated = call coldcc ptr addrspace(1)
; CHECK-NEXT: gc.statepoint
; CHECK-NEXT: %obj.relocated2 = call coldcc ptr addrspace(1)
-; A simple derived pointer
call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
ret ptr addrspace(1) %obj
}
+; A simple derived pointer
define i8 @test3(ptr addrspace(1) %obj) gc "statepoint-example" {
entry:
; CHECK-LABEL: entry:
@@ -39,8 +38,6 @@ entry:
; CHECK-NEXT: %derived.relocated = call coldcc ptr addrspace(1)
; CHECK-NEXT: load i8, ptr addrspace(1) %derived.relocated
; CHECK-NEXT: load i8, ptr addrspace(1) %obj.relocated
-; Tests to make sure we visit both the taken and untaken predeccessor
-; of merge. This was a bug in the dataflow liveness at one point.
%derived = getelementptr i8, ptr addrspace(1) %obj, i64 10
call void @foo() [ "deopt"(i32 0, i32 -1, i32 0, i32 0, i32 0) ]
%a = load i8, ptr addrspace(1) %derived
@@ -49,6 +46,8 @@ entry:
ret i8 %c
}
+; Tests to make sure we visit both the taken and untaken predeccessor
+; of merge. This was a bug in the dataflow liveness at one point.
define ptr addrspace(1) @test4(i1 %cmp, ptr addrspace(1) %obj) gc "statepoint-example" {
entry:
br i1 %cmp, label %taken, label %untaken
@@ -71,10 +70,10 @@ merge: ; preds = %untaken, %taken
; CHECK-LABEL: merge:
; CHECK-NEXT: %.0 = phi ptr addrspace(1) [ %obj.relocated, %taken ], [ %obj.relocated2, %untaken ]
; CHECK-NEXT: ret ptr addrspace(1) %.0
-; When run over a function which doesn't opt in, should do nothing!
ret ptr addrspace(1) %obj
}
+; When run over a function which doesn't opt in, should do nothing!
define ptr addrspace(1) @test5(ptr addrspace(1) %obj) gc "ocaml" {
; CHECK-LABEL: @test5
entry:
diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/codegen-cond.ll b/llvm/test/Transforms/RewriteStatepointsForGC/codegen-cond.ll
index be3b01d68827c..f26d6e25bbc7d 100644
--- a/llvm/test/Transforms/RewriteStatepointsForGC/codegen-cond.ll
+++ b/llvm/test/Transforms/RewriteStatepointsForGC/codegen-cond.ll
@@ -1,7 +1,6 @@
; RUN: opt -passes=rewrite-statepoints-for-gc -S < %s | FileCheck %s
; A null test of a single value
-
define i1 @test(ptr addrspace(1) %p, i1 %rare) gc "statepoint-example" {
; CHECK-LABEL: @test
entry:
@@ -19,7 +18,6 @@ continue: ; preds = %safepoint, %entry
; CHECK-DAG: [ %p, %entry ]
; CHECK: %cond = icmp
; CHECK: br i1 %cond
-; Comparing two pointers
br i1 %cond, label %taken, label %untaken
taken: ; preds = %continue
@@ -29,6 +27,7 @@ untaken: ; preds = %continue
ret i1 false
}
+; Comparing two pointers
define i1 @test2(ptr addrspace(1) %p, ptr addrspace(1) %q, i1 %rare) gc "statepoint-example" {
; CHECK-LABEL: @test2
entry:
@@ -49,8 +48,6 @@ continue: ; preds = %safepoint, %entry
; CHECK-DAG: [ %p, %entry ]
; CHECK: %cond = icmp
; CHECK: br i1 %cond
-; Check that nothing bad happens if already last instruction
-; before terminator
br i1 %cond, label %taken, label %untaken
taken: ; preds = %continue
@@ -60,6 +57,8 @@ untaken: ; preds = %continue
ret i1 false
}
+; Check that nothing bad happens if already last instruction
+; before terminator
define i1 @test3(ptr addrspace(1) %p, ptr addrspace(1) %q, i1 %rare) gc "statepoint-example" {
; CHECK-LABEL: @test3
; CHECK: gc.statepoint
diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/constants.ll b/llvm/test/Transforms/RewriteStatepointsForGC/constants.ll
index d1459bc69fe91..547074e4a08d3 100644
--- a/llvm/test/Transforms/RewriteStatepointsForGC/constants.ll
+++ b/llvm/test/Transforms/RewriteStatepointsForGC/constants.ll
@@ -2,28 +2,27 @@
target datalayout = "e-ni:1:6"
-; constants don't get relocated.
@G = addrspace(1) global i8 5
declare void @foo()
+; constants don't get relocated.
define i8 @test() gc "statepoint-example" {
; CHECK-LABEL: @test
; CHECK: gc.statepoint
; CHECK-NEXT: load i8, ptr addrspace(1) inttoptr (i64 15 to ptr addrspace(1))
-; Mostly just here to show reasonable code test can come from.
entry:
call void @foo() [ "deopt"() ]
%res = load i8, ptr addrspace(1) inttoptr (i64 15 to ptr addrspace(1))
ret i8 %res
}
+; Mostly just here to show reasonable code test can come from.
define i8 @test2(ptr addrspace(1) %p) gc "statepoint-example" {
; CHECK-LABEL: @test2
; CHECK: gc.statepoint
; CHECK-NEXT: gc.relocate
; CHECK-NEXT: icmp
-; Globals don't move and thus don't get relocated
entry:
call void @foo() [ "deopt"() ]
%cmp = icmp eq ptr addrspace(1) %p, null
@@ -37,11 +36,17 @@ not_taken: ; preds = %entry
br i1 %cmp2, label %taken, label %dead
dead: ; preds = %not_taken
+ ; We see that dead can't be reached, but the optimizer might not. It's
+ ; completely legal for it to exploit the fact that if dead executed, %p
+ ; would have to equal null. This can produce intermediate states which
+ ; look like that of test above, even if arbitrary constant addresses aren't
+ ; legal in the source language
%addr = getelementptr i8, ptr addrspace(1) %p, i32 15
%res = load i8, ptr addrspace(1) %addr
ret i8 %res
}
+; Globals don't move and thus don't get relocated
define i8 @test3(i1 %always_true) gc "statepoint-example" {
; CHECK-LABEL: @test3
; CHECK: gc.statepoint
@@ -274,4 +279,4 @@ entry:
; CHECK-DAG: call {{.*}}gc.relocate{{.*}}(%val.base, %val.base)
; CHECK-DAG: call {{.*}}gc.relocate{{.*}}(%val.base, %val)
ret ptr addrspace(1) %val
-}
\ No newline at end of file
+}
diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/liveness-basics.ll b/llvm/test/Transforms/RewriteStatepointsForGC/liveness-basics.ll
index e827ff6e6e6a6..0d9dc24e06e00 100644
--- a/llvm/test/Transforms/RewriteStatepointsForGC/liveness-basics.ll
+++ b/llvm/test/Transforms/RewriteStatepointsForGC/liveness-basics.ll
@@ -4,7 +4,6 @@
; Tests to make sure we consider %obj live in both the taken and untaken
; predeccessor of merge.
-
define ptr addrspace(1) @test1(i1 %cmp, ptr addrspace(1) %obj) gc "statepoint-example" {
; CHECK-LABEL: @test1
entry:
@@ -30,10 +29,10 @@ merge: ; preds = %untaken, %taken
; CHECK-LABEL: merge:
; CHECK-NEXT: %.0 = phi ptr addrspace(1) [ %obj.relocated, %taken ], [ %obj.relocated2, %untaken ]
; CHECK-NEXT: ret ptr addrspace(1) %.0
-; A local kill should not effect liveness in predecessor block
ret ptr addrspace(1) %obj
}
+; A local kill should not effect liveness in predecessor block
define ptr addrspace(1) @test2(i1 %cmp, ptr %loc) gc "statepoint-example" {
; CHECK-LABEL: @test2
entry:
@@ -49,8 +48,6 @@ taken: ; preds = %entry
; CHECK-NEXT: gc.statepoint
; CHECK-NEXT: gc.relocate
; CHECK-NEXT: ret ptr addrspace(1) %obj.relocated
-; A local kill should effect values live from a successor phi. Also, we
-; should only propagate liveness from a phi to the appropriate predecessors.
%obj = load ptr addrspace(1), ptr %loc
call void @foo() [ "deopt"() ]
ret ptr addrspace(1) %obj
@@ -59,6 +56,8 @@ untaken: ; preds = %entry
ret ptr addrspace(1) null
}
+; A local kill should effect values live from a successor phi. Also, we
+; should only propagate liveness from a phi to the appropriate predecessors.
define ptr addrspace(1) @test3(i1 %cmp, ptr %loc) gc "statepoint-example" {
; CHECK-LABEL: @test3
entry:
@@ -80,8 +79,6 @@ untaken: ; preds = %entry
; CHECK-LABEL: taken:
; CHECK-NEXT: gc.statepoint
; CHECK-NEXT: br label %merge
-; A base pointer must be live if it is needed at a later statepoint,
-; even if the base pointer is otherwise unused.
call void @foo() [ "deopt"() ]
br label %merge
@@ -90,6 +87,8 @@ merge: ; preds = %untaken, %taken
ret ptr addrspace(1) %phi
}
+; A base pointer must be live if it is needed at a later statepoint,
+; even if the base pointer is otherwise unused.
define ptr addrspace(1) @test4(i1 %cmp, ptr addrspace(1) %obj) gc "statepoint-example" {
; CHECK-LABEL: @test4
entry:
@@ -105,9 +104,6 @@ entry:
; CHECK-NEXT: %obj.relocated3 =
; CHECK-NEXT: ret ptr addrspace(1) %derived.relocated2
;
-; Make sure that a phi def visited during iteration is considered a kill.
-; Also, liveness after base pointer analysis can change based on new uses,
-; not just new defs.
%derived = getelementptr i64, ptr addrspace(1) %obj, i64 8
call void @foo() [ "deopt"() ]
call void @foo() [ "deopt"() ]
@@ -116,6 +112,9 @@ entry:
declare void @consume(...) readonly "gc-leaf-function"
+; Make sure that a phi def visited during iteration is considered a kill.
+; Also, liveness after base pointer analysis can change based on new uses,
+; not just new defs.
define ptr addrspace(1) @test5(i1 %cmp, ptr addrspace(1) %obj) gc "statepoint-example" {
; CHECK-LABEL: @test5
entry:
diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/preprocess.ll b/llvm/test/Transforms/RewriteStatepointsForGC/preprocess.ll
index 4c7f9d66d92f6..7ac2d053821e3 100644
--- a/llvm/test/Transforms/RewriteStatepointsForGC/preprocess.ll
+++ b/llvm/test/Transforms/RewriteStatepointsForGC/preprocess.ll
@@ -1,10 +1,9 @@
; RUN: opt -passes=rewrite-statepoints-for-gc -S < %s | FileCheck %s
-; Test to make sure we destroy LCSSA's single entry phi nodes before
-; running liveness
-
declare void @consume(...) "gc-leaf-function"
+; Test to make sure we destroy LCSSA's single entry phi nodes before
+; running liveness
define void @test6(ptr addrspace(1) %obj) gc "statepoint-example" {
; CHECK-LABEL: @test6
entry:
@@ -16,7 +15,6 @@ next: ; preds = %entry
; CHECK-NEXT: gc.relocate
; CHECK-NEXT: @consume(ptr addrspace(1) %obj.relocated)
; CHECK-NEXT: @consume(ptr addrspace(1) %obj.relocated)
-; Need to delete unreachable gc.statepoint call
%obj2 = phi ptr addrspace(1) [ %obj, %entry ]
call void @foo() [ "deopt"() ]
call void (...) @consume(ptr addrspace(1) %obj2)
@@ -24,11 +22,10 @@ next: ; preds = %entry
ret void
}
+; Need to delete unreachable gc.statepoint call
define void @test7() gc "statepoint-example" {
; CHECK-LABEL: test7
; CHECK-NOT: gc.statepoint
-; Need to delete unreachable gc.statepoint invoke - tested separately given
-; a correct implementation could only remove the instructions, not the block
ret void
unreached: ; preds = %unreached
@@ -38,6 +35,8 @@ unreached: ; preds = %unreached
br label %unreached
}
+; Need to delete unreachable gc.statepoint invoke - tested separately given
+; a correct implementation could only remove the instructions, not the block
define void @test8() gc "statepoint-example" personality ptr undef {
; CHECK-LABEL: test8
; CHECK-NOT: gc.statepoint
diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/relocation.ll b/llvm/test/Transforms/RewriteStatepointsForGC/relocation.ll
index 249b3c1d09ac1..be6e7a9005b04 100644
--- a/llvm/test/Transforms/RewriteStatepointsForGC/relocation.ll
+++ b/llvm/test/Transforms/RewriteStatepointsForGC/relocation.ll
@@ -88,11 +88,11 @@ else_branch: ; preds = %bci_0
; CHECK-LABEL: else_branch:
; CHECK: gc.statepoint
; CHECK: gc.relocate
-; We need to end up with a single relocation phi updated from both paths
call void @foo() [ "deopt"() ]
br label %join
join: ; preds = %else_branch, %if_branch
+; We need to end up with a single relocation phi updated from both paths
; CHECK-LABEL: join:
; CHECK: phi ptr addrspace(1)
; CHECK-DAG: [ %arg.relocated, %if_branch ]
@@ -246,11 +246,11 @@ inner-loop: ; preds = %inner-loop, %outer-
outer-inc: ; preds = %inner-loop
; CHECK-LABEL: outer-inc:
-; This test shows why updating just those uses of the original value being
-; relocated dominated by the inserted relocation is not always sufficient.
br label %outer-loop
}
+; This test shows why updating just those uses of the original value being
+; relocated dominated by the inserted relocation is not always sufficient.
define ptr addrspace(1) @test7(ptr addrspace(1) %obj, ptr addrspace(1) %obj2, i1 %condition) gc "statepoint-example" {
; CHECK-LABEL: @test7
entry:
@@ -269,6 +269,9 @@ join: ; preds = %callbb, %entry
; CHECK: phi ptr addrspace(1)
; CHECK-DAG: [ %obj, %entry ]
; CHECK-DAG: [ %obj2.relocated, %callbb ]
+ ; This is a phi outside the dominator region of the new defs inserted by
+ ; the safepoint, BUT we can't stop the search here or we miss the second
+ ; phi below.
%phi1 = phi ptr addrspace(1) [ %obj, %entry ], [ %obj2, %callbb ]
br label %join2
More information about the llvm-commits
mailing list