[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:1620
@@ -1619,1 +1619,3 @@
   for (Value *L : LiveSet) {
+    Value *Base = PointerToBase.find(L)->second;
+    if (isa<Constant>(Base))
----------------
reames wrote:
> I think the general approach of filtering the liveset here can work, but this is the wrong place for it.  In particular, it's after the vector splitting and re-materialization logic.  The vector splitting probably isn't an issue - I'm about to revise/remove that anyways - but the remat logic definitely has coupling with the relocation logic.  
> 
> I think you probably should pull this out as a separate step between the recomputation of liveness after base pointer rewriting and the first use of the new liveness in splitVectorValues.  
> 
> I'll also require much better comments.  :)  For instance, why does simply rewriting the live value as a constant-expr fail?  (i.e. why is the right approach?)
> 
> p.s. A possible alternate approach would be to stop filtering constants from the liveset.  In that case, we would have the live constant base in the liveset.  Have you considered this option?
Agreed, filterung the liveset earlier makes sense. I'll also add a comment.

I'm not sure what you mean by "rewriting the live value as a constant-expr". Please clarify if that's still relevant for my next diff.

Adding the constant base to the liveset would probably solve the issue, too. However that would make the liveset bigger and would still need filtering later, at least for my custom stack map format which doesn't support constants.


http://reviews.llvm.org/D15556