Use JIT_CHECK for lir::Operand getters

Summary: Always enforce that an operand is of the correct type before unwrapping it.

Reviewed By: swtaarrs

Differential Revision: D56532445

fbshipit-source-id: 7f1ab5378033ec662b72ef646a16c3270504fd24

cinderx/Jit/codegen/autogen.cpp

@@ -347,7 +347,6 @@ void emitStoreGenYieldPoint(
 
   auto calc_spill_offset = [&](size_t live_input_n) {
     int mem_loc = yield->getInput(live_input_n)->getStackSlot();
-    JIT_CHECK(mem_loc < 0, "Expected variable to have memory location");
     return mem_loc / kPointerSize;
   };
 
@@ -383,7 +382,6 @@ void emitLoadResumedYieldInputs(
     PhyLocation sent_in_source_loc,
     x86::Gp tstate_reg) {
   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();
@@ -417,7 +415,6 @@ void translateYieldInitial(Environ* env, const Instruction* instr) {
   // 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)->getStackSlot();
-  JIT_CHECK(tstate_loc < 0, "__asm_tstate should be spilled");
   as->mov(x86::rsi, x86::ptr(x86::rbp, tstate_loc));
 
   // Make a generator object to be returned by the epilogue.
@@ -483,15 +480,13 @@ void translateYieldValue(Environ* env, const Instruction* instr) {
 
   // Make sure tstate is in RDI for use in epilogue.
   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)->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));
   }
 
@@ -516,7 +511,6 @@ void translateYieldFrom(Environ* env, const Instruction* instr) {
 
   // Make sure tstate is in RDI for use in epilogue and here.
   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));
 
@@ -525,7 +519,6 @@ void translateYieldFrom(Environ* env, const Instruction* instr) {
   // put initial send value in RSI, the same location future send values will
   // be on resume.
   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;
   as->mov(x86::gpq(send_value_phys_reg), x86::ptr(x86::rbp, send_value_loc));
@@ -558,10 +551,6 @@ void translateYieldFrom(Environ* env, const Instruction* instr) {
 
   // Load sub-iterator into RDI
   int iter_loc = instr->getInput(2)->getStackSlot();
-  JIT_CHECK(
-      iter_loc < 0,
-      "Iter should be spilled. Instead it's in {}",
-      PhyLocation(iter_loc).toString().c_str());
   as->mov(x86::rdi, x86::ptr(x86::rbp, iter_loc));
 
   uint64_t func = reinterpret_cast<uint64_t>(

cinderx/Jit/lir/operand.h

@@ -290,8 +290,11 @@ class Operand : public OperandBase {
   }
 
   int getPhyRegister() const override {
-    JIT_DCHECK(
-        type_ == kReg, "Unable to get physical register from the operand.");
+    JIT_CHECK(
+        type_ == kReg,
+        "Trying to treat operand [type={},val={:#x}] as a physical register",
+        type_,
+        rawValue());
     return std::get<int>(value_);
   }
 
@@ -301,7 +304,11 @@ class Operand : public OperandBase {
   }
 
   int getStackSlot() const override {
-    JIT_DCHECK(type_ == kStack, "Unable to get a memory stack slot.");
+    JIT_CHECK(
+        type_ == kStack,
+        "Trying to treat operand [type={},val={:#x}] as a stack slot",
+        type_,
+        rawValue());
     return std::get<int>(value_);
   }
 
@@ -325,17 +332,21 @@ class Operand : public OperandBase {
       case kStack:
         return getStackSlot();
       default:
-        JIT_DCHECK(
-            false,
-            "Unable to get a physical register or a memory stack slot from the "
-            "operand");
-        break;
+        JIT_ABORT(
+            "Trying to treat operand [type={},val={:#x} as a physical register "
+            "or a stack slot",
+            type_,
+            rawValue());
     }
     return -1;
   }
 
   void* getMemoryAddress() const override {
-    JIT_DCHECK(type_ == kMem, "Unable to get a memory address.");
+    JIT_CHECK(
+        type_ == kMem,
+        "Trying to treat operand [type={},val={:#x}] as a memory address",
+        type_,
+        rawValue());
     return std::get<void*>(value_);
   }
 
@@ -345,7 +356,11 @@ class Operand : public OperandBase {
   }
 
   MemoryIndirect* getMemoryIndirect() const override {
-    JIT_DCHECK(type_ == kInd, "Unable to get a memory indirect.");
+    JIT_CHECK(
+        type_ == kInd,
+        "Trying to treat operand [type={},val={:#x}] as a memory indirect",
+        type_,
+        rawValue());
     return std::get<std::unique_ptr<MemoryIndirect>>(value_).get();
   }
 
@@ -362,7 +377,11 @@ class Operand : public OperandBase {
   }
 
   BasicBlock* getBasicBlock() const override {
-    JIT_DCHECK(type_ == kLabel, "Unable to get a basic block address.");
+    JIT_CHECK(
+        type_ == kLabel,
+        "Trying to treat operand [type={},val={:#x}] as a basic block address",
+        type_,
+        rawValue());
     return std::get<BasicBlock*>(value_);
   }
 
@@ -412,6 +431,24 @@ class Operand : public OperandBase {
   void removeUse(LinkedOperand* use);
 
  private:
+  uint64_t rawValue() const {
+    if (const auto ptr = std::get_if<uint64_t>(&value_)) {
+      return *ptr;
+    } else if (const auto ptr = std::get_if<int>(&value_)) {
+      return static_cast<uint64_t>(*ptr);
+    } else if (const auto ptr = std::get_if<void*>(&value_)) {
+      return reinterpret_cast<uint64_t>(*ptr);
+    } else if (const auto ptr = std::get_if<BasicBlock*>(&value_)) {
+      return reinterpret_cast<uint64_t>(*ptr);
+    } else if (
+        const auto ptr =
+            std::get_if<std::unique_ptr<MemoryIndirect>>(&value_)) {
+      return reinterpret_cast<uint64_t>(ptr->get());
+    }
+
+    JIT_ABORT("Unknown operand value type, has index {}", value_.index());
+  }
+
   Type type_{kNone};
   DataType data_type_{kObject};