[PATCH] D12583: [RewriteStatepointsForGC] Extend base pointer inference to handle insertelement

[Sanjoy Das via llvm-commits]

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

lgtm with comments inline:


================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:292
@@ -291,3 +291,3 @@
 
 /// Return a base defining value for the 'Index' element of the given vector
 /// instruction 'I'.  If Index is null, returns a BDV for the entire vector
----------------
Please update the comment, now that there is no `Index`.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:860
@@ +859,3 @@
+    // the conflict state.
+    if (isa<InsertElementInst>(I)) {
+      assert(State.isConflict());
----------------
Braces unnecessary here.

Minor style nit:  I'd put the entire implication inside the `assert`, like `assert(!isa<InsertEle>() || State.isConflict())`.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:1006
@@ -1012,3 +1005,3 @@
         // Either conflict or base.
         assert(states.count(Base));
         Base = states[Base].getBase();
----------------
These blocks of code should be pulled into a common lambda or a static helper.  But that's okay to do post-commit.


http://reviews.llvm.org/D12583