Recycle block local registers in fast pass

Summary:
The register allocator has the ability to honour a memory limit that is
proportional to the product of the number of instructions and basic
blocks in the function being allocated. Unfortunately, functions that
hit this limit by definition have a lot of instructions Even in the
most degenerate case where every block has one instruction, you need
4000 instructions to hit the 10M limit.

This diff tries to improve the quality of generated code in cases where
most values are used within the basic block they are defined in. In such
cases, we currently make the register available after the end of the
block. With this diff, the registers become available after their last
use in the block.

This is useful for functions with extremely large basic blocks, where
the current approach would end up allocating a huge number of registers
since the registers cannot be used within the same block.

Reviewed By: avp

Differential Revision: D59072005

fbshipit-source-id: 83109ad366ac1a0a5445806fce407d3a42ef2232

Author: neildhar

include/hermes/BCGen/RegAlloc.h

@@ -381,9 +381,9 @@ class RegisterAllocator {
   /// predecessor blocks.
   void lowerPhis(ArrayRef<BasicBlock *> order);
 
-  /// Allocate the registers for the instructions in the function. The order of
-  /// the block needs to match the order which we'll use for instruction
-  /// selection.
+  /// Allocate the registers for the instructions in the function. The blocks
+  /// must be in reverse-post-order, and must match the order which we'll use
+  /// for instruction selection.
   void allocate(ArrayRef<BasicBlock *> order);
 
   /// Reserves consecutive registers that will be manually managed by the user.

lib/BCGen/RegAlloc.cpp

@@ -512,19 +512,6 @@ void RegisterAllocator::coalesce(
   }
 }
 
