Skip to content

Commit ce7fc02

Browse files
DinoVmeta-codesync[bot]
authored andcommitted
Properly track inlined callers in deopt metadata
Summary: In the face of inlined methods we're not tracking the deopt index the same as we do on x64. This adds tracking for the inline methods so we can have the whole information. Reviewed By: yoney Differential Revision: D99380230 fbshipit-source-id: 5b73e28afd8b7a6e723297a8160982cb3df8b675
1 parent ae73cc0 commit ce7fc02

2 files changed

Lines changed: 119 additions & 20 deletions

File tree

cinderx/Jit/lir/generator.cpp

Lines changed: 112 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -401,10 +401,68 @@ std::unique_ptr<jit::lir::Function> LIRGenerator::TranslateFunction() {
401401
}
402402
#endif
403403

404+
#if defined(CINDER_AARCH64)
405+
// Compute the caller FrameState and active code object at each block's entry
406+
// by walking the CFG. Needed because blocks inside an inlined function may
407+
// not contain BeginInlinedFunction themselves (they're reached via branches).
408+
struct BlockInlineCtx {
409+
const hir::FrameState* caller_fs{nullptr};
410+
BorrowedRef<PyCodeObject> code{nullptr};
411+
};
412+
UnorderedMap<const hir::BasicBlock*, BlockInlineCtx> block_inline_ctx;
413+
if (deopt_idx_addr_ != nullptr) {
414+
auto hir_entry_block = GetHIRFunction()->cfg.entry_block;
415+
std::vector<const hir::BasicBlock*> bfs;
416+
bfs.push_back(hir_entry_block);
417+
block_inline_ctx[hir_entry_block] = {nullptr, func_->code};
418+
for (size_t bi = 0; bi < bfs.size(); ++bi) {
419+
const auto* block = bfs[bi];
420+
auto ctx = block_inline_ctx[block];
421+
for (const auto& instr : *block) {
422+
if (instr.opcode() == Opcode::kBeginInlinedFunction) {
423+
auto bif = static_cast<const BeginInlinedFunction*>(&instr);
424+
ctx.caller_fs = bif->callerFrameState();
425+
ctx.code = bif->code();
426+
} else if (instr.opcode() == Opcode::kEndInlinedFunction) {
427+
if (ctx.caller_fs != nullptr) {
428+
ctx.code = ctx.caller_fs->code;
429+
ctx.caller_fs = ctx.caller_fs->parent;
430+
} else {
431+
ctx.code = nullptr;
432+
}
433+
}
434+
}
435+
auto term = block->GetTerminator();
436+
for (int s = 0, ns = term->numEdges(); s < ns; s++) {
437+
auto succ = term->successor(s);
438+
auto [it, inserted] = block_inline_ctx.insert({succ, ctx});
439+
if (inserted) {
440+
bfs.push_back(succ);
441+
} else {
442+
JIT_DCHECK(
443+
block_inline_ctx[succ].caller_fs == ctx.caller_fs &&
444+
block_inline_ctx[succ].code == ctx.code,
445+
"inconsistent inline context");
446+
}
447+
}
448+
}
449+
}
450+
#endif
451+
404452
UnorderedMap<const hir::BasicBlock*, TranslatedBlock> bb_map;
405453
std::vector<const hir::BasicBlock*> translated;
406454
auto translate_block = [&](const hir::BasicBlock* hir_bb) {
455+
#if defined(CINDER_AARCH64)
456+
auto it = block_inline_ctx.find(hir_bb);
457+
const hir::FrameState* entry_fs =
458+
it != block_inline_ctx.end() ? it->second.caller_fs : nullptr;
459+
BorrowedRef<PyCodeObject> entry_code =
460+
it != block_inline_ctx.end() ? it->second.code : nullptr;
461+
bb_map.emplace(
462+
hir_bb, TranslateOneBasicBlock(hir_bb, entry_fs, entry_code));
463+
#else
407464
bb_map.emplace(hir_bb, TranslateOneBasicBlock(hir_bb));
465+
#endif
408466
translated.emplace_back(hir_bb);
409467
};
410468

@@ -715,23 +773,39 @@ void LIRGenerator::MakeDecref(
715773
}
716774

