fix stack mapping code to do as many passes as necessary

Previously, we had been doing exactly two passes over the event log to
caculate the stack object reference map at each trace point.  It turns
out the correct number of passes depends on how many incorrect
assumptions we make about what the stack looks like at instructions with
multiple predecessors (i.e. targets of jumps and branches).

Each time we detect we've made one or more incorrect assumptions during
a pass, we must do another pass to correct those assumptions.  That pass
may in turn reveal further incorrect assumptions, and so on.

src/compile.cpp

@@ -496,6 +496,7 @@ class Context {
   TraceElement* traceLog;
   uint16_t* visitTable;
   uintptr_t* rootTable;
+  bool dirtyRoots;
   Vector eventLog;
   MyProtector protector;
 };
@@ -3537,7 +3538,8 @@ calculateFrameMaps(MyThread* t, Context* context, uintptr_t* originalRoots,
   // that position, or the contents of that position are as yet
   // unknown.
 
-  while (eventIndex < context->eventLog.length()) {
+  unsigned length = context->eventLog.length();
+  while (eventIndex < length) {
     Event e = static_cast<Event>(context->eventLog.get(eventIndex++));
     switch (e) {
     case PushEvent: {
@@ -3549,6 +3551,7 @@ calculateFrameMaps(MyThread* t, Context* context, uintptr_t* originalRoots,
 
     case IpEvent: {
       ip = context->eventLog.get2(eventIndex);
+      eventIndex += 2;
 
       if (DebugFrameMaps) {
         fprintf(stderr, "      roots at ip %3d: ", ip);
@@ -3560,7 +3563,20 @@ calculateFrameMaps(MyThread* t, Context* context, uintptr_t* originalRoots,
 
       if (context->visitTable[ip] > 1) {
         for (unsigned wi = 0; wi < mapSize; ++wi) {
-          tableRoots[wi] &= roots[wi];
+          uintptr_t newRoots = tableRoots[wi] & roots[wi];
+
+          if ((eventIndex == length
+               or context->eventLog.get(eventIndex) == PopEvent)
+              and newRoots != tableRoots[wi])
+          {
+            if (DebugFrameMaps) {
+              fprintf(stderr, "dirty roots!\n");
+            }
+
+            context->dirtyRoots = true;
+          }
+
+          tableRoots[wi] = newRoots;
           roots[wi] &= tableRoots[wi];
         }
 
@@ -3572,92 +3588,20 @@ calculateFrameMaps(MyThread* t, Context* context, uintptr_t* originalRoots,
       } else {
         memcpy(tableRoots, roots, mapSize * BytesPerWord);
       }
-
-      eventIndex += 2;
     } break;
 
     case MarkEvent: {
       unsigned i = context->eventLog.get2(eventIndex);
-      markBit(roots, i);
-
       eventIndex += 2;
+
+      markBit(roots, i);
     } break;
 
     case ClearEvent: {
       unsigned i = context->eventLog.get2(eventIndex);
+      eventIndex += 2;
+
       clearBit(roots, i);
-
-      eventIndex += 2;
-    } break;
-
-    case TraceEvent: {
-      eventIndex += BytesPerWord;
-    } break;
-
-    default: abort(t);
-    }
-  }
-
-  return eventIndex;
-}
-
-unsigned
-updateTraceElements(MyThread* t, Context* context, uintptr_t* originalRoots,
-                    unsigned eventIndex)
-{
-  unsigned mapSize = frameMapSizeInWords(t, context->method);
-
-  uintptr_t roots[mapSize];
-  if (originalRoots) {
-    memcpy(roots, originalRoots, mapSize * BytesPerWord);
-  } else {
-    memset(roots, 0, mapSize * BytesPerWord);
-  }
-
-  int32_t ip = -1;
-
-  while (eventIndex < context->eventLog.length()) {
-    Event e = static_cast<Event>(context->eventLog.get(eventIndex++));
-    switch (e) {
-    case PushEvent: {
-      eventIndex = updateTraceElements(t, context, roots, eventIndex);
-    } break;
-
-    case PopEvent:
-      return eventIndex;
-
-    case IpEvent: {
-      ip = context->eventLog.get2(eventIndex);
-
-      if (DebugFrameMaps) {
-        fprintf(stderr, "        map at ip %3d: ", ip);
-        printSet(*roots);
-        fprintf(stderr, "\n");
-      }
-
-      if (context->visitTable[ip] > 1) {
-        uintptr_t* tableRoots = context->rootTable + (ip * mapSize);
-        
-        for (unsigned wi = 0; wi < mapSize; ++wi) {
-          roots[wi] &= tableRoots[wi];
-        }
-      }
-
-      eventIndex += 2;
-    } break;
-
-    case MarkEvent: {
-      unsigned i = context->eventLog.get2(eventIndex);
-      markBit(roots, i);
-
-      eventIndex += 2;
-    } break;
-
-    case ClearEvent: {
-      unsigned i = context->eventLog.get2(eventIndex);
-      clearBit(roots, i);
-
-      eventIndex += 2;
     } break;
 
     case TraceEvent: {
@@ -3811,6 +3755,7 @@ compile(MyThread* t, Context* context)
   compile(t, &frame, 0);
   if (UNLIKELY(t->exception)) return 0;
 
+  context->dirtyRoots = false;
   unsigned eventIndex = calculateFrameMaps(t, context, 0, 0);
 
   object eht = codeExceptionHandlerTable(t, methodCode(t, context->method));
@@ -3867,7 +3812,10 @@ compile(MyThread* t, Context* context)
     }
   }
 
-  updateTraceElements(t, context, 0, 0);
+  while (context->dirtyRoots) {
+    context->dirtyRoots = false;
+    calculateFrameMaps(t, context, 0, 0);
+  }
 
   return finish(t, context, 0);
 }

test/GC.java

@@ -87,6 +87,23 @@ public class GC {
     System.gc();
   }
 
+  private static void stackMap6(boolean predicate) {
+    if (predicate) {
+      int a = 42;
+    } else {
+      Object a = null;
+    }
+
+    if (predicate) {
+      noop();
+    } else {
+      Object a = null;
+    }
+
+    noop();
+    System.gc();
+  }
+
   public static void main(String[] args) {
     Object[] array = new Object[1024 * 1024];
     array[0] = new Object();
@@ -119,6 +136,9 @@ public class GC {
 
     stackMap5(true);
     stackMap5(false);
+
+    stackMap6(true);
+    stackMap6(false);
   }
 
 }