Skip to content

Commit 90c924b

Browse files
alexmalyshevmeta-codesync[bot]
authored andcommitted
Sink non-escaping PrimitiveBox onto deopt paths
Summary: Chained primitive float arithmetic already runs unboxed on the fast path (`simplifyFloatBinaryOp` emits `PrimitiveUnbox`/`DoubleBinaryOp`/`PrimitiveBox`, and `simplifyUnbox` fuses `unbox(box(x))`). But the intermediate `PrimitiveBox` instructions often survive: the boxed value has no data uses, yet it is held in deopt frame state (`Locals`/`Stack`) so the interpreter can recover it if a guard deopts. The box is then pure fast-path overhead, an allocation that exists only for a path that usually does not run. This adds a `SinkPrimitiveBox` HIR pass. For a `PrimitiveBox` whose result never escapes (no data-operand use, only deopt frame-state references and `UseType` assertions) it rewrites the frame-state references to the unboxed source value and deletes the box (and the now-meaningless `UseType`s). The deopt machinery already re-boxes unboxed live values from their `LiveValue::value_kind` (`kDouble` -> `PyFloat_FromDouble`), so deopt stays correct while the fast path stays unboxed. `UseType` operands are excluded when deciding whether a value escapes, since `UseType` is a no-op type assertion, not a real consumer. Scoped to `CDouble` (floats) for now; the pass generalizes to other primitive kinds (e.g. `CInt64`/`CUInt64`) with a single type-check change, which is the higher-value follow-up for integer-arithmetic-heavy code where boxing is expensive. Reviewed By: mpage Differential Revision: D110384610 fbshipit-source-id: 502d54a7199abbda046af2f663f9caa6ee626c55
1 parent 8332aaa commit 90c924b

6 files changed

Lines changed: 127 additions & 0 deletions

File tree

cinderx/Jit/compiler.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "cinderx/Jit/hir/printer.h"
2121
#include "cinderx/Jit/hir/refcount_insertion.h"
2222
#include "cinderx/Jit/hir/simplify.h"
23+
#include "cinderx/Jit/hir/sink_primitive_box.h"
2324
#include "cinderx/Jit/hir/ssa.h"
2425
#include "cinderx/Jit/hir/stats.h"
2526
#include "cinderx/Jit/jit_time_log.h"
@@ -107,6 +108,7 @@ void Compiler::runPasses(
107108
hir::BuiltinLoadMethodElimination{}, PassConfig::kBuiltinLoadMethodElim);
108109
runPassIf(hir::Simplify{}, PassConfig::kSimplify);
109110
runPassIf(hir::CleanCFG{}, PassConfig::kCleanCFG);
111+
runPassIf(hir::SinkPrimitiveBox{}, PassConfig::kSinkPrimitiveBox);
110112
runPassIf(hir::DeadCodeElimination{}, PassConfig::kDeadCodeElim);
111113
runPassIf(hir::CleanCFG{}, PassConfig::kCleanCFG);
112114

@@ -164,6 +166,7 @@ PassConfig createConfig() {
164166
set(hir_opts.insert_update_prev_instr, PassConfig::kInsertUpdatePrevInstr);
165167
set(hir_opts.phi_elim, PassConfig::kPhiElim);
166168
set(hir_opts.simplify, PassConfig::kSimplify);
169+
set(hir_opts.sink_primitive_box, PassConfig::kSinkPrimitiveBox);
167170

168171
return static_cast<PassConfig>(result);
169172
}

