Remove kYieldInital instruction

Summary: This is now just a call + a yield value - it's straight forward to simplify now that 3.10 is gone.

Reviewed By: mpage

Differential Revision: D103313543

fbshipit-source-id: 9c187ad601d11c11db777b27b440aea8e443bd58

Author: DinoV

cinderx/Jit/codegen/autogen.cpp

@@ -735,82 +735,6 @@ void translateLoadThreadState(Environ* env, const Instruction* instr) {
 #endif
 }
 
-void translateYieldInitial(Environ* env, const Instruction* instr) {
-#if defined(CINDER_X86_64)
-  arch::Builder* as = env->as;
-
-  // Load tstate into RDI for call to
-  // JITRT_UnlinkGenFrameAndReturnGenDataFooter.
-
-  // Consider avoiding reloading the 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.
-  PhyLocation tstate = instr->getInput(0)->getStackSlot();
-  as->mov(x86::rdi, x86::ptr(x86::rbp, tstate.loc));
-
-  emitCall(
-      *env,
-      reinterpret_cast<uint64_t>(JITRT_UnlinkGenFrameAndReturnGenDataFooter),
-      instr);
-  // This will return pointers to a generator in RAX and JIT data in RDX.
-
-  // Arbitrary scratch register for use in emitStoreGenYieldPoint(). Any
-  // caller-saved register not used in this scope will do because we're on the
-  // exit path now.
-  auto scratch_r = x86::r9;
-  env->pending_yield_resume_label = as->newLabel();
-  emitStoreGenYieldPoint(
-      as,
-      env,
-      instr,
-      env->pending_yield_resume_label,
-      x86::rdx,
-      scratch_r,
-      false /* is_yield_from */);
-
-  // The jmp to exit and resume label binding are handled by separate
-  // kBranchToYieldExit and kResumeGenYield instructions.
-#elif defined(CINDER_AARCH64)
-  arch::Builder* as = env->as;
-
-  // Load tstate into X0 for call to
-  // JITRT_UnlinkGenFrameAndReturnGenDataFooter.
-
-  // Consider avoiding reloading the 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.
-  PhyLocation tstate = instr->getInput(0)->getStackSlot();
-  as->ldr(
-      a64::x0,
-      arch::ptr_resolve(as, arch::fp, tstate.loc, arch::reg_scratch_0));
-
-  emitCall(
-      *env,
-      reinterpret_cast<uint64_t>(JITRT_UnlinkGenFrameAndReturnGenDataFooter),
-      instr);
-  // This will return pointers to a generator in X0 and JIT data in X1.
-
-  // Arbitrary scratch register for use in emitStoreGenYieldPoint(). Any
-  // caller-saved register not used in this scope will do because we're on the
-  // exit path now.
-  auto scratch_r = arch::reg_scratch_0;
-  env->pending_yield_resume_label = as->newLabel();
-  emitStoreGenYieldPoint(
-      as,
-      env,
-      instr,
-      env->pending_yield_resume_label,
-      a64::x1,
-      scratch_r,
-      false /* is_yield_from */);
-
-  // The jmp to exit and resume label binding are handled by separate
-  // kBranchToYieldExit and kResumeGenYield instructions.
-#else
-  CINDER_UNSUPPORTED
-#endif
-}
-
 void translateStoreGenYieldPoint(Environ* env, const Instruction* instr) {
 #if defined(CINDER_X86_64)
   arch::Builder* as = env->as;
@@ -2502,9 +2426,6 @@ void AutoTranslator::translateInstr(Environ* env, const Instruction* instr)
     case Instruction::kLoadThreadState:
       translateLoadThreadState(env, instr);
       return;
-    case Instruction::kYieldInitial:
-      translateYieldInitial(env, instr);
-      return;
     case Instruction::kStoreGenYieldPoint:
       translateStoreGenYieldPoint(env, instr);
       return;
@@ -2952,9 +2873,6 @@ void AutoTranslator::translateInstr(Environ* env, const Instruction* instr)
     case Instruction::kLoadThreadState:
       translateLoadThreadState(env, instr);
       return;
-    case Instruction::kYieldInitial:
-      translateYieldInitial(env, instr);
-      return;
     case Instruction::kStoreGenYieldPoint:
       translateStoreGenYieldPoint(env, instr);
       return;

cinderx/Jit/codegen/environ.h

@@ -52,9 +52,9 @@ struct Environ {
   std::vector<std::pair<int, jit::lir::BasicBlock*>>
       static_typecheck_jt_entries;
 
-  // Resume label shared between StoreGenYieldPoint/YieldInitial and
-  // ResumeGenYield. Created by translateStoreGenYieldPoint or
-  // translateYieldInitial (3.12+), bound by translateResumeGenYield.
+  // Resume label shared between StoreGenYieldPoint and ResumeGenYield.
+  // Created by translateStoreGenYieldPoint, bound by
+  // translateResumeGenYield.
   asmjit::Label pending_yield_resume_label;
 
   // Map from deopt metadata index to the stage 1 deopt exit LIR block.
@@ -148,7 +148,6 @@ struct Environ {
       inline_frame_map;
 
   FrameMode frame_mode;
-  int initial_yield_spill_size_{-1};
 
   int max_arg_buffer_size{0};
 

cinderx/Jit/codegen/gen_asm.cpp

@@ -712,10 +712,6 @@ void* NativeGenerator::getVectorcallEntry() {
   env_.changed_regs = lsalloc.getChangedRegs();
   env_.exit_label = as_->newLabel();
   env_.frame_mode = GetFunction()->frameMode;
-  if (GetFunction()->code->co_flags & kCoFlagsAnyGenerator) {
-    env_.initial_yield_spill_size_ = lsalloc.initialYieldSpillSize();
-  }
-
   JIT_LOGIF(
       getConfig().log.dump_lir,
       "LIR for {} after register allocation:\n{}",

cinderx/Jit/lir/generator.cpp

@@ -1203,10 +1203,9 @@ std::unique_ptr<jit::lir::Function> LIRGenerator::TranslateFunction() {
   // dispatches to resume targets via indirect jump (populated post-regalloc
   // by PopulateResumeEntryBlock). Its successors are the resume blocks,
   // which keeps them reachable during sortBasicBlocks.
-  // We always create this for generators because translateYieldInitial
-  // references gen_resume_entry_label (the label bound to this block).
-  // On pre-3.12, kYieldInitial is monolithic and doesn't add to
-  // resume_blocks_, but still needs the resume entry block to exist.
+  // We always create this for generators because the codegen for
+  // kStoreGenYieldPoint references gen_resume_entry_label (the label
+  // bound to this block).
   if (is_gen_) {
     auto* resume_entry = lir_func_->allocateBasicBlock();
     // Remove from basic_blocks_ immediately — this is a placeholder block
@@ -2318,18 +2317,16 @@ LIRGenerator::TranslatedBlock LIRGenerator::TranslateOneBasicBlock(
       }
       case Opcode::kInitialYield: {
         auto hir_instr = static_cast<const InitialYield*>(&i);
-        // 3.12+: decompose kYieldInitial into setup + branch + resume.
-        // kYieldInitial does the setup and store yield point but does NOT
-        // emit the jmp or bind the resume label.
-        Instruction* instr =
-            bbb.appendInstr(Instruction::kYieldInitial, env_->asm_tstate);
-        finishYield(bbb, instr, hir_instr);
-
-        // Capture the gen object from the return register into a vreg.
-        Instruction* gen_obj = bbb.appendInstr(
+
+        // Unlink the generator frame and get the gen object back.
+        Instruction* gen_obj = bbb.appendCallInstruction(
             OutVReg{},
-            Instruction::kBind,
-            PhyReg{codegen::arch::reg_general_return_loc});
+            JITRT_UnlinkGenFrameAndReturnGenDataFooter,
+            env_->asm_tstate);
+
+        // Store yield point metadata (same as kYieldValue).
+        Instruction* store = bbb.appendInstr(Instruction::kStoreGenYieldPoint);
+        finishYield(bbb, store, hir_instr);
 
         // kBranchToYieldExit: terminates this block.
         auto* branch = bbb.appendInstr(Instruction::kBranchToYieldExit);

cinderx/Jit/lir/instruction.cpp

@@ -292,7 +292,6 @@ bool Instruction::isTerminator() const {
 
 bool Instruction::isAnyYield() const {
   switch (opcode_) {
-    case kYieldInitial:
     case kStoreGenYieldPoint:
     case kStoreGenYieldFromPoint:
       return true;

cinderx/Jit/lir/instruction.h

@@ -171,7 +171,6 @@ enum OperandSizeType {
   X(MovSXD)                                                                   \
   X(IntToBool, false, FlagEffects::kSet, kDefault, 1, {1})                    \
   X(LoadThreadState, false, FlagEffects::kInvalidate, kDefault, 0, {}, 0)     \
-  X(YieldInitial, false, FlagEffects::kInvalidate, kDefault, 0, {}, 1)        \
   X(StoreGenYieldPoint, false, FlagEffects::kInvalidate, kDefault, 0, {}, 1)  \
   X(StoreGenYieldFromPoint,                                                   \
     false,                                                                    \

cinderx/Jit/lir/postalloc.cpp

@@ -905,7 +905,6 @@ RewriteResult rewriteMemoryInputsToReg(instr_iter_t instr_iter) {
     case Instruction::kMovSX:
     case Instruction::kMovSXD:
     case Instruction::kLoadThreadState:
-    case Instruction::kYieldInitial:
     case Instruction::kStoreGenYieldPoint:
     case Instruction::kStoreGenYieldFromPoint:
     case Instruction::kBranchToYieldExit:

cinderx/Jit/lir/regalloc.cpp

@@ -293,14 +293,6 @@ int LinearScanAllocator::getFrameSize() const {
   return -max_stack_slot_;
 }
 
-int LinearScanAllocator::initialYieldSpillSize() const {
-  JIT_CHECK(
-      initial_yield_spill_size_ != -1,
-      "Don't have InitialYield spill size yet");
-
-  return initial_yield_spill_size_;
-}
-
 bool LinearScanAllocator::isPredefinedUsed(const Operand* operand) const {
   auto& block = func_->basicblocks()[0];
 
@@ -596,22 +588,6 @@ void LinearScanAllocator::spillRegistersForYield(int instr_id) {
   reserveRegisters(instr_id, INIT_REGISTERS);
 }
 
-void LinearScanAllocator::computeInitialYieldSpillSize(
-    const UnorderedMap<const Operand*, const LiveInterval*>& mapping) {
-  JIT_CHECK(
-      initial_yield_spill_size_ == -1,
-      "Already computed InitialYield spill size");
-
-  for (auto& pair : mapping) {
-    const LiveInterval* interval = pair.second;
-    if (interval->allocated_loc.is_register()) {
-      continue;
-    }
-    initial_yield_spill_size_ =
-        std::max(initial_yield_spill_size_, -interval->allocated_loc.loc);
-  }
-}
-
 void LinearScanAllocator::reserveCallerSaveRegisters(int instr_id) {
   reserveRegisters(instr_id, CALLER_SAVE_REGS);
 }
@@ -1112,9 +1088,6 @@ void LinearScanAllocator::rewriteLIR() {
           if (instr->output()->isInd()) {
             rewriteInstrOutput(instr, mapping, &last_use_vregs);
           }
-          if (instr->isYieldInitial()) {
-            computeInitialYieldSpillSize(mapping);
-          }
         }
       } else {
         rewriteInstrOutput(instr, mapping, &last_use_vregs);

cinderx/Jit/lir/regalloc.h

@@ -109,8 +109,6 @@ class LinearScanAllocator {
   // Return the number of bytes that should be allocated below the base pointer.
   int getFrameSize() const;
 
-  int initialYieldSpillSize() const;
-
   // returns true if the variables defined in the entry block is
   // used in the function.
   bool isPredefinedUsed(const Operand* operand) const;
@@ -142,8 +140,6 @@ class LinearScanAllocator {
   void calculateLiveIntervals();
 
   void spillRegistersForYield(int instr_id);
-  void computeInitialYieldSpillSize(
-      const UnorderedMap<const Operand*, const LiveInterval*>& mapping);
 
   // Reserve all caller-saved registers for a function call.
   void reserveCallerSaveRegisters(int instr_id);
@@ -273,7 +269,6 @@ class LinearScanAllocator {
   std::vector<PhyLocation> free_stack_slots_;
 
   codegen::PhyRegisterSet changed_regs_;
-  int initial_yield_spill_size_{-1};
 
   // record vreg-to-physical-location mapping at the end of each basic block,
   // which is needed for resolve edges.

cinderx/RuntimeTests/lir_abi_test.cpp

@@ -88,10 +88,6 @@ class LIRABITest : public RuntimeTest {
         environ.block_label_map.emplace(deopt_bb, as.newLabel());
         break;
       }
-      case Instruction::kYieldInitial:
-        environ.code_rt->addDeoptMetadata(DeoptMetadata{});
-        environ.initial_yield_spill_size_ = 16;
-        break;
       default:
         break;
     }
@@ -1299,22 +1295,6 @@ TEST_F(LIRABITest, TestkBitTest_PhyReg_PhyReg) {
   translateInstr(Instruction::kBitTest, makePhyReg(0), Imm{63});
 }
 
-// kYieldInitial ANY
-TEST_F(LIRABITest, TestkYieldInitial) {
-  PyCodeObject code;
-  hir::FrameState frameState(BorrowedRef(&code), nullptr, nullptr, nullptr);
-
-  hir::Register out(0);
-  auto origin = std::unique_ptr<hir::InitialYield>(
-      hir::InitialYield::create(&out, frameState));
-
-  auto tstate = makeStk(-16);
-  auto live_regs = Imm{0};
-  auto deopt_idx = Imm{0};
-
-  translateInstrWithOrigin(
-      Instruction::kYieldInitial, origin.get(), tstate, live_regs, deopt_idx);
-}
 // kSelect R r r r
 TEST_F(LIRABITest, TestkSelect_OutPhyReg_PhyReg_PhyReg_PhyReg) {
 #if defined(CINDER_X86_64)