Break getPhysRegOrStackSlot() uses into getStackSlot() or getPhysRegi…

…ster()

Summary:
There's a number of uses of getPhysRegOrStackSlot() where the caller already
expects the operand to either be a stack slot or a register, or they're going to
check which one it is right afterwards.

For the former case it's better to use the more specific getter.  For the latter
case, we're leaking the ">=0 means register, <0 means stack slot" abstraction
all throughout the codebase.  It's better to use the isStack() and isReg()
methods in tandem with the getters.

This doesn't get rid of all uses of getPhysRegOrStackSlot().  There are still
some legitimate uses in the register allocator in particular.  But in the future
we should be able to replace them by making LIR operands comparable.

Reviewed By: swtaarrs

Differential Revision: D56523076

fbshipit-source-id: 2a96624bd1413c7b3023d40327e99fed223c2b2b

cinderx/Jit/codegen/autogen.cpp

@@ -346,7 +346,7 @@ void emitStoreGenYieldPoint(
       yield->isYieldFromHandleStopAsyncIteration();
 
   auto calc_spill_offset = [&](size_t live_input_n) {
-    int mem_loc = yield->getInput(live_input_n)->getPhyRegOrStackSlot();
+    int mem_loc = yield->getInput(live_input_n)->getStackSlot();
     JIT_CHECK(mem_loc < 0, "Expected variable to have memory location");
     return mem_loc / kPointerSize;
   };
@@ -382,21 +382,31 @@ void emitLoadResumedYieldInputs(
     const Instruction* instr,
     PhyLocation sent_in_source_loc,
     x86::Gp tstate_reg) {
-  int tstate_loc = instr->getInput(0)->getPhyRegOrStackSlot();
+  int tstate_loc = instr->getInput(0)->getStackSlot();
   JIT_CHECK(tstate_loc < 0, "__asm_tstate should be spilled");
   as->mov(x86::ptr(x86::rbp, tstate_loc), tstate_reg);
 
   const lir::Operand* target = instr->output();
-  if (target->type() != OperandBase::kNone) {
-    PhyLocation target_loc = target->getPhyRegOrStackSlot();
-    if (target_loc.is_register()) {
-      if (target_loc != sent_in_source_loc) {
-        as->mov(x86::gpq(target_loc), x86::gpq(sent_in_source_loc));
-      }
-    } else {
-      as->mov(x86::ptr(x86::rbp, target_loc), x86::gpq(sent_in_source_loc));
+
+  if (target->isStack()) {
+    as->mov(
+        x86::ptr(x86::rbp, target->getStackSlot()),
+        x86::gpq(sent_in_source_loc));
+    return;
+  }
+
+  if (target->isReg()) {
+    PhyLocation target_loc = target->getPhyRegister();
+    if (target_loc != sent_in_source_loc) {
+      as->mov(x86::gpq(target_loc), x86::gpq(sent_in_source_loc));
     }
+    return;
   }
+
+  JIT_CHECK(
+      target->isNone(),
+      "Have an output that isn't a register or a stack slot, {}",
+      target->type());
 }
 
 void translateYieldInitial(Environ* env, const Instruction* instr) {
@@ -406,7 +416,7 @@ void translateYieldInitial(Environ* env, const Instruction* instr) {
   // TODO(jbower) Avoid reloading tstate in from memory if it was already in a
   // register before spilling. Still needs to be in memory though so it can be
   // recovered after calling JITRT_MakeGenObject* which will trash it.
-  int tstate_loc = instr->getInput(0)->getPhyRegOrStackSlot();
+  int tstate_loc = instr->getInput(0)->getStackSlot();
   JIT_CHECK(tstate_loc < 0, "__asm_tstate should be spilled");
   as->mov(x86::rsi, x86::ptr(x86::rbp, tstate_loc));
 
@@ -472,15 +482,15 @@ void translateYieldValue(Environ* env, const Instruction* instr) {
   asmjit::x86::Builder* as = env->as;
 
   // Make sure tstate is in RDI for use in epilogue.
-  int tstate_loc = instr->getInput(0)->getPhyRegOrStackSlot();
+  int tstate_loc = instr->getInput(0)->getStackSlot();
   JIT_CHECK(tstate_loc < 0, "__asm_tstate should be spilled");
   as->mov(x86::rdi, x86::ptr(x86::rbp, tstate_loc));
 
   // Value to send goes to RAX so it can be yielded (returned) by epilogue.
   if (instr->getInput(1)->isImm()) {
     as->mov(x86::rax, instr->getInput(1)->getConstant());
   } else {
-    int value_out_loc = instr->getInput(1)->getPhyRegOrStackSlot();
+    int value_out_loc = instr->getInput(1)->getStackSlot();
     JIT_CHECK(value_out_loc < 0, "value to send out should be spilled");
     as->mov(x86::rax, x86::ptr(x86::rbp, value_out_loc));
   }
@@ -505,7 +515,7 @@ void translateYieldFrom(Environ* env, const Instruction* instr) {
   bool skip_initial_send = instr->isYieldFromSkipInitialSend();
 
   // Make sure tstate is in RDI for use in epilogue and here.
-  int tstate_loc = instr->getInput(0)->getPhyRegOrStackSlot();
+  int tstate_loc = instr->getInput(0)->getStackSlot();
   JIT_CHECK(tstate_loc < 0, "__asm_tstate should be spilled");
   auto tstate_phys_reg = x86::rdi;
   as->mov(tstate_phys_reg, x86::ptr(x86::rbp, tstate_loc));
@@ -514,7 +524,7 @@ void translateYieldFrom(Environ* env, const Instruction* instr) {
   // value to yield and so needs to go into RAX to be returned. Otherwise,
   // put initial send value in RSI, the same location future send values will
   // be on resume.
-  int send_value_loc = instr->getInput(1)->getPhyRegOrStackSlot();
+  int send_value_loc = instr->getInput(1)->getStackSlot();
   JIT_CHECK(send_value_loc < 0, "value to send out should be spilled");
   const auto send_value_phys_reg =
       skip_initial_send ? PhyLocation::RAX : PhyLocation::RSI;
@@ -547,7 +557,7 @@ void translateYieldFrom(Environ* env, const Instruction* instr) {
   // point args.
 
   // Load sub-iterator into RDI
-  int iter_loc = instr->getInput(2)->getPhyRegOrStackSlot();
+  int iter_loc = instr->getInput(2)->getStackSlot();
   JIT_CHECK(
       iter_loc < 0,
       "Iter should be spilled. Instead it's in {}",

cinderx/Jit/lir/postalloc.cpp

@@ -796,7 +796,7 @@ bool PostRegAllocRewrite::insertMoveToRegister(
     } else if (op->isImm()) {
       move->addOperands(Imm(op->getConstant()));
     } else if (op->isStack()) {
-      move->addOperands(Stk(op->getPhyRegOrStackSlot(), op->dataType()));
+      move->addOperands(Stk(op->getStackSlot(), op->dataType()));
     } else if (op->isMem()) {
       JIT_ABORT("Unsupported: div from mem");
     } else {
@@ -830,17 +830,18 @@ void PostRegAllocRewrite::insertMoveToMemoryLocation(
     return;
   }
 
-  PhyLocation loc = operand->getPhyRegOrStackSlot();
-  if (loc.is_memory()) {
+  if (operand->isReg()) {
+    PhyLocation loc = operand->getPhyRegister();
     block->allocateInstrBefore(
-        instr_iter, Instruction::kMove, OutPhyReg(temp), Stk(loc));
-    block->allocateInstrBefore(
-        instr_iter, Instruction::kMove, OutInd(base, index), PhyReg(temp));
+        instr_iter, Instruction::kMove, OutInd(base, index), PhyReg(loc));
     return;
   }
 
+  PhyLocation loc = operand->getStackSlot();
+  block->allocateInstrBefore(
+      instr_iter, Instruction::kMove, OutPhyReg(temp), Stk(loc));
   block->allocateInstrBefore(
-      instr_iter, Instruction::kMove, OutInd(base, index), PhyReg(loc));
+      instr_iter, Instruction::kMove, OutInd(base, index), PhyReg(temp));
 }
 
 // record register-to-memory moves and map between them.