717775
LIRGenerator::TranslatedBlock LIRGenerator::TranslateOneBasicBlock(
718-
const hir::BasicBlock* hir_bb) {
776+
const hir::BasicBlock* hir_bb,
777+
const jit::hir::FrameState* initial_caller_fs,
778+
BorrowedRef<PyCodeObject> initial_inlined_code) {
719779
BasicBlockBuilder bbb{env_, lir_func_};
720780
BasicBlock* entry_block = bbb.allocateBlock();
721781
bbb.switchBlock(entry_block);
722782

723783
#if defined(CINDER_AARCH64)
724-
// Track the last line number stored in deopt_idx so we can avoid redundant
725-
// updates for Decrefs on the same line.
726784
int last_deopt_line = -1;
785+
const jit::hir::FrameState* caller_fs = initial_caller_fs;
786+
BorrowedRef<PyCodeObject> inlined_code = initial_inlined_code;
727787
#endif
728788

729789
for (auto& i : *hir_bb) {
730790
auto opcode = i.opcode();
731791
bbb.setCurrentInstr(&i);
732792

733793
#if defined(CINDER_AARCH64)
734-
updateDeoptIndex(bbb, i, opcode, last_deopt_line);
794+
if (opcode == Opcode::kBeginInlinedFunction) {
795+
auto bif = static_cast<const BeginInlinedFunction*>(&i);
796+
caller_fs = bif->callerFrameState();
797+
inlined_code = bif->code();
798+
last_deopt_line = -1;
799+
} else if (opcode == Opcode::kEndInlinedFunction) {
800+
if (caller_fs != nullptr) {
801+
inlined_code = caller_fs->code;
802+
caller_fs = caller_fs->parent;
803+
} else {
804+
inlined_code = nullptr;
805+
}
806+
last_deopt_line = -1;
807+
}
808+
updateDeoptIndex(bbb, i, opcode, last_deopt_line, caller_fs, inlined_code);
735809
#endif
736810

737811
switch (opcode) {
@@ -3716,7 +3790,9 @@ void LIRGenerator::updateDeoptIndex(
37163790
BasicBlockBuilder& bbb,
37173791
const jit::hir::Instr& i,
37183792
jit::hir::Opcode opcode,
3719-
int& last_deopt_line) {
3793+
int& last_deopt_line,
3794+
const jit::hir::FrameState* caller_fs,
3795+
BorrowedRef<PyCodeObject> inlined_code) {
37203796
// For any instruction that can deopt, store the deopt index in the
37213797
// outermost frame header before the instruction executes. This allows
37223798
// frame introspection (e.g. sys._current_frames) to recover the current
@@ -3727,28 +3803,46 @@ void LIRGenerator::updateDeoptIndex(
37273803
OutInd{deopt_idx_addr_, 0},
37283804
Instruction::kMove,
37293805
Imm{deopt_id, DataType::k64bit});
3730-
auto code = func_->codeFor(i);
3731-
last_deopt_line = code != nullptr
3732-
? PyCode_Addr2Line(code, i.bytecodeOffset().value())
3733-
: -1;
3806+
// Use inlined_code when inside an inlined function, otherwise codeFor().
3807+
auto code = inlined_code != nullptr ? inlined_code : func_->codeFor(i);
3808+
int bc_off = i.bytecodeOffset().value();
3809+
last_deopt_line =
3810+
(code != nullptr && bc_off >= 0) ? PyCode_Addr2Line(code, bc_off) : -1;
37343811
} else if (
37353812
deopt_idx_addr_ != nullptr &&
37363813
(opcode == Opcode::kDecref || opcode == Opcode::kXDecref ||
37373814
opcode == Opcode::kBatchDecref)) {
37383815
// Decref/XDecref can run arbitrary code via __del__ but don't have
3739-
// deopt metadata. Create a minimal entry so that frame introspection
3816+
// deopt metadata. Create an entry so that frame introspection
37403817
// reports the correct bytecode offset / line number, but only when
37413818
// the line differs from the last deopt point in this basic block.
3742-
auto code = func_->codeFor(i);
3743-
int decref_line = code != nullptr
3744-
? PyCode_Addr2Line(code, i.bytecodeOffset().value())
3745-
: -1;
3819+
//
3820+
// When executing inside an inlined function, we must include entries
3821+
// for all active frames (not just the innermost) so that
3822+
// getUnitCallStackFromDeoptIdx returns a stack matching getUnitFrames.
3823+
// Use inlined_code when inside an inlined function, otherwise codeFor().
3824+
auto code = inlined_code != nullptr ? inlined_code : func_->codeFor(i);
3825+
int bc_off = i.bytecodeOffset().value();
3826+
int decref_line =
3827+
(code != nullptr && bc_off >= 0) ? PyCode_Addr2Line(code, bc_off) : -1;
37463828
if (decref_line != last_deopt_line) {
37473829
DeoptMetadata meta;
3748-
DeoptFrameMetadata frame_meta;
3749-
frame_meta.code = code;
3750-
frame_meta.cause_instr_idx = i.bytecodeOffset();
3751-
meta.frame_meta = {frame_meta};
3830+
int num_callers = 0;
3831+
for (const auto* f = caller_fs; f != nullptr; f = f->parent) {
3832+
num_callers++;
3833+
}
3834+
int num_frames = num_callers + 1;
3835+
meta.frame_meta.resize(num_frames);
3836+
// Innermost frame: the Decref's own code and bytecode offset.
3837+
meta.frame_meta[num_frames - 1].code = code;
3838+
meta.frame_meta[num_frames - 1].cause_instr_idx = i.bytecodeOffset();
3839+
// Caller frames from the FrameState parent chain (outermost first).
3840+
int idx = num_frames - 2;
3841+
for (const auto* f = caller_fs; f != nullptr; f = f->parent) {
3842+
meta.frame_meta[idx].code = f->code;
3843+
meta.frame_meta[idx].cause_instr_idx = f->instrOffset();
3844+
idx--;
3845+
}
37523846
auto deopt_id = env_->code_rt->addDeoptMetadata(std::move(meta));
37533847
bbb.appendInstr(
37543848
OutInd{deopt_idx_addr_, 0},

cinderx/Jit/lir/generator.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,19 @@ class LIRGenerator {
129129
BasicBlockBuilder& bbb,
130130
const jit::hir::Instr& i,
131131
jit::hir::Opcode opcode,
132-
int& last_deopt_line);
132+
int& last_deopt_line,
133+
const jit::hir::FrameState* caller_fs,
134+
BorrowedRef<PyCodeObject> inlined_code);
133135
#endif
134136

135137
bool TranslateSpecializedCall(
136138
BasicBlockBuilder& bbb,
137139
const hir::VectorCall& instr);
138140

139-
TranslatedBlock TranslateOneBasicBlock(const hir::BasicBlock* bb);
141+
TranslatedBlock TranslateOneBasicBlock(
142+
const hir::BasicBlock* bb,
143+
const hir::FrameState* initial_caller_fs = nullptr,
144+
BorrowedRef<PyCodeObject> initial_inlined_code = nullptr);
140145

141146
// Fill in operands for phi instructions. This is executed after LIR
142147
// instructions have been generated for all values in the control flow graph.

0 commit comments

Comments
 (0)