[llvm] r230047 - [GC, RewriteStatepointsForGC] Style cleanup and bug fix

[Philip Reames]

Author: reames
Date: Fri Feb 20 13:51:56 2015
New Revision: 230047

URL: http://llvm.org/viewvc/llvm-project?rev=230047&view=rev
Log:
[GC, RewriteStatepointsForGC] Style cleanup and bug fix

When doing style cleanup, I noticed a minor bug in this code.  If we have a pointer that we think is unused after a statepoint and thus doesn't need relocation, we store a null pointer into the alloca we're about to promote.  This helps turn a mistake in liveness analysis into an easily debuggable crash.  It turned out this code had never been updated to handle invoke statepoints.  

There's no test for this.  Without a bug in liveness, it appears impossible to make this trigger in a way which is visible in the resulting IR.  We might store the null, but when promoting the alloca, there will be no uses and thus nothing to test against.  Suggestions on how to test are very welcome.



Modified:
    llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp?rev=230047&r1=230046&r2=230047&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp Fri Feb 20 13:51:56 2015
@@ -1538,21 +1538,41 @@ static void relocationViaAlloca(
 #ifndef NDEBUG
     // As a debuging aid, pretend that an unrelocated pointer becomes null at
     // the gc.statepoint.  This will turn some subtle GC problems into slightly
-    // easy to debug SEGVs
+    // easier to debug SEGVs
+    SmallVector<AllocaInst *, 64> ToClobber;
     for (auto Pair : allocaMap) {
-      Value *def = Pair.first;
-      Value *alloca = Pair.second;
+      Value *Def = Pair.first;
+      AllocaInst *Alloca = cast<AllocaInst>(Pair.second);
 
       // This value was relocated
-      if (visitedLiveValues.count(def)) {
+      if (visitedLiveValues.count(Def)) {
         continue;
       }
-
-      auto PT = cast<PointerType>(def->getType());
-      Constant *CPN = ConstantPointerNull::get(PT);
-      StoreInst *store = new StoreInst(CPN, alloca);
-      store->insertBefore(info.SafepointBounds.second);
+      ToClobber.push_back(Alloca);
     }
+
+    Instruction *Statepoint = info.SafepointBounds.first;
+    auto InsertClobbersAt = [&](Instruction *IP) {
+      for (auto *AI : ToClobber) {
+        auto AIType = cast<PointerType>(AI->getType());
+        auto PT = cast<PointerType>(AIType->getElementType());
+        Constant *CPN = ConstantPointerNull::get(PT);
+        StoreInst *store = new StoreInst(CPN, AI);
+        store->insertBefore(IP);
+      }
+    };
+
+    // Insert the clobbering stores.  These may get intermixed with the
+    // gc.results and gc.relocates, but that's fine.  
+    if (auto II = dyn_cast<InvokeInst>(Statepoint)) {
+      InsertClobbersAt(II->getNormalDest()->getFirstInsertionPt());
+      InsertClobbersAt(II->getUnwindDest()->getFirstInsertionPt());
+    } else if (auto CI = dyn_cast<CallInst>(Statepoint)) {
+      BasicBlock::iterator Next(CI);
+      Next++;
+      InsertClobbersAt(Next);
+    } else
+      llvm_unreachable("illegal statepoint instruction type?");
 #endif
   }
   // update use with load allocas and add store for gc_relocated