Fix a bug in codegen block order; add a step to choose the order

Codegen had a bug where if you emit a block in astubs that
has a getNext() which is also in astubs, it would omit the jump.
However, it might visit another block in a first, which is allowed to
put code in astubs.  Generally the block order was just defined by the
order that happened to be in the block list by the time we get there.

Change it to emit blocks in the RPO defined by sortCfg for now, after
partitioning them into groups for a and astubs.  This fixes the bug,
and at least makes codegen use a defined order.  We later will
probably want to see about whether we may be able to avoid more jumps
with smarter layout.

hphp/runtime/base/runtime_option.h

@@ -439,6 +439,7 @@ public:
   F(bool, HHIRDirectExit,              true)                            \
   F(bool, HHIRDeadCodeElim,            true)                            \
   F(bool, HHIRPredictionOpts,          true)                            \
+  F(bool, HHIRStressCodegenBlocks,     false)                           \
   /* DumpBytecode =1 dumps user php, =2 dumps systemlib & user php */   \
   F(int32_t, DumpBytecode,             0)                               \
   F(bool, DumpTC,                      false)                           \

hphp/runtime/vm/translator/hopt/block.h

@@ -64,8 +64,31 @@ struct Block : boost::noncopyable {
   void        addEdge(IRInstruction* jmp);
   void        removeEdge(IRInstruction* jmp);
 
+  /*
+   * Returns true if this block is the main trace exit.  This block
+   * post-dominates all main trace blocks.
+   *
+   * Currently there is only ever a single main trace exit.
+   */
+  bool isMainExit() const;
+
+  /*
+   * Returns true if this block is part of the main trace.  I.e. it is
+   * post-dominated by a block with isMainExit() == true.
+   */
   bool isMain() const;
 
+  /*
+   * Returns: !getTaken() && !getNext().
+   */
+  bool isExit() const;
+
+  /*
+   * Returns whether this block is the initial entry block for the
+   * tracelet.
+   */
+  bool isEntry() const { return getId() == 0; }
+
   // return the last instruction in the block
   IRInstruction* back() const {
     assert(!m_instrs.empty());
@@ -181,8 +204,6 @@ struct Block : boost::noncopyable {
 typedef std::list<Block*> BlockList;
 typedef std::forward_list<Block*> BlockPtrList;
 
-
-
 }}
 
 #endif

hphp/runtime/vm/translator/hopt/codegen.cpp

@@ -16,9 +16,10 @@
 
 #include "hphp/runtime/vm/translator/hopt/codegen.h"
 
-#include <string.h>
+#include <cstring>
 
 #include "folly/ScopeGuard.h"
+#include "folly/Format.h"
 #include "hphp/util/trace.h"
 #include "hphp/util/util.h"
 
@@ -45,6 +46,7 @@
 #include "hphp/runtime/vm/translator/hopt/linearscan.h"
 #include "hphp/runtime/vm/translator/hopt/nativecalls.h"
 #include "hphp/runtime/vm/translator/hopt/print.h"
+#include "hphp/runtime/vm/translator/hopt/layout.h"
 
 using HPHP::Transl::TCA;
 using namespace HPHP::Transl::TargetCache;
@@ -454,7 +456,9 @@ CALL_OPCODE(EmptyElem)
 #undef PUNT_OPCODE
 
 // Thread chain of patch locations using the 4 byte space in each jmp/jcc
-void prependPatchAddr(CodegenState& state, Block* block, TCA patchAddr) {
+static void prependPatchAddr(CodegenState& state,
+                             Block* block,
+                             TCA patchAddr) {
   auto &patches = state.patches;
   ssize_t diff = patches[block] ? (patchAddr - (TCA)patches[block]) : 0;
   assert(deltaFits(diff, sz::dword));
@@ -462,33 +466,30 @@ void prependPatchAddr(CodegenState& state, Block* block, TCA patchAddr) {
   patches[block] = patchAddr;
 }
 
-Address CodeGenerator::emitFwdJcc(Asm& a, ConditionCode cc, Block* target) {
-  assert(target);
-  Address start = a.code.frontier;
-  a.jcc(cc, a.code.frontier);
-  TCA immPtr = a.code.frontier - 4;
-  prependPatchAddr(m_state, target, immPtr);
-  return start;
-}
+static void emitFwdJmp(Asm& a, Block* target, CodegenState& state) {
+  if (auto addr = state.addresses[target]) {
+    return a.jmpAuto(addr);
+  }
 
-Address CodeGenerator::emitFwdJmp(Asm& a, Block* target, CodegenState& state) {
-  Address start = a.code.frontier;
+  // TODO(#2101926): it'd be nice to get 1-byte forward jumps here
   a.jmp(a.code.frontier);
   TCA immPtr = a.code.frontier - 4;
   prependPatchAddr(state, target, immPtr);
-  return start;
 }
 
-Address CodeGenerator::emitFwdJmp(Asm& a, Block* target) {
-  return emitFwdJmp(a, target, m_state);
+void CodeGenerator::emitFwdJcc(Asm& a, ConditionCode cc, Block* target) {
+  if (auto addr = m_state.addresses[target]) {
+    return a.jccAuto(cc, addr);
+  }
+
+  // TODO(#2101926): it'd be nice to get 1-byte forward jumps here
+  a.jcc(cc, a.code.frontier);
+  TCA immPtr = a.code.frontier - 4;
+  prependPatchAddr(m_state, target, immPtr);
 }
 
-Address CodeGenerator::emitFwdJcc(ConditionCode cc, Block* target) {
-  return emitFwdJcc(m_as, cc, target);
-}
-
-Address CodeGenerator::emitFwdJmp(Block* target) {
-  return emitFwdJmp(m_as, target);
+void CodeGenerator::emitFwdJcc(ConditionCode cc, Block* target) {
+  emitFwdJcc(m_as, cc, target);
 }
 
 void emitLoadImm(CodeGenerator::Asm& as, int64_t val, PhysReg dstReg) {
@@ -567,8 +568,8 @@ static void emitStoreReg(CodeGenerator::Asm& as, PhysReg reg, Mem mem) {
   }
 }
 
-void shuffle2(CodeGenerator::Asm& a,
-              PhysReg s0, PhysReg s1, PhysReg d0, PhysReg d1) {
+static void shuffle2(CodeGenerator::Asm& a,
+                     PhysReg s0, PhysReg s1, PhysReg d0, PhysReg d1) {
   assert(s0 != s1);
   if (d0 == s1 && d1 != InvalidReg) {
     assert(d0 != d1);
@@ -2199,17 +2200,8 @@ void checkFrame(ActRec* fp, Cell* sp, bool checkLocals) {
     }
   }
   // We unfortunately can't do the same kind of check for the stack
-  // because it may contain ActRecs.
-#if 0
-  for (Cell* c=sp; c < firstSp; c++) {
-    TypedValue* tv = (TypedValue*)c;
-    assert(tvIsPlausible(tv));
-    DataType t = tv->m_type;
-    if (IS_REFCOUNTED_TYPE(t)) {
-      assert(tv->m_data.pstr->getCount() > 0);
-    }
-  }
-#endif
+  // without knowing about FPI regions, because it may contain
+  // ActRecs.
 }
 
 void traceRet(ActRec* fp, Cell* sp, void* rip) {
@@ -4741,7 +4733,7 @@ void CodeGenerator::cgJmp_(IRInstruction* inst) {
     shuffleArgs(m_as, args);
   }
   if (!m_state.noTerminalJmp_) {
-    emitFwdJmp(inst->getTaken());
+    emitFwdJmp(m_as, inst->getTaken(), m_state);
   }
 }
 
@@ -5119,8 +5111,9 @@ void CodeGenerator::cgVerifyParamCls(IRInstruction* inst) {
   ifThen(m_as, CC_NE, [&]{ cgCallNative(inst); });
 }
 
-void CodeGenerator::emitTraceCall(CodeGenerator::Asm& as, int64_t pcOff,
-                                  Transl::TranslatorX64* tx64) {
+static void emitTraceCall(CodeGenerator::Asm& as,
+                          int64_t pcOff,
+                          Transl::TranslatorX64* tx64) {
   // call to a trace function
   as.mov_imm64_reg((int64_t)as.code.frontier, reg::rcx);
   as.mov_reg64_reg64(rVmFp, reg::rdi);
@@ -5130,6 +5123,11 @@ void CodeGenerator::emitTraceCall(CodeGenerator::Asm& as, int64_t pcOff,
   tx64->emitCall(as, (TCA)traceCallback);
 }
 
+void CodeGenerator::print() const {
+  JIT::print(std::cout, m_curTrace, &m_state.regs, m_state.lifetime,
+             m_state.asmInfo);
+}
+
 static void patchJumps(Asm& as, CodegenState& state, Block* block) {
   void* list = state.patches[block];
   Address labelAddr = as.code.frontier;
@@ -5145,6 +5143,8 @@ static void patchJumps(Asm& as, CodegenState& state, Block* block) {
 }
 
 void CodeGenerator::cgBlock(Block* block, vector<TransBCMapping>* bcMap) {
+  FTRACE(6, "cgBlock: {}\n", block->getId());
+
   for (IRInstruction& instr : *block) {
     IRInstruction* inst = &instr;
     if (inst->op() == Marker) {
@@ -5166,64 +5166,6 @@ void CodeGenerator::cgBlock(Block* block, vector<TransBCMapping>* bcMap) {
   }
 }
 
-void cgTrace(Trace* trace, Asm& amain, Asm& astubs, Transl::TranslatorX64* tx64,
-             vector<TransBCMapping>* bcMap, CodegenState& state) {
-  state.lastMarker = nullptr;
-  if (RuntimeOption::EvalHHIRGenerateAsserts && trace->isMain()) {
-    CodeGenerator::emitTraceCall(amain, trace->getBcOff(), tx64);
-  }
-  auto chooseAs = [&](Block* b) {
-    return b->getHint() != Block::Unlikely ? &amain : &astubs;
-  };
-  auto& blocks = trace->getBlocks();
-  for (auto it = blocks.begin(), end = blocks.end(); it != end;) {
-    Block* block = *it; ++it;
-    Asm* as = chooseAs(block);
-    TCA asmStart = as->code.frontier;
-    TCA astubsStart = astubs.code.frontier;
-    patchJumps(*as, state, block);
-
-    // Grab the next block that will go into this assembler
-    Block* nextThisAs = nullptr;
-    for (auto next = it; next != end; ++next) {
-      if (chooseAs(*next) == as) {
-        nextThisAs = *next;
-        break;
-      }
-    }
-
-    // If the block ends with a Jmp_ to the next block for this
-    // assembler, it doesn't need to actually emit a jmp.
-    IRInstruction* last = block->back();
-    state.noTerminalJmp_ =
-      last->op() == Jmp_ && last->getTaken() == nextThisAs;
-
-    CodeGenerator cg(trace, *as, astubs, tx64, state);
-    if (state.asmInfo) {
-      state.asmInfo->asmRanges[block] = TcaRange(asmStart, as->code.frontier);
-    }
-    cg.cgBlock(block, bcMap);
-    Block* next = block->getNext();
-    if (next && next != nextThisAs) {
-      // if there's a fallthrough block and it's not the next thing
-      // going into this assembler, then emit a jump to it.
-      CodeGenerator::emitFwdJmp(*as, next, state);
-    }
-    if (state.asmInfo) {
-      state.asmInfo->asmRanges[block] = TcaRange(asmStart, as->code.frontier);
-      if (as != &astubs) {
-        state.asmInfo->astubRanges[block] = TcaRange(astubsStart,
-                                                     astubs.code.frontier);
-      }
-    }
-  }
-}
-
-void CodeGenerator::print() const {
-  JIT::print(std::cout, m_curTrace, &m_state.regs, m_state.lifetime,
-             m_state.asmInfo);
-}
-
 /*
  * Compute and save registers that are live *across* each inst, not including
  * registers whose lifetimes end at inst, nor registers defined by inst.
@@ -5254,7 +5196,6 @@ LiveRegs computeLiveRegs(const IRFactory* factory, const RegAllocInfo& regs,
   return live_regs;
 }
 
-// select instructions for the trace and its exits
 void genCodeForTrace(Trace* trace,
                      CodeGenerator::Asm& as,
                      CodeGenerator::Asm& astubs,
@@ -5267,9 +5208,77 @@ void genCodeForTrace(Trace* trace,
   assert(trace->isMain());
   LiveRegs live_regs = computeLiveRegs(irFactory, regs, trace->front());
   CodegenState state(irFactory, regs, live_regs, lifetime, asmInfo);
-  cgTrace(trace, as, astubs, tx64, bcMap, state);
-  for (Trace* exit : trace->getExitTraces()) {
-    cgTrace(exit, astubs, astubs, tx64, nullptr, state);
+
+  // Returns: whether a block has already been emitted.
+  auto isEmitted = [&](Block* block) { return state.addresses[block]; };
+
+  /*
+   * Emit the given block on the supplied assembler.  The `nextBlock'
+   * is the nextBlock that will be emitted on this assembler.  If is
+   * not the fallthrough block, emit a patchable jump to the
+   * fallthrough block.
+   */
+  auto emitBlock = [&](Asm& a, Block* block, Block* nextBlock) {
+    assert(!isEmitted(block));
+
+    FTRACE(6, "cgBlock {} on {}\n", block->getId(),
+           &a == &astubs ? "astubs" : "a");
+
+    auto const aStart      = a.code.frontier;
+    auto const astubsStart = astubs.code.frontier;
+    patchJumps(a, state, block);
+    state.addresses[block] = aStart;
+
+    // If the block ends with a Jmp_ and the next block is going to be
+    // its target, we don't need to actually emit it.
+    IRInstruction* last = block->back();
+    state.noTerminalJmp_ = last->op() == Jmp_ && nextBlock == last->getTaken();
+
+    CodeGenerator cg(trace, a, astubs, tx64, state);
+    if (state.asmInfo) {
+      state.asmInfo->asmRanges[block] = TcaRange(aStart, a.code.frontier);
+    }
+
+    cg.cgBlock(block, bcMap);
+    state.lastMarker = nullptr;
+    if (auto next = block->getNext()) {
+      if (next != nextBlock) {
+        // If there's a fallthrough block and it's not the next thing
+        // going into this assembler, then emit a jump to it.
+        emitFwdJmp(a, next, state);
+      }
+    }
+
+    if (state.asmInfo) {
+      state.asmInfo->asmRanges[block] = TcaRange(aStart, a.code.frontier);
+      if (&a != &astubs) {
+        state.asmInfo->astubRanges[block] = TcaRange(astubsStart,
+                                                     astubs.code.frontier);
+      }
+    }
+  };
+
+  if (RuntimeOption::EvalHHIRGenerateAsserts && trace->isMain()) {
+    emitTraceCall(as, trace->getBcOff(), tx64);
+  }
+
+  auto const linfo = layoutBlocks(trace, *irFactory);
+
+  for (auto it = linfo.blocks.begin(); it != linfo.astubsIt; ++it) {
+    Block* nextBlock = boost::next(it) != linfo.astubsIt
+      ? *boost::next(it) : nullptr;
+    emitBlock(as, *it, nextBlock);
+  }
+  for (auto it = linfo.astubsIt; it != linfo.blocks.end(); ++it) {
+    Block* nextBlock = boost::next(it) != linfo.blocks.end()
+      ? *boost::next(it) : nullptr;
+    emitBlock(astubs, *it, nextBlock);
+  }
+
+  if (debug) {
+    for (Block* UNUSED block : linfo.blocks) {
+      assert(isEmitted(block));
+    }
   }
 }
 

hphp/runtime/vm/translator/hopt/codegen.h

@@ -78,6 +78,7 @@ struct CodegenState {
                const LiveRegs& liveRegs, const LifetimeInfo* lifetime,
                AsmInfo* asmInfo)
     : patches(factory, nullptr)
+    , addresses(factory, nullptr)
     , lastMarker(nullptr)
     , regs(regs)
     , liveRegs(liveRegs)
@@ -85,8 +86,10 @@ struct CodegenState {
     , asmInfo(asmInfo)
   {}
 
-  // Each block has a list of addresses to patch
+  // Each block has a list of addresses to patch, and an address if
+  // it's already been emitted.
   StateVector<Block,void*> patches;
+  StateVector<Block,TCA> addresses;
 
   // Keep track of the most recent Marker instruction we've seen in the
   // current trace (even across blocks).
@@ -134,10 +137,6 @@ struct CodeGenerator {
 
   void cgBlock(Block* block, vector<TransBCMapping>* bcMap);
 
-  static void emitTraceCall(CodeGenerator::Asm& as, int64_t pcOff,
-                            Transl::TranslatorX64* tx64);
-  static Address emitFwdJmp(Asm& as, Block* target, CodegenState& state);
-
 private:
   Address cgInst(IRInstruction* inst);
 
@@ -310,10 +309,8 @@ private:
   void cgIterInitCommon(IRInstruction* inst, bool isInitK);
   void cgLdFuncCachedCommon(IRInstruction* inst);
   TargetCache::CacheHandle cgLdClsCachedCommon(IRInstruction* inst);
-  Address emitFwdJcc(ConditionCode cc, Block* target);
-  Address emitFwdJcc(Asm& a, ConditionCode cc, Block* target);
-  Address emitFwdJmp(Asm& as, Block* target);
-  Address emitFwdJmp(Block* target);
+  void emitFwdJcc(ConditionCode cc, Block* target);
+  void emitFwdJcc(Asm& a, ConditionCode cc, Block* target);
   void emitContVarEnvHelperCall(SSATmp* fp, TCA helper);
   const Func* getCurFunc() const;
   Class*      getCurClass() const { return getCurFunc()->cls(); }

hphp/runtime/vm/translator/hopt/ir.cpp

@@ -886,10 +886,18 @@ void Block::removeEdge(IRInstruction* jmp) {
   assert((node->next = nullptr, true));
 }
 
+bool Block::isMainExit() const {
+  return isMain() && isExit();
+}
+
 bool Block::isMain() const {
   return m_trace->isMain();
 }
 
+bool Block::isExit() const {
+  return !getTaken() && !getNext();
+}
+
 bool IRInstruction::cseEquals(IRInstruction* inst) const {
   assert(canCSE());
 

hphp/runtime/vm/translator/hopt/layout.cpp

@@ -0,0 +1,116 @@
+/*
+   +----------------------------------------------------------------------+
+   | HipHop for PHP                                                       |
+   +----------------------------------------------------------------------+
+   | Copyright (c) 2010- Facebook, Inc. (http://www.facebook.com)         |
+   +----------------------------------------------------------------------+
+   | This source file is subject to version 3.01 of the PHP license,      |
+   | that is bundled with this package in the file LICENSE, and is        |
+   | available through the world-wide-web at the following url:           |
+   | http://www.php.net/license/3_01.txt                                  |
+   | If you did not receive a copy of the PHP license and are unable to   |
+   | obtain it through the world-wide-web, please send a note to          |
+   | license@php.net so we can mail you a copy immediately.               |
+   +----------------------------------------------------------------------+
+*/
+#include "hphp/runtime/vm/translator/hopt/layout.h"
+
+#include "hphp/util/trace.h"
+#include "hphp/runtime/vm/translator/hopt/cfg.h"
+#include "hphp/runtime/vm/translator/hopt/state_vector.h"
+
+namespace HPHP { namespace JIT {
+
+TRACE_SET_MOD(hhir);
+
+//////////////////////////////////////////////////////////////////////
+
+namespace {
+
+void postorderWalk(smart::vector<Block*>& out,
+                   StateVector<Block,bool>& visited,
+                   Block* block) {
+  if (visited[block]) return;
+  visited[block] = true;
+  if (auto t = block->getTaken()) postorderWalk(out, visited, t);
+  if (auto n = block->getNext())  postorderWalk(out, visited, n);
+  out.push_back(block);
+}
+
+smart::vector<Block*> rpoForCodegen(const IRFactory& factory, Block* head) {
+  StateVector<Block,bool> visited(&factory, false);
+  smart::vector<Block*> ret;
+  ret.reserve(factory.numBlocks());
+  postorderWalk(ret, visited, head);
+  std::reverse(ret.begin(), ret.end());
+  return ret;
+}
+
+}
+
+//////////////////////////////////////////////////////////////////////
+
+/*
+ * Currently we have very limited control flow in any given tracelet,
+ * so this just selects an appropriate reverse post order on the
+ * blocks, and partitions the unlikely ones to astubs.
+ */
+LayoutInfo layoutBlocks(Trace* trace, const IRFactory& irFactory) {
+  LayoutInfo ret;
+  ret.blocks = rpoForCodegen(irFactory, trace->getBlocks().front());
+
+  // Optionally stress test by randomizing the positions.
+  if (RuntimeOption::EvalHHIRStressCodegenBlocks) {
+    auto seed = std::chrono::system_clock::now().time_since_epoch().count();
+    std::default_random_engine gen(seed);
+    std::random_shuffle(ret.blocks.begin() + 1, ret.blocks.end(),
+                        [&](int i) { return gen() % i; });
+  }
+
+  // Partition into a and astubs, without changing relative order.
+  ret.astubsIt = std::stable_partition(
+    ret.blocks.begin(), ret.blocks.end(),
+    [&] (Block* b) {
+      return b->isMain() && b->getHint() != Block::Unlikely;
+    }
+  );
+
+  if (HPHP::Trace::moduleEnabled(HPHP::Trace::hhir, 5)) {
+    std::string str = "CG Layout:";
+
+    auto printRegion = [&] (const char* what,
+                            smart::vector<Block*>::iterator& it,
+                            smart::vector<Block*>::iterator stop) {
+      folly::toAppend(what, &str);
+      for (; it != stop; ++it) {
+        folly::toAppend((*it)->getId(), &str);
+        folly::toAppend(" ", &str);
+      }
+    };
+
+    auto it = ret.blocks.begin();
+    printRegion("\n       a: ", it, ret.astubsIt);
+    printRegion("\n  astubs: ", it, ret.blocks.end());
+
+    HPHP::Trace::traceRelease("%s\n", str.c_str());
+  }
+
+  /*
+   * No matter what happens above, it's going to be very broken if the
+   * entry block isn't first, and it's going to perform poorly if the
+   * main exit isn't the last block in a.  Assert these.
+   *
+   * Note: this isn't the case if the main exit contains a return, but
+   * we can revisit that later.
+   */
+  if (!RuntimeOption::EvalHHIRStressCodegenBlocks) {
+    always_assert(ret.blocks.front()->isEntry());
+    always_assert((*boost::prior(ret.astubsIt))->isMainExit());
+  }
+
+  return ret;
+}
+
+//////////////////////////////////////////////////////////////////////
+
+}}

hphp/runtime/vm/translator/hopt/layout.h

@@ -0,0 +1,50 @@
+/*
+   +----------------------------------------------------------------------+
+   | HipHop for PHP                                                       |
+   +----------------------------------------------------------------------+
+   | Copyright (c) 2010- Facebook, Inc. (http://www.facebook.com)         |
+   +----------------------------------------------------------------------+
+   | This source file is subject to version 3.01 of the PHP license,      |
+   | that is bundled with this package in the file LICENSE, and is        |
+   | available through the world-wide-web at the following url:           |
+   | http://www.php.net/license/3_01.txt                                  |
+   | If you did not receive a copy of the PHP license and are unable to   |
+   | obtain it through the world-wide-web, please send a note to          |
+   | license@php.net so we can mail you a copy immediately.               |
+   +----------------------------------------------------------------------+
+*/
+#ifndef incl_HPHP_JIT_LAYOUT_H_
+#define incl_HPHP_JIT_LAYOUT_H_
+
+#include "hphp/runtime/base/memory/memory_manager.h"
+#include "hphp/runtime/vm/translator/hopt/ir.h"
+#include "hphp/runtime/vm/translator/hopt/block.h"
+
+namespace HPHP { namespace JIT {
+
+//////////////////////////////////////////////////////////////////////
+
+/*
+ * Information about where to position the blocks in a trace.
+ *
+ * The blocks are listed in the order they should be positioned, with
+ * astubsIt pointing at the place we've split the blocks between a and
+ * astubs.
+ */
+struct LayoutInfo {
+  smart::vector<Block*> blocks;
+  smart::vector<Block*>::iterator astubsIt;
+};
+
+/*
+ * Determine the order that blocks should be emitted for codegen.  The
+ * goal is to minimize branching and put related blocks close to each
+ * other.
+ */
+LayoutInfo layoutBlocks(Trace*, const IRFactory&);
+
+//////////////////////////////////////////////////////////////////////
+
+}}
+
+#endif

hphp/runtime/vm/translator/hopt/print.cpp

@@ -118,7 +118,12 @@ void printSrcs(std::ostream& os, const IRInstruction* inst,
 void printLabel(std::ostream& os, const Block* block) {
   os << color(ANSI_COLOR_MAGENTA);
   os << "L" << block->getId();
-  if (block->getHint() == Block::Unlikely) os << "<Unlikely>";
+  switch (block->getHint()) {
+  case Block::Unlikely:    os << "<Unlikely>"; break;
+  case Block::Likely:      os << "<Likely>"; break;
+  default:
+    break;
+  }
   os << color(ANSI_COLOR_END);
 }
 
@@ -265,6 +270,45 @@ void print(const Trace* trace) {
   print(std::cout, trace);
 }
 
+// Print unlikely blocks at the end in normal generation.  If we have
+// asmInfo, order the blocks based on how they were layed out.
+static smart::vector<Block*> getBlocks(const Trace* trace,
+                                       const AsmInfo* asmInfo) {
+  smart::vector<Block*> blocks;
+
+  if (!asmInfo) {
+    smart::vector<Block*> unlikely;
+    for (Block* block : trace->getBlocks()) {
+      if (block->getHint() == Block::Unlikely) {
+        unlikely.push_back(block);
+      } else {
+        blocks.push_back(block);
+      }
+    }
+    for (Trace* e : trace->getExitTraces()) {
+      unlikely.insert(unlikely.end(),
+                      e->getBlocks().begin(),
+                      e->getBlocks().end());
+    }
+    blocks.insert(blocks.end(), unlikely.begin(), unlikely.end());
+    return blocks;
+  }
+
+  blocks.assign(trace->getBlocks().begin(), trace->getBlocks().end());
+  for (Trace* e : trace->getExitTraces()) {
+    blocks.insert(blocks.end(), e->getBlocks().begin(), e->getBlocks().end());
+  }
+  std::sort(
+    blocks.begin(),
+    blocks.end(),
+    [&] (Block* a, Block* b) {
+      return asmInfo->asmRanges[a].begin() < asmInfo->asmRanges[b].begin();
+    }
+  );
+
+  return blocks;
+}
+
 void print(std::ostream& os, const Trace* trace, const RegAllocInfo* regs,
            const LifetimeInfo* lifetime, const AsmInfo* asmInfo) {
   static const int kIndent = 4;
@@ -272,18 +316,13 @@ void print(std::ostream& os, const Trace* trace, const RegAllocInfo* regs,
                                  .printEncoding(dumpIREnabled(kExtraLevel))
                                  .color(color(ANSI_COLOR_BROWN)));
 
-  // Print unlikely blocks at the end
-  BlockList blocks, unlikely;
-  for (Block* block : trace->getBlocks()) {
-    if (block->getHint() == Block::Unlikely) {
-      unlikely.push_back(block);
-    } else {
-      blocks.push_back(block);
+  for (Block* block : getBlocks(trace, asmInfo)) {
+    if (!block->isMain()) {
+      os << "\n" << color(ANSI_COLOR_GREEN)
+         << "    -------  Exit Trace  -------"
+         << color(ANSI_COLOR_END) << '\n';
     }
-  }
-  blocks.splice(blocks.end(), unlikely);
 
-  for (Block* block : blocks) {
     TcaRange blockRange = asmInfo ? asmInfo->asmRanges[block] :
                           TcaRange(nullptr, nullptr);
     for (auto it = block->begin(); it != block->end();) {
@@ -373,16 +412,10 @@ void print(std::ostream& os, const Trace* trace, const RegAllocInfo* regs,
       }
     }
   }
-
-  for (auto* exitTrace : trace->getExitTraces()) {
-    os << "\n" << color(ANSI_COLOR_GREEN)
-       << "    -------  Exit Trace  -------"
-       << color(ANSI_COLOR_END) << '\n';
-    print(os, exitTrace, regs, lifetime, asmInfo);
-  }
 }
 
-void dumpTraceImpl(const Trace* trace, std::ostream& out,
+void dumpTraceImpl(const Trace* trace,
+                   std::ostream& out,
                    const RegAllocInfo* regs,
                    const LifetimeInfo* lifetime,
                    const AsmInfo* asmInfo) {

hphp/runtime/vm/translator/hopt/state_vector.h

@@ -18,7 +18,6 @@
 #define incl_HPHP_JIT_STATE_VECTOR_H_
 
 #include "hphp/runtime/base/memory/memory_manager.h"
-
 #include "hphp/runtime/vm/translator/hopt/irfactory.h"
 
 namespace HPHP {  namespace JIT {
@@ -42,7 +41,7 @@ struct StateVector {
 
   StateVector(const IRFactory* factory, Info init)
     : m_factory(factory)
-    , m_info(numIds(factory, (Key*)nullptr), init)
+    , m_info(numIds(factory, static_cast<Key*>(nullptr)), init)
     , m_init(init) {
   }
 

hphp/runtime/vm/translator/translator-x64.h

@@ -316,8 +316,10 @@ private:
                                const NormalizedInstruction& i,
                                ScratchReg& output,
                                ptrdiff_t ch);
+public:
   void emitCall(Asm& a, TCA dest, bool killRegs=false);
   void emitCall(Asm& a, Call call, bool killRegs=false);
+private:
 
   /* Continuation-related helpers */
   static bool mapContParams(ContParamMap& map, const Func* origFunc,

hphp/util/asm-x64.h

@@ -1144,6 +1144,24 @@ struct X64Assembler {
     emitCJ8(instr_jcc, cond, (ssize_t)dest);
   }
 
+  void jmpAuto(CodeAddress dest) {
+    auto delta = dest - (code.frontier + 2);
+    if (deltaFits(delta, sz::byte)) {
+      jmp8(dest);
+    } else {
+      jmp(dest);
+    }
+  }
+
+  void jccAuto(ConditionCode cc, CodeAddress dest) {
+    auto delta = dest - (code.frontier + 2);
+    if (deltaFits(delta, sz::byte)) {
+      jcc8(cc, dest);
+    } else {
+      jcc(cc, dest);
+    }
+  }
+
   void call(Label&);
   void jmp(Label&);
   void jmp8(Label&);