Skip to content

Commit df7f36f

Browse files
committed
compiler: super early exploration of instruction reordering
See comments in InstructionReordering.ts. This needs substantial iteration before landing in some form, just putting up to share for discussion. ghstack-source-id: 153eca4 Pull Request resolved: #29579
1 parent 867edc6 commit df7f36f

File tree

59 files changed

+862
-800
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

59 files changed

+862
-800
lines changed

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
deadCodeElimination,
4242
pruneMaybeThrows,
4343
} from "../Optimization";
44+
import { instructionReordering } from "../Optimization/InstructionReordering";
4445
import {
4546
CodegenFunction,
4647
alignObjectMethodScopes,
@@ -195,6 +196,9 @@ function* runWithEnvironment(
195196
deadCodeElimination(hir);
196197
yield log({ kind: "hir", name: "DeadCodeElimination", value: hir });
197198

199+
instructionReordering(hir);
200+
yield log({ kind: "hir", name: "InstructionReordering", value: hir });
201+
198202
pruneMaybeThrows(hir);
199203
yield log({ kind: "hir", name: "PruneMaybeThrows", value: hir });
200204

compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,12 @@ class InferenceState {
737737
* For debugging purposes, dumps the state to a plain
738738
* object so that it can printed as JSON.
739739
*/
740+
inspect(): any {
741+
return {
742+
values: this.#values,
743+
variables: this.#variables,
744+
};
745+
}
740746
debug(): any {
741747
const result: any = { values: {}, variables: {} };
742748
const objects: Map<InstructionValue, number> = new Map();

compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ function evaluateInstruction(
441441
}
442442
case "LoadLocal": {
443443
const placeValue = read(constants, value.place);
444-
if (placeValue !== null) {
444+
if (placeValue != null) {
445445
instr.value = placeValue;
446446
}
447447
return placeValue;
Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
import {
2+
BasicBlock,
3+
Environment,
4+
HIRFunction,
5+
IdentifierId,
6+
Instruction,
7+
markInstructionIds,
8+
} from "../HIR";
9+
import {
10+
eachInstructionValueLValue,
11+
eachInstructionValueOperand,
12+
eachTerminalOperand,
13+
} from "../HIR/visitors";
14+
import { mayAllocate } from "../ReactiveScopes/InferReactiveScopeVariables";
15+
import { getOrInsertDefault } from "../Utils/utils";
16+
17+
/**
18+
* WIP early exploration of instruction reordering. This is a fairly aggressive form and has
19+
* some issues. The idea of what's implemented:
20+
*
21+
* The high-level approach is to build a dependency graph where nodes generally correspond
22+
* either to instructions OR to particular lvalue assignments of an expresssion. So
23+
* `Destructure [x, y] = z` creates 3 nodes: one for the instruction, and one each for x and y.
24+
* The lvalue nodes depend on the instruction node that assigns them.
25+
*
26+
* We add dependency edges for all the rvalues/lvalues of each instruction. In addition, we
27+
* implicitly add dependencies btw non-reorderable instructions (more on that criteria) to
28+
* serialize any instruction where order might be observable.
29+
*
30+
* We then distinguish two types of instructions that are reorderable:
31+
* - Primitives, JSXText, JSX elements, and globals can be *globally* reordered, ie across blocks.
32+
* We defer emitting them until they are first used globally.
33+
* - Array and object expressions are reorderable within basic blocks. This could likely be relaxed to be global.
34+
* - StoreLocal, LoadLocal, and Destructure are reorderable within basic blocks. However, we serialize all
35+
* references to each named variable (reads and writes) to ensure that we aren't changing the order of evaluation
36+
* of variable references.
37+
*
38+
* The variable reordering relies on the fact that any variables that could be reassigned via a function expression
39+
* are promoted to "context" variables and use LoadContext/StoreContext, which are not reorderable.
40+
*
41+
* In theory it might even be safe to do this variable reordering globally, but i want to think through that more.
42+
*
43+
* With the above context, the algorithm is approximately:
44+
* - For each basic block:
45+
* - Iterate the instructions to create the dependency graph
46+
* - Re-emit instructions, "pulling" from all the values that are depended upon by the block's terminal.
47+
* - Emit any remaining instructions that cannot be globally reordered, starting from later instructions first.
48+
* - Save any globally-reorderable instructions into a global map that is shared across blocks, so they can be
49+
* emitted by the first block that needs them.
50+
*
51+
* Emitting instructions is currently naive: we just iterate in the order that the dependencies were established.
52+
* If instruction 4 depends on instructions 1, 2, and 3, we'll visit in depth-first order and emit 1, 2, 3, 4.
53+
* That's true even if instruction 1 and 2 are simple instructions (for ex primitives) while instruction 3 has its
54+
* own large dependency tree.
55+
*
56+
* ## Issues/things to explore:
57+
*
58+
* - An obvious improvement is to weight the nodes and emit dependencies based on weight. Alternatively, we could try to
59+
* determine the reactive dependencies of each node, and try to emit nodes that have the same dependencies together.
60+
* - Reordering destructure statements means that we also end up deferring the evaluation of its RHS. So i noticed some
61+
* `const [state, setState] = useState(...)` getting moved around. But i think i might have just messed up the bit that
62+
* ensures non-reorderable instructions (like the useState() call here) are serialized. So this should just be a simple fix,
63+
* if i didn't already fix it (need to go back through the fixture output changes)
64+
* - I also noticed that destructuring being moved meant that some reactive scopes ended up with less precise input, because
65+
* the destructure moved into the reactive scope itself (so the scope depends on the rvalue of the destructure, not the lvalues).
66+
* This is weird, i need to debug.
67+
* - Probably more things.
68+
*/
69+
export function instructionReordering(fn: HIRFunction): void {
70+
const globalDependencies: Dependencies = new Map();
71+
for (const [, block] of fn.body.blocks) {
72+
reorderBlock(fn.env, block, globalDependencies);
73+
}
74+
markInstructionIds(fn.body);
75+
}
76+
77+
type Dependencies = Map<IdentifierId, Node>;
78+
type Node = {
79+
instruction: Instruction | null;
80+
dependencies: Array<IdentifierId>;
81+
depth: number | null;
82+
};
83+
84+
function reorderBlock(
85+
env: Environment,
86+
block: BasicBlock,
87+
globalDependencies: Dependencies
88+
): void {
89+
const dependencies: Dependencies = new Map();
90+
const locals = new Map<string, IdentifierId>();
91+
let previousIdentifier: IdentifierId | null = null;
92+
for (const instr of block.instructions) {
93+
const node: Node = getOrInsertDefault(
94+
dependencies,
95+
instr.lvalue.identifier.id,
96+
{
97+
instruction: instr,
98+
dependencies: [],
99+
depth: null,
100+
}
101+
);
102+
if (getReorderingLevel(instr) === ReorderingLevel.None) {
103+
if (previousIdentifier !== null) {
104+
node.dependencies.push(previousIdentifier);
105+
}
106+
previousIdentifier = instr.lvalue.identifier.id;
107+
}
108+
for (const operand of eachInstructionValueOperand(instr.value)) {
109+
if (
110+
operand.identifier.name !== null &&
111+
operand.identifier.name.kind === "named"
112+
) {
113+
const previous = locals.get(operand.identifier.name.value);
114+
if (previous !== undefined) {
115+
node.dependencies.push(previous);
116+
}
117+
locals.set(operand.identifier.name.value, instr.lvalue.identifier.id);
118+
} else {
119+
if (
120+
dependencies.has(operand.identifier.id) ||
121+
globalDependencies.has(operand.identifier.id)
122+
) {
123+
node.dependencies.push(operand.identifier.id);
124+
}
125+
}
126+
}
127+
dependencies.set(instr.lvalue.identifier.id, node);
128+
129+
for (const lvalue of eachInstructionValueLValue(instr.value)) {
130+
const lvalueNode = getOrInsertDefault(
131+
dependencies,
132+
lvalue.identifier.id,
133+
{
134+
instruction: null,
135+
dependencies: [],
136+
depth: null,
137+
}
138+
);
139+
lvalueNode.dependencies.push(instr.lvalue.identifier.id);
140+
if (
141+
lvalue.identifier.name !== null &&
142+
lvalue.identifier.name.kind === "named"
143+
) {
144+
const previous = locals.get(lvalue.identifier.name.value);
145+
if (previous !== undefined) {
146+
node.dependencies.push(previous);
147+
}
148+
locals.set(lvalue.identifier.name.value, instr.lvalue.identifier.id);
149+
}
150+
}
151+
}
152+
153+
function getDepth(env: Environment, id: IdentifierId): number {
154+
const node = dependencies.get(id);
155+
if (node == null) {
156+
return 0;
157+
}
158+
if (node.depth !== null) {
159+
return node.depth;
160+
}
161+
node.depth = 0;
162+
let depth =
163+
node.instruction != null && mayAllocate(env, node.instruction) ? 1 : 0;
164+
for (const dep of node.dependencies) {
165+
depth += getDepth(env, dep);
166+
}
167+
node.depth = depth;
168+
return depth;
169+
}
170+
171+
const instructions: Array<Instruction> = [];
172+
173+
function emit(id: IdentifierId): void {
174+
const node = dependencies.get(id) ?? globalDependencies.get(id);
175+
if (node == null) {
176+
return;
177+
}
178+
dependencies.delete(id);
179+
globalDependencies.delete(id);
180+
node.dependencies.sort((a, b) => {
181+
const aDepth = getDepth(env, a);
182+
const bDepth = getDepth(env, b);
183+
return bDepth - aDepth;
184+
});
185+
for (const dep of node.dependencies) {
186+
emit(dep);
187+
}
188+
if (node.instruction !== null) {
189+
instructions.push(node.instruction);
190+
}
191+
}
192+
193+
for (const operand of eachTerminalOperand(block.terminal)) {
194+
emit(operand.identifier.id);
195+
}
196+
/**
197+
* Gross hack: for value blocks we want the terminal operand to be emitted last, since that's its value.
198+
* For other blocks the exact order doesn't matter, we assume instructions whose values aren't depended
199+
* upon by the block terminal are used later, so it makes sense to order them last.
200+
*/
201+
const index = instructions.length;
202+
for (const id of Array.from(dependencies.keys()).reverse()) {
203+
const node = dependencies.get(id);
204+
if (node == null) {
205+
continue;
206+
}
207+
if (
208+
node.instruction !== null &&
209+
getReorderingLevel(node.instruction) === ReorderingLevel.Global &&
210+
(block.kind === "block" || block.kind === "catch")
211+
) {
212+
globalDependencies.set(id, node);
213+
} else {
214+
emit(id);
215+
}
216+
}
217+
if (block.kind !== "block" && block.kind !== "catch") {
218+
const extra = instructions.splice(index);
219+
instructions.splice(0, 0, ...extra);
220+
}
221+
block.instructions = instructions;
222+
}
223+
224+
enum ReorderingLevel {
225+
None = "none",
226+
Local = "local",
227+
Global = "global",
228+
}
229+
function getReorderingLevel(instr: Instruction): ReorderingLevel {
230+
switch (instr.value.kind) {
231+
case "JsxExpression":
232+
case "JsxFragment":
233+
case "JSXText":
234+
case "LoadGlobal":
235+
case "Primitive":
236+
case "TemplateLiteral": {
237+
return ReorderingLevel.Global;
238+
}
239+
/*
240+
* For locals, a simple and robust strategy is to figure out the range of instructions where the identifier may be reassigned,
241+
* and then allow reordering of LoadLocal instructions which occur after this range. Obviously for const, this means that all
242+
* LoadLocals can be reordered, so a simpler thing to start with is just to only allow reordering of loads of known-consts.
243+
*
244+
* With this overall strategy we can allow global reordering of LoadLocals and remove the global/local reordering distinction
245+
* (all reordering can be global).
246+
*
247+
* case "Destructure":
248+
* case "StoreLocal":
249+
* case "LoadLocal":
250+
* {
251+
* return ReorderingLevel.Local;
252+
* }
253+
*/
254+
default: {
255+
return ReorderingLevel.None;
256+
}
257+
}
258+
}

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,10 @@ export function isMutable({ id }: Instruction, place: Place): boolean {
165165
return id >= range.start && id < range.end;
166166
}
167167

168-
function mayAllocate(env: Environment, instruction: Instruction): boolean {
168+
export function mayAllocate(
169+
env: Environment,
170+
instruction: Instruction
171+
): boolean {
169172
const { value } = instruction;
170173
switch (value.kind) {
171174
case "Destructure": {

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtOperandsInSameScope.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ function visit(fn: HIRFunction, fbtValues: Set<IdentifierId>): void {
9494
operand.identifier.mutableRange.start
9595
)
9696
);
97+
98+
fbtValues.add(operand.identifier.id);
9799
}
98100
} else if (
99101
isFbtJsxExpression(fbtValues, value) ||
@@ -126,6 +128,23 @@ function visit(fn: HIRFunction, fbtValues: Set<IdentifierId>): void {
126128
*/
127129
fbtValues.add(operand.identifier.id);
128130
}
131+
} else if (fbtValues.has(lvalue.identifier.id)) {
132+
const fbtScope = lvalue.identifier.scope;
133+
if (fbtScope === null) {
134+
return;
135+
}
136+
137+
for (const operand of eachReactiveValueOperand(value)) {
138+
operand.identifier.scope = fbtScope;
139+
140+
// Expand the jsx element's range to account for its operands
141+
fbtScope.range.start = makeInstructionId(
142+
Math.min(
143+
fbtScope.range.start,
144+
operand.identifier.mutableRange.start
145+
)
146+
);
147+
}
129148
}
130149
}
131150
}

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ import {
2929
} from "../HIR/ObjectShape";
3030
import { eachInstructionLValue } from "../HIR/visitors";
3131
import { assertExhaustive } from "../Utils/utils";
32-
import { printReactiveScopeSummary } from "./PrintReactiveFunction";
32+
import {
33+
printReactiveFunction,
34+
printReactiveScopeSummary,
35+
} from "./PrintReactiveFunction";
3336
import {
3437
ReactiveFunctionTransform,
3538
ReactiveFunctionVisitor,
@@ -84,9 +87,16 @@ import {
8487
export function mergeReactiveScopesThatInvalidateTogether(
8588
fn: ReactiveFunction
8689
): void {
90+
if (DEBUG) {
91+
console.log(printReactiveFunction(fn));
92+
}
8793
const lastUsageVisitor = new FindLastUsageVisitor();
8894
visitReactiveFunction(fn, lastUsageVisitor, undefined);
8995
visitReactiveFunction(fn, new Transform(lastUsageVisitor.lastUsage), null);
96+
97+
if (DEBUG) {
98+
console.log(printReactiveFunction(fn));
99+
}
90100
}
91101

92102
const DEBUG: boolean = false;
@@ -176,6 +186,7 @@ class Transform extends ReactiveFunctionTransform<ReactiveScopeDependencies | nu
176186
}
177187
case "instruction": {
178188
switch (instr.instruction.value.kind) {
189+
case "LoadGlobal":
179190
case "ComputedLoad":
180191
case "JSXText":
181192
case "LoadLocal":

compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ export function assertExhaustive(_: never, errorMsg: string): never {
3030
throw new Error(errorMsg);
3131
}
3232

33-
// Modifies @param array in place, retaining only the items where the predicate returns true.
33+
/**
34+
* Modifies @param array in place, retaining only the items where the predicate returns true.
35+
*/
3436
export function retainWhere<T>(
3537
array: Array<T>,
3638
predicate: (item: T) => boolean

0 commit comments

Comments
 (0)