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

[Philip Reames via llvm-commits]

reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM w/minor comments addressed.


================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2390
@@ -2389,1 +2389,3 @@
 
+  // It is possible that non-constant live variables have a constant base.  In
+  // this case we can remove it from the liveset.  We already don't add
----------------
Can you give a small example here?  It would make it easier to understand.  Possibly: "for instance, a variable offset into a constant global"

================
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.
----------------
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.  

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2394
@@ +2393,3 @@
+  // Also, they won't move if the base is constant.
+  // Note that the relocation placement code relies on the following code
+  // because it expects the base to be in the liveset, which isn't true if the
----------------
relies on this filtering for correctness as it expects...

================
Comment at: test/Transforms/RewriteStatepointsForGC/base-pointers-12.ll:16
@@ +15,3 @@
+  ret i8 %load
+}
+
----------------
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?


http://reviews.llvm.org/D15556