[PATCH] D12575: [RewriteStatepointsForGC] Bugfix for change 246133

[Sanjoy Das via llvm-commits]

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

lgtm with minor nits inline.


================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:616
@@ -615,3 +615,3 @@
   Status status;
   Value *base; // non null only if status == base
 };
----------------
Not directly related to this change, but does this invariant actually hold?  We're constructing a `BDVState` below with `status` == `Conflict` and a non-nullptr `base`.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:1048
@@ -1046,1 +1047,3 @@
+    for (User *U : BaseI->users())
       if (auto *UI = dyn_cast<Instruction>(U))
+        if (NewInsts.count(UI) && UI != BaseI)
----------------
Can there be non `Instruction` uses of `BaseI`.  If not, maybe this should be a `cast<>`?

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:1056
@@ -1049,1 +1055,3 @@
+    BaseI->eraseFromParent();
+    states[BDV] = BDVState(BDVState::Conflict, Replacement);
   };
----------------
Can we assert `states.count(BDV)` here?  Or maybe even `states[BDV].base == BaseI`?


http://reviews.llvm.org/D12575