[RS4GC] Fix some test comments that got clobbered

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"
+declare i32 @spam() gc "statepoint-example"

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:

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

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
-}
+}

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:

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,19 +15,17 @@ 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)
   call void (...) @consume(ptr addrspace(1) %obj)
   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

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