[PATCH] D15556: [RS4GC] Fix crash in the case that a live variable has a constant base.

[Manuel Jacob via llvm-commits]

mjacob added inline comments.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2392
@@ +2391,3 @@
+  // this case we can remove it from the liveset.  We already don't add
+  // constants to the liveset because we assume they won't move at runtime.
+  // Also, they won't move if the base is constant.
----------------
reames wrote:
> and that the GC doesn't otherwise need to know about them.  
> 
> p.s. Does your GC actually have constant references?  Or are these just small offsets off of null?  That's the only case we've seen to date.  
Yes, my GC has constant references. GC objects in constants are immortal (they can't be deallocated), so they don't have to be reported by the statepoint mechanism. There is a separate mechanism that tracks references from immortal objects to potentially non-immortal objects.

================
Comment at: test/Transforms/RewriteStatepointsForGC/base-pointers-12.ll:16
@@ +15,3 @@
+  ret i8 %load
+}
+
----------------
reames wrote:
> Can you add one or two other examples?  For instance, a variable offset GEP from a global or a variable offset from a null pointer down a dead path?
Should they go to another file?

Regarding "variable offset GEP from a global": the simple version of this with just the GEP live over the statepoint doesn't crash without the patch because of rematerialization. I can (a) add a more complex example which isn't handled by rematerialization or (b) check that it doesn't get rematerialized. I'm not sure which would be more useful.

Regarding "variable offset from a null pointer down a dead path": first, what exactly do you mean by "dead"? Statically dead paths are removed by RS4GC. Can you give an simple example of a dynamically, but not statically, dead path? I hope I use the correct terminology here. Also, a constant null pointer as a base will fire an assert `assert(!isa<ConstantPointerNull>(base) ...` in `findBasePointers()`.


http://reviews.llvm.org/D15556