passes: handle alloca kill and def in the same block

It is possible that an alloca is killed then defined in the same
block. One example is loop unrolling. For example, a loop

b1:
  def x
  goto b2
b2:
  kill x
  goto b1

after unrolling:

b1.1:
  def x
  goto b2
b2.1:
  kill x
  // continue on b1.2
  def x
  goto b2.2
b2.2
  kill x

b2.1 contains a kill and a def. x is defined at the end of b2.1.
We should record this information in the alloca def set.

Currently, we subtract the kill set from the def set. This
information is lost. Fix this by removing x from the kill set
when we see a def.

Change-Id: If8eef510f0fa14c0d7f5bb0dcd840dac19fedd4c
Reviewed-on: https://go-review.googlesource.com/c/163202
Reviewed-by: Than McIntosh <thanm@google.com>
diff --git a/passes/GoStatepoints.cpp b/passes/GoStatepoints.cpp
index 50cd653..0d14919 100644
--- a/passes/GoStatepoints.cpp
+++ b/passes/GoStatepoints.cpp
@@ -2312,6 +2312,19 @@
                   SetVector<Value *> &AllocaDefs,
                   SetVector<Value *> &AllocaKills,
                   DefiningValueMapTy &DVCache) {
+  // Iterate forwards over the instructions, record the defs and kills
+  // of allocas.
+  // Notes on the special cases about def and kill appearing in the same
+  // block:
+  // - kill after def:
+  //   Record the kill but don't remove it from def set, since we will
+  //   subtract the kill set anyway. And when a slot is initialized and
+  //   then killed in the same block, we don't lose information.
+  // - def after kill:
+  //   The def overrides the kill, i.e. remove it from the kill set
+  //   (unless we see a kill again later). So we have the information
+  //   that the slot is initialized at the end of the block, even after
+  //   we subtract the kill set.
   for (auto &I : make_range(Begin, End)) {
     // skip Phi ?
     if (isa<PHINode>(I))
@@ -2319,16 +2332,20 @@
 
     if (StoreInst *SI = dyn_cast<StoreInst>(&I)){
       Value *V = SI->getPointerOperand();
-      if (Value *Base = isTrackedAlloca(V, DVCache))
+      if (Value *Base = isTrackedAlloca(V, DVCache)) {
         AllocaDefs.insert(Base);
+        AllocaKills.remove(Base);
+      }
       continue;
     }
 
     if (CallInst *CI = dyn_cast<CallInst>(&I)){
       if (CI->hasStructRetAttr()) {
         Value *V = CI->getOperand(0);
-        if (Value *Base = isTrackedAlloca(V, DVCache))
+        if (Value *Base = isTrackedAlloca(V, DVCache)) {
           AllocaDefs.insert(Base);
+          AllocaKills.remove(Base);
+        }
       }
       if (Function *Fn = CI->getCalledFunction())
         switch (Fn->getIntrinsicID()) {
@@ -2337,17 +2354,16 @@
         case Intrinsic::memset: {
           // We're writing to the first arg.
           Value *V = CI->getOperand(0);
-          if (Value *Base = isTrackedAlloca(V, DVCache))
+          if (Value *Base = isTrackedAlloca(V, DVCache)) {
             AllocaDefs.insert(Base);
+            AllocaKills.remove(Base);
+          }
           break;
         }
         case Intrinsic::lifetime_end: {
           Value *V = CI->getOperand(1);
           if (Value *Base = isTrackedAlloca(V, DVCache)) {
             AllocaKills.insert(Base);
-            // We don't remove it from def set, since we will subtract
-            // the kill set anyway. And when a slot is initialized and
-            // then killed in the same block, we don't lose information.
           }
           break;
         }
@@ -2360,8 +2376,10 @@
     if (InvokeInst *II = dyn_cast<InvokeInst>(&I)) {
       if (II->hasStructRetAttr()) {
         Value *V = II->getOperand(0);
-        if (Value *Base = isTrackedAlloca(V, DVCache))
+        if (Value *Base = isTrackedAlloca(V, DVCache)) {
           AllocaDefs.insert(Base);
+          AllocaKills.remove(Base);
+        }
       }
       continue;
     }