[PATCH] D12004: [RewriteStatepointsForGC] Reduce the number of new instructions for base pointers

[Philip Reames via llvm-commits]

reames added inline comments.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:1032
@@ +1031,3 @@
+    if (auto *BaseI = dyn_cast<Instruction>(Base)) {
+      NewInsts.insert(BaseI);
+      Worklist.insert(BaseI);
----------------
sanjoy wrote:
> reames wrote:
> > sanjoy wrote:
> > > Can't this set be populated as we create the instructions above?
> > It absolutely could.  I was trying to keep the code separate to make it standalone, but I can merge if you'd prefer.  
> I'd prefer that -- it makes it more obvious what happened.
I spoke with Sanjoy offline.  We convinced ourselves that tracking all the new instructions was actually more complex than the current patch, so I'm going ahead with the current scheme.

Two options which might be worth exploring in the future:
- Using a custom IR builder with an insertion callback as InstCombine does.  We'd have to be a bit careful here since we purposely have partially constructed phis/selects.
- Switch the states map to being entirely bi-directional and maintain that all the way through.


http://reviews.llvm.org/D12004