-namespace {
-/// Determines whether the Instruction is ever used outside its BasicBlock.
-bool isBlockLocal(Instruction *inst) {
-  BasicBlock *parent = inst->getParent();
-  for (auto user : inst->getUsers()) {
-    if (parent != user->getParent()) {
-      return false;
-    }
-  }
-  return true;
-}
-} // namespace
-
 void RegisterAllocator::allocateFastPass(ArrayRef<BasicBlock *> order) {
   // Make sure Phis and related Movs get the same register
   for (auto *bb : order) {
@@ -540,26 +527,69 @@ void RegisterAllocator::allocateFastPass(ArrayRef<BasicBlock *> order) {
     }
   }
 
-  llvh::SmallVector<Register, 16> blockLocals;
+  // Bit vector indicating whether a register with a given index is being used
+  // as a block local register.
+  llvh::BitVector blockLocals;
+
+  // List of free block local registers. We have to maintain this outside the
+  // file because we cannot determine interference between local and global
+  // registers. So we have to ensure that the local registers are only reused
+  // for other block-local instructions.
+  llvh::SmallVector<Register, 8> freeBlockLocals;
+
+  // A dummy register used for all instructions that have no users.
+  Register deadReg = file.allocateRegister();
+
+  // Iterate in reverse, so we can cheaply determine whether an instruction
+  // is local, and assign it a register accordingly.
+  for (auto *bb : llvh::reverse(order)) {
+    for (auto &inst : llvh::reverse(*bb)) {
+      if (isAllocated(&inst)) {
+        // If this is using a local register, we know the register is free after
+        // we visit the definition.
+        auto reg = getRegister(&inst);
+        auto idx = reg.getIndex();
+        if (idx < blockLocals.size() && blockLocals.test(idx))
+          freeBlockLocals.push_back(reg);
+      } else {
+        // Unallocated instruction means the result is dead, because all users
+        // are visited first. Allocate a temporary register.
+        assert(!inst.hasUsers());
+        updateRegister(&inst, deadReg);
+      }
 
-  // Then just allocate the rest sequentially, while optimizing the case
-  // where an inst is only ever used in its own block.
-  for (auto *bb : order) {
-    for (auto &inst : *bb) {
-      if (!isAllocated(&inst)) {
-        Register R = file.allocateRegister();
-        updateRegister(&inst, R);
-        if (inst.getNumUsers() == 0) {
-          file.killRegister(R);
-        } else if (isBlockLocal(&inst)) {
-          blockLocals.push_back(R);
+      // Allocate a register to unallocated operands.
+      for (size_t i = 0, e = inst.getNumOperands(); i < e; ++i) {
+        auto *op = llvh::dyn_cast<Instruction>(inst.getOperand(i));
+
+        // Skip if op is not an instruction or already has a register.
+        if (!op || isAllocated(op))
+          continue;
+
+        if (op->getParent() != bb) {
+          // Live across blocks, allocate a global regigster.
+          updateRegister(op, file.allocateRegister());
+          continue;
+        }
+
+        // We know this operand is local because:
+        // 1. The operand is in the same block as this one.
+        // 2. All blocks dominated by this block have been visited.
+        // 3. All users must be dominated by their def, since Phis are
+        //    allocated beforehand.
+        if (!freeBlockLocals.empty()) {
+          updateRegister(op, freeBlockLocals.pop_back_val());
+          continue;
         }
+
+        // No free local register, allocate another one.
+        Register reg = file.allocateRegister();
+        if (blockLocals.size() <= reg.getIndex())
+          blockLocals.resize(reg.getIndex() + 1);
+        blockLocals.set(reg.getIndex());
+        updateRegister(op, reg);
       }
     }
-    for (auto &reg : blockLocals) {
-      file.killRegister(reg);
-    }
-    blockLocals.clear();
   }
 }
 

test/BCGen/HBC/RA/assign_to_argument.js

@@ -17,16 +17,16 @@ function foo(a) {
 
 // CHECK:function global(): any
 // CHECK-NEXT:%BB0:
-// CHECK-NEXT:  $Reg0 = CreateScopeInst (:environment) %VS0: any, empty: any
-// CHECK-NEXT:  $Reg1 = DeclareGlobalVarInst "foo": string
-// CHECK-NEXT:  $Reg1 = CreateFunctionInst (:object) $Reg0, %foo(): functionCode
+// CHECK-NEXT:  $Reg1 = CreateScopeInst (:environment) %VS0: any, empty: any
+// CHECK-NEXT:  $Reg0 = DeclareGlobalVarInst "foo": string
+// CHECK-NEXT:  $Reg1 = CreateFunctionInst (:object) $Reg1, %foo(): functionCode
 // CHECK-NEXT:  $Reg2 = HBCGetGlobalObjectInst (:object)
-// CHECK-NEXT:  $Reg3 = StorePropertyLooseInst $Reg1, $Reg2, "foo": string
-// CHECK-NEXT:  $Reg3 = AllocStackInst (:any) $?anon_0_ret: any
-// CHECK-NEXT:  $Reg4 = HBCLoadConstInst (:undefined) undefined: undefined
-// CHECK-NEXT:  $Reg5 = StoreStackInst $Reg4, $Reg3
-// CHECK-NEXT:  $Reg5 = LoadStackInst (:any) $Reg3
-// CHECK-NEXT:  $Reg6 = ReturnInst $Reg5
+// CHECK-NEXT:  $Reg0 = StorePropertyLooseInst $Reg1, $Reg2, "foo": string
+// CHECK-NEXT:  $Reg1 = AllocStackInst (:any) $?anon_0_ret: any
+// CHECK-NEXT:  $Reg2 = HBCLoadConstInst (:undefined) undefined: undefined
+// CHECK-NEXT:  $Reg0 = StoreStackInst $Reg2, $Reg1
+// CHECK-NEXT:  $Reg1 = LoadStackInst (:any) $Reg1
+// CHECK-NEXT:  $Reg0 = ReturnInst $Reg1
 // CHECK-NEXT:function_end
 
 // CHECK:scope %VS0 []
@@ -35,12 +35,12 @@ function foo(a) {
 
 // CHECK:function foo(a: any): any
 // CHECK-NEXT:%BB0:
-// CHECK-NEXT:  $Reg0 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
-// CHECK-NEXT:  $Reg1 = CreateScopeInst (:environment) %VS1: any, $Reg0
+// CHECK-NEXT:  $Reg1 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
+// CHECK-NEXT:  $Reg1 = CreateScopeInst (:environment) %VS1: any, $Reg1
 // CHECK-NEXT:  $Reg2 = LoadParamInst (:any) %a: any
-// CHECK-NEXT:  $Reg3 = StoreFrameInst $Reg1, $Reg2, [%VS1.a]: any
-// CHECK-NEXT:  $Reg3 = LoadFrameInst (:any) $Reg1, [%VS1.a]: any
-// CHECK-NEXT:  $Reg4 = StoreFrameInst $Reg1, $Reg3, [%VS1.a]: any
-// CHECK-NEXT:  $Reg4 = HBCLoadConstInst (:undefined) undefined: undefined
-// CHECK-NEXT:  $Reg5 = ReturnInst $Reg4
+// CHECK-NEXT:  $Reg0 = StoreFrameInst $Reg1, $Reg2, [%VS1.a]: any
+// CHECK-NEXT:  $Reg2 = LoadFrameInst (:any) $Reg1, [%VS1.a]: any
+// CHECK-NEXT:  $Reg0 = StoreFrameInst $Reg1, $Reg2, [%VS1.a]: any
+// CHECK-NEXT:  $Reg1 = HBCLoadConstInst (:undefined) undefined: undefined
+// CHECK-NEXT:  $Reg0 = ReturnInst $Reg1
 // CHECK-NEXT:function_end

test/BCGen/HBC/RA/callee.js

@@ -19,20 +19,20 @@ function foo(x) {
 
 // CHECK:function global(): any
 // CHECK-NEXT:%BB0:
-// CHECK-NEXT:  $Reg0 = CreateScopeInst (:environment) %VS0: any, empty: any
-// CHECK-NEXT:  $Reg1 = DeclareGlobalVarInst "sink": string
-// CHECK-NEXT:  $Reg1 = DeclareGlobalVarInst "foo": string
-// CHECK-NEXT:  $Reg1 = CreateFunctionInst (:object) $Reg0, %sink(): functionCode
+// CHECK-NEXT:  $Reg1 = CreateScopeInst (:environment) %VS0: any, empty: any
+// CHECK-NEXT:  $Reg0 = DeclareGlobalVarInst "sink": string
+// CHECK-NEXT:  $Reg0 = DeclareGlobalVarInst "foo": string
+// CHECK-NEXT:  $Reg2 = CreateFunctionInst (:object) $Reg1, %sink(): functionCode
+// CHECK-NEXT:  $Reg3 = HBCGetGlobalObjectInst (:object)
+// CHECK-NEXT:  $Reg0 = StorePropertyLooseInst $Reg2, $Reg3, "sink": string
+// CHECK-NEXT:  $Reg1 = CreateFunctionInst (:object) $Reg1, %foo(): functionCode
 // CHECK-NEXT:  $Reg2 = HBCGetGlobalObjectInst (:object)
-// CHECK-NEXT:  $Reg3 = StorePropertyLooseInst $Reg1, $Reg2, "sink": string
-// CHECK-NEXT:  $Reg3 = CreateFunctionInst (:object) $Reg0, %foo(): functionCode
-// CHECK-NEXT:  $Reg4 = HBCGetGlobalObjectInst (:object)
-// CHECK-NEXT:  $Reg5 = StorePropertyLooseInst $Reg3, $Reg4, "foo": string
-// CHECK-NEXT:  $Reg5 = AllocStackInst (:any) $?anon_0_ret: any
-// CHECK-NEXT:  $Reg6 = HBCLoadConstInst (:undefined) undefined: undefined
-// CHECK-NEXT:  $Reg7 = StoreStackInst $Reg6, $Reg5
-// CHECK-NEXT:  $Reg7 = LoadStackInst (:any) $Reg5
-// CHECK-NEXT:  $Reg8 = ReturnInst $Reg7
+// CHECK-NEXT:  $Reg0 = StorePropertyLooseInst $Reg1, $Reg2, "foo": string
+// CHECK-NEXT:  $Reg1 = AllocStackInst (:any) $?anon_0_ret: any
+// CHECK-NEXT:  $Reg2 = HBCLoadConstInst (:undefined) undefined: undefined
+// CHECK-NEXT:  $Reg0 = StoreStackInst $Reg2, $Reg1
+// CHECK-NEXT:  $Reg1 = LoadStackInst (:any) $Reg1
+// CHECK-NEXT:  $Reg0 = ReturnInst $Reg1
 // CHECK-NEXT:function_end
 
 // CHECK:scope %VS0 []
@@ -41,16 +41,16 @@ function foo(x) {
 
 // CHECK:function sink(x: any, y: any, z: any): any
 // CHECK-NEXT:%BB0:
-// CHECK-NEXT:  $Reg0 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
-// CHECK-NEXT:  $Reg1 = CreateScopeInst (:environment) %VS1: any, $Reg0
+// CHECK-NEXT:  $Reg1 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
+// CHECK-NEXT:  $Reg1 = CreateScopeInst (:environment) %VS1: any, $Reg1
 // CHECK-NEXT:  $Reg2 = LoadParamInst (:any) %x: any
-// CHECK-NEXT:  $Reg3 = StoreFrameInst $Reg1, $Reg2, [%VS1.x]: any
-// CHECK-NEXT:  $Reg3 = LoadParamInst (:any) %y: any
-// CHECK-NEXT:  $Reg4 = StoreFrameInst $Reg1, $Reg3, [%VS1.y]: any
-// CHECK-NEXT:  $Reg4 = LoadParamInst (:any) %z: any
-// CHECK-NEXT:  $Reg5 = StoreFrameInst $Reg1, $Reg4, [%VS1.z]: any
-// CHECK-NEXT:  $Reg5 = HBCLoadConstInst (:undefined) undefined: undefined
-// CHECK-NEXT:  $Reg6 = ReturnInst $Reg5
+// CHECK-NEXT:  $Reg0 = StoreFrameInst $Reg1, $Reg2, [%VS1.x]: any
+// CHECK-NEXT:  $Reg2 = LoadParamInst (:any) %y: any
+// CHECK-NEXT:  $Reg0 = StoreFrameInst $Reg1, $Reg2, [%VS1.y]: any
+// CHECK-NEXT:  $Reg2 = LoadParamInst (:any) %z: any
+// CHECK-NEXT:  $Reg0 = StoreFrameInst $Reg1, $Reg2, [%VS1.z]: any
+// CHECK-NEXT:  $Reg1 = HBCLoadConstInst (:undefined) undefined: undefined
+// CHECK-NEXT:  $Reg0 = ReturnInst $Reg1
 // CHECK-NEXT:function_end
 
 // CHECK:scope %VS0 []
@@ -59,17 +59,17 @@ function foo(x) {
 
 // CHECK:function foo(x: any): any
 // CHECK-NEXT:%BB0:
-// CHECK-NEXT:  $Reg0 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
-// CHECK-NEXT:  $Reg1 = CreateScopeInst (:environment) %VS1: any, $Reg0
-// CHECK-NEXT:  $Reg2 = LoadParamInst (:any) %x: any
-// CHECK-NEXT:  $Reg3 = StoreFrameInst $Reg1, $Reg2, [%VS1.x]: any
-// CHECK-NEXT:  $Reg3 = LoadFrameInst (:any) $Reg1, [%VS1.x]: any
-// CHECK-NEXT:  $Reg4 = LoadPropertyInst (:any) $Reg3, "sink": string
-// CHECK-NEXT:  $Reg5 = HBCLoadConstInst (:undefined) undefined: undefined
-// CHECK-NEXT:  $Reg6 = HBCLoadConstInst (:number) 1: number
-// CHECK-NEXT:  $Reg7 = HBCLoadConstInst (:number) 2: number
-// CHECK-NEXT:  $Reg8 = HBCLoadConstInst (:number) 3: number
-// CHECK-NEXT:  $Reg9 = HBCCallNInst (:any) $Reg4, empty: any, empty: any, $Reg5, $Reg3, $Reg6, $Reg7, $Reg8
-// CHECK-NEXT:  $Reg9 = HBCLoadConstInst (:undefined) undefined: undefined
-// CHECK-NEXT:  $Reg10 = ReturnInst $Reg9
+// CHECK-NEXT:  $Reg3 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
+// CHECK-NEXT:  $Reg3 = CreateScopeInst (:environment) %VS1: any, $Reg3
+// CHECK-NEXT:  $Reg1 = LoadParamInst (:any) %x: any
+// CHECK-NEXT:  $Reg0 = StoreFrameInst $Reg3, $Reg1, [%VS1.x]: any
+// CHECK-NEXT:  $Reg3 = LoadFrameInst (:any) $Reg3, [%VS1.x]: any
+// CHECK-NEXT:  $Reg1 = LoadPropertyInst (:any) $Reg3, "sink": string
+// CHECK-NEXT:  $Reg2 = HBCLoadConstInst (:undefined) undefined: undefined
+// CHECK-NEXT:  $Reg4 = HBCLoadConstInst (:number) 1: number
+// CHECK-NEXT:  $Reg5 = HBCLoadConstInst (:number) 2: number
+// CHECK-NEXT:  $Reg6 = HBCLoadConstInst (:number) 3: number
+// CHECK-NEXT:  $Reg0 = HBCCallNInst (:any) $Reg1, empty: any, empty: any, $Reg2, $Reg3, $Reg4, $Reg5, $Reg6
+// CHECK-NEXT:  $Reg1 = HBCLoadConstInst (:undefined) undefined: undefined
+// CHECK-NEXT:  $Reg0 = ReturnInst $Reg1
 // CHECK-NEXT:function_end