[RewriteStatepointsForGC] Remove obsolete assertion

[igor-laevsky]

Summary

This is assertion is no longer necessary since we never record constants in the live set anyway. (They are never recorded in the initial live set, and constant bases are removed near line 2119)

[sanjoy]

Drop by comments.

test/Transforms/RewriteStatepointsForGC/constants.ll

107

The bitcast seems unnecessary -- do we need it for reproducing the crash?

[reames]

LGTM w/the test simplifications Sanjoy pointed out.

FTR, I've reviewed the code which handles constant base and derived pointers in the entire file and it appears to be correct. I had thought we'd already removed this assert and am more than a bit surprised to find we hadn't.

lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

findBasePointers(const StatepointLiveSetTy &live,
   for (Value *ptr : live) {
     Value *base = findBasePointer(ptr, DVCache);
     assert(base && "failed to find base pointer");
     PointerToBase[ptr] = base;
     assert((!isa<Instruction>(base) || !isa<Instruction>(ptr) ||
             DT->dominates(cast<Instruction>(base)->getParent(),
                           cast<Instruction>(ptr)->getParent())) &&
            "The base we found better dominate the derived pointer");
-
-    // If you see this trip and like to live really dangerously, the code should
-    // be correct, just with idioms the verifier can't handle.  You can try
-    // disabling the verifier at your own substantial risk.
-    assert(!isa<ConstantPointerNull>(base) &&
-           "the relocation code needs adjustment to handle the relocation of "
-           "a null pointer constant without causing false positives in the "
-           "safepoint ir verifier.");
   }
 }
 
 /// Find the required based pointers (and adjust the live set) for the given
 /// parse point.
 static void findBasePointers(DominatorTree &DT, DefiningValueMapTy &DVCache,
                              const CallSite &CS,
                              PartiallyConstructedSafepointRecord &result) {

test/Transforms/RewriteStatepointsForGC/constants.ll

 ; CHECK-LABEL: @test5
 ; CHECK: gc.statepoint
 ; CHECK-NEXT: %res = extractelement <2 x i8 addrspace(1)*> <i8 addrspace(1)* @G, i8 addrspace(1)* @G>, i32 0
 entry:
   call void @foo()
   %res = extractelement <2 x i8 addrspace(1)*> <i8 addrspace(1)* @G, i8 addrspace(1)* @G>, i32 0
   ret i8 addrspace(1)* %res
 }
+
+define i32 addrspace(1)* @test6(i64 %arg) gc "statepoint-example" {
+entry:
+  ; Don't fail any assertions and don't record null as a live value
+  ; CHECK-LABEL: test6
+  ; CHECK: gc.statepoint
+  ; CHECK-NOT: call {{.*}}gc.relocate
+  %load_addr.i.i.i7 = getelementptr i8, i8 addrspace(1)* null, i64 %arg
+  call void @foo() [ "deopt"() ]
+  %res = bitcast i8 addrspace(1)* %load_addr.i.i.i7 to i32 addrspace(1)*
+  ret i32 addrspace(1)* %res
+}
+
+define i32 addrspace(1)* @test7(i64 %arg) gc "statepoint-example" {
+entry:
+  ; Same as test7 but use regular constant instead of a null
+  ; CHECK-LABEL: test7
+  ; CHECK: gc.statepoint
+  ; CHECK-NOT: call {{.*}}gc.relocate
+  %load_addr.i.i.i7 = getelementptr i8, i8 addrspace(1)* inttoptr (i64 15 to i8 addrspace(1)*), i64 %arg
+  call void @foo() [ "deopt"() ]
+  %res = bitcast i8 addrspace(1)* %load_addr.i.i.i7 to i32 addrspace(1)*
+  ret i32 addrspace(1)* %res
+}