From 0ae5908b618805921515ea129e0f02e70a5d6069 Mon Sep 17 00:00:00 2001
From: Guilherme Ottoni <ottoni@fb.com>
Date: Fri, 19 Jul 2013 10:02:11 -0700
Subject: [PATCH] Eliminate unconditional jump from main trace to exit trace
 before DCE

I ran into this problem with the region JIT.  Without this fix, an
IncRef may appear to be dead in the main trace during ref-counting
optimization (performed along with DCE).
---
 hphp/runtime/vm/jit/dce.cpp       | 18 ++++++++++++----
 hphp/runtime/vm/jit/jump-opts.cpp | 36 +++++++++++++++----------------
 hphp/runtime/vm/jit/opt.h         |  3 ++-
 3 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/hphp/runtime/vm/jit/dce.cpp b/hphp/runtime/vm/jit/dce.cpp
index 78f87a7d2..6c25c33cb 100644
--- a/hphp/runtime/vm/jit/dce.cpp
+++ b/hphp/runtime/vm/jit/dce.cpp
@@ -296,10 +296,14 @@ void optimizeRefCount(IRTrace* trace, DceState& state, UseCounts& uses) {
 
 /*
  * Sink IncRefs consumed off trace.
- * Assumptions: Flow graph must not have critical edges, and the instructions
- * have been annotated already by the DCE algorithm.  This pass uses
- * the REFCOUNT_CONSUMED* flags to copy IncRefs from the main trace to each
- * exit trace that consumes the incremented pointer.
+ * Assumptions:
+ *   a) Flow graph must not have critical edges.
+ *   b) The main trace doesn't unconditionally jump to an exit trace.
+ *   c) The instructions have been annotated already by the DCE algorithm.
+ *      This pass uses the REFCOUNT_CONSUMED* flags to copy IncRefs from the
+ *      main trace to each exit trace that consumes the incremented pointer.
+ *
+ * Algorithm:
  * 1. toSink = {}
  * 2. iterate forwards over the main trace:
  *    * when a movable IncRef is found, insert into toSink list and mark
@@ -316,6 +320,8 @@ void optimizeRefCount(IRTrace* trace, DceState& state, UseCounts& uses) {
 void sinkIncRefs(IRTrace* trace, IRFactory* irFactory, DceState& state) {
   assert(trace->isMain());
 
+  assert(trace->back()->back()->op() != Jmp_);
+
   auto copyPropTrace = [] (IRTrace* trace) {
     forEachInst(trace, copyProp);
   };
@@ -542,6 +548,10 @@ void eliminateDeadCode(IRTrace* trace, IRFactory* irFactory) {
   BlockList blocks = removeUnreachable(trace, irFactory);
   removeEmptyExitTraces();
 
+  // Ensure that main trace doesn't unconditionally jump to an exit
+  // trace.  This invariant is needed by the ref-counting optimization.
+  eliminateUnconditionalJump(trace, irFactory);
+
   // mark the essential instructions and add them to the initial
   // work list; this will also mark reachable exit traces. All
   // other instructions marked dead.
diff --git a/hphp/runtime/vm/jit/jump-opts.cpp b/hphp/runtime/vm/jit/jump-opts.cpp
index 34c39158f..03ec8a144 100644
--- a/hphp/runtime/vm/jit/jump-opts.cpp
+++ b/hphp/runtime/vm/jit/jump-opts.cpp
@@ -26,25 +26,9 @@ namespace HPHP { namespace JIT {
 
 TRACE_SET_MOD(hhir);
 
-namespace {
-
 //////////////////////////////////////////////////////////////////////
 
-// If main trace ends with an unconditional jump, and the target is not
-// reached by any other branch, then copy the target of the jump to the
-// end of the trace
-void elimUnconditionalJump(IRTrace* trace, IRFactory* irFactory) {
-  Block* lastBlock = trace->back();
-  auto lastInst = lastBlock->backIter(); // iterator to last instruction
-  IRInstruction& jmp = *lastInst;
-  if (jmp.op() == Jmp_ && jmp.taken()->numPreds() == 1) {
-    Block* target = jmp.taken();
-    lastBlock->splice(lastInst, target, target->skipHeader(), target->end(),
-                      lastInst->marker());
-    jmp.convertToNop();         // unlink it from its Edge
-    lastBlock->erase(lastInst); // delete the jmp
-  }
-}
+namespace {
 
 Block* findMainExitBlock(IRTrace* trace, IRFactory* irFactory) {
   assert(trace->isMain());
@@ -214,8 +198,24 @@ void optimizeSideExits(IRTrace* trace, IRFactory* irFactory) {
 
 //////////////////////////////////////////////////////////////////////
 
+// If main trace ends with an unconditional jump, and the target is not
+// reached by any other branch, then copy the target of the jump to the
+// end of the trace
+void eliminateUnconditionalJump(IRTrace* trace, IRFactory* irFactory) {
+  Block* lastBlock = trace->back();
+  auto lastInst = lastBlock->backIter(); // iterator to last instruction
+  IRInstruction& jmp = *lastInst;
+  if (jmp.op() == Jmp_ && jmp.taken()->numPreds() == 1) {
+    Block* target = jmp.taken();
+    lastBlock->splice(lastInst, target, target->skipHeader(), target->end(),
+                      lastInst->marker());
+    jmp.convertToNop();         // unlink it from its Edge
+    lastBlock->erase(lastInst); // delete the jmp
+  }
+}
+
 void optimizeJumps(IRTrace* trace, IRFactory* irFactory) {
-  elimUnconditionalJump(trace, irFactory);
+  eliminateUnconditionalJump(trace, irFactory);
 
   if (RuntimeOption::EvalHHIRDirectExit) {
     optimizeCondTraceExit(trace, irFactory);
diff --git a/hphp/runtime/vm/jit/opt.h b/hphp/runtime/vm/jit/opt.h
index 27765c161..c51f4d9b8 100644
--- a/hphp/runtime/vm/jit/opt.h
+++ b/hphp/runtime/vm/jit/opt.h
@@ -32,8 +32,9 @@ class IRInstruction;
  */
 void optimizeMemoryAccesses(IRTrace*, IRFactory*);
 void optimizePredictions(IRTrace*, IRFactory*);
-void eliminateDeadCode(IRTrace*, IRFactory*);
 void optimizeJumps(IRTrace*, IRFactory*);
+void eliminateUnconditionalJump(IRTrace*, IRFactory*);
+void eliminateDeadCode(IRTrace*, IRFactory*);
 
 /*
  * Run all the optimization passes.