cinderx/Jit/compiler.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ enum PassConfig : uint64_t {
3838
kPhiElim = 1 << 7,
3939
kSimplify = 1 << 8,
4040
kInsertUpdatePrevInstr = 1 << 9,
41+
kSinkPrimitiveBox = 1 << 10,
4142

4243
// Run all the passes.
4344
kAll = ~uint64_t{0},

cinderx/Jit/config.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ struct HIROptimizations {
4343
bool insert_update_prev_instr{true};
4444
bool phi_elim{true};
4545
bool simplify{true};
46+
bool sink_primitive_box{true};
4647
};
4748

4849
// List of LIR optimization passes to run.
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Copyright (c) Meta Platforms, Inc. and affiliates.
2+
3+
#include "cinderx/Jit/hir/sink_primitive_box.h"
4+
5+
#include "cinderx/Jit/hir/hir.h"
6+
7+
#include <unordered_map>
8+
#include <unordered_set>
9+
#include <vector>
10+
11+
namespace cinderx::jit::hir {
12+
13+
void SinkPrimitiveBox::run(Function& func) {
14+
// Registers that escape as a real PyObject, i.e. appear as a data operand of
15+
// some consuming instruction. UseType is excluded: it is a no-op type
16+
// assertion (it keeps a GuardType alive), not a real consumer, so a box used
17+
// only by UseType and deopt frame state does not actually escape.
18+
std::unordered_set<Register*> escapes;
19+
for (auto& block : func.cfg.blocks) {
20+
for (Instr& instr : block) {
21+
if (instr.isUseType()) {
22+
continue;
23+
}
24+
for (std::size_t i = 0, n = instr.numOperands(); i < n; ++i) {
25+
escapes.insert(instr.getOperand(i));
26+
}
27+
}
28+
}
29+
30+
// Map each sinkable box's result to its unboxed source value, remember the
31+
// box instructions, and collect the UseType assertions on those boxes (which
32+
// become meaningless once the box is gone).
33+
std::unordered_map<Register*, Register*> sink_map;
34+
std::vector<Instr*> dead_instrs;
35+
for (auto& block : func.cfg.blocks) {
36+
for (Instr& instr : block) {
37+
if (!instr.isPrimitiveBox()) {
38+
continue;
39+
}
40+
auto& box = static_cast<PrimitiveBox&>(instr);
41+
// Limited to floats for now: deopt re-boxes a CDouble via PyFloat.
42+
if (!(box.type() <= TCDouble)) {
43+
continue;
44+
}
45+
if (!escapes.contains(box.output())) {
46+
sink_map.emplace(box.output(), box.value());
47+
dead_instrs.push_back(&instr);
48+
}
49+
}
50+
}
51+
52+
if (sink_map.empty()) {
53+
return;
54+
}
55+
56+
// Drop UseType assertions on sunk boxes before rewriting, so we don't rewrite
57+
// them to a primitive-typed register (which would be type-inconsistent).
58+
for (auto& block : func.cfg.blocks) {
59+
for (Instr& instr : block) {
60+
if (instr.isUseType() && sink_map.contains(instr.getOperand(0))) {
61+
dead_instrs.push_back(&instr);
62+
}
63+
}
64+
}
65+
66+
// Rewrite the boxes' remaining (frame-state only) uses to the unboxed value.
67+
// The deopt machinery records the unboxed value's kind and re-boxes it if a
68+
// deopt fires, so the box is no longer needed on the fast path.
69+
for (auto& block : func.cfg.blocks) {
70+
for (Instr& instr : block) {
71+
instr.visitUses([&](Register*& reg) {
72+
auto it = sink_map.find(reg);
73+
if (it != sink_map.end()) {
74+
reg = it->second;
75+
}
76+
return true;
77+
});
78+
}
79+
}
80+
81+
// The boxes (and their UseType assertions) now have no uses.
82+
for (Instr* instr : dead_instrs) {
83+
instr->unlink();
84+
delete instr;
85+
}
86+
}
87+
88+
} // namespace cinderx::jit::hir
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright (c) Meta Platforms, Inc. and affiliates.
2+
3+
#pragma once
4+
5+
#include "cinderx/Jit/hir/pass.h"
6+
7+
namespace cinderx::jit::hir {
8+
9+
// Sink PrimitiveBox instructions onto deopt paths.
10+
//
11+
// When a boxed primitive's result has no data-operand uses (it is referenced
12+
// only by deopt frame state) the box exists solely so the value can be
13+
// materialized for the interpreter if a deopt occurs. The deopt machinery can
14+
// re-box an unboxed primitive from its LiveValue (via value_kind), so we
15+
// rewrite those frame-state references to the unboxed source value, leaving the
16+
// box dead. This keeps chained primitive arithmetic unboxed on the fast path
17+
// while staying correct on deopt. Currently limited to CDouble (floats).
18+
class SinkPrimitiveBox final : public Pass {
19+
public:
20+
SinkPrimitiveBox() : Pass("SinkPrimitiveBox") {}
21+
22+
void run(Function& irfunc) override;
23+
24+
static std::unique_ptr<SinkPrimitiveBox> factory() {
25+
return std::make_unique<SinkPrimitiveBox>();
26+
}
27+
};
28+
29+
} // namespace cinderx::jit::hir

cinderx/Jit/pyjit.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,11 @@ FlagProcessor initFlagProcessor() {
564564
"CINDERX_JIT_PHI_ELIM");
565565
HIR_OPTIMIZATION_OPTION(
566566
"simplify", simplify, "cinderx-jit-simplify", "CINDERX_JIT_SIMPLIFY");
567+
HIR_OPTIMIZATION_OPTION(
568+
"sink primitive box",
569+
sink_primitive_box,
570+
"cinderx-jit-sink-primitive-box",
571+
"CINDERX_JIT_SINK_PRIMITIVE_BOX");
567572

568573
flag_processor.addOption(
569574
"cinderx-jit-simplify-iteration-limit",

0 commit comments

Comments
 (0)