Skip to content

Commit 970bb0a

Browse files
committed
[compiler] Maintain RPO and uniquely instruction ids when constructing scope terminals
Later passes may rely on HIR invariants such as blocks being in RPO or instructions having unique, ascending InstructionIds. However, BuildReactiveScopeTerminalsHIR doesn't currently gurantee this. This PR updates that pass to first restore RPO, fixup predecessors (the previous logic tried to do this but failed on unreachable blocks, where `markPredecessors()` handles that case), and renumber instructions. Then it walks instructions and scopes to update identifier and scope ranges given the new instruction ids. ghstack-source-id: 2127da7 Pull Request resolved: #30399
1 parent 6859a29 commit 970bb0a

File tree

2 files changed

+57
-8
lines changed

2 files changed

+57
-8
lines changed

compiler/packages/babel-plugin-react-compiler/src/HIR/AssertTerminalBlocksExist.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export function assertTerminalPredsExist(fn: HIRFunction): void {
3939
[...eachTerminalSuccessor(predBlock.terminal)].includes(block.id),
4040
{
4141
reason: 'Terminal successor does not reference correct predecessor',
42+
description: `Block bb${block.id} has bb${predBlock.id} as a predecessor, but bb${predBlock.id}'s successors do not include bb${block.id}`,
4243
loc: GeneratedSource,
4344
},
4445
);

compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,17 @@ import {
99
GotoVariant,
1010
HIRFunction,
1111
InstructionId,
12+
makeInstructionId,
1213
ReactiveScope,
1314
ReactiveScopeTerminal,
1415
ScopeId,
1516
} from './HIR';
17+
import {
18+
markInstructionIds,
19+
markPredecessors,
20+
reversePostorderBlocks,
21+
} from './HIRBuilder';
22+
import {eachInstructionLValue} from './visitors';
1623

1724
/**
1825
* This pass assumes that all program blocks are properly nested with respect to fallthroughs
@@ -142,16 +149,9 @@ export function buildReactiveScopeTerminalsHIR(fn: HIRFunction): void {
142149

143150
/**
144151
* Step 3:
145-
* Repoint preds and phis when they refer to a rewritten block.
152+
* Repoint phis when they refer to a rewritten block.
146153
*/
147154
for (const [, block] of originalBlocks) {
148-
for (const pred of block.preds) {
149-
const newId = rewrittenFinalBlocks.get(pred);
150-
if (newId != null) {
151-
block.preds.delete(pred);
152-
block.preds.add(newId);
153-
}
154-
}
155155
for (const phi of block.phis) {
156156
for (const [originalId, value] of phi.operands) {
157157
const newId = rewrittenFinalBlocks.get(originalId);
@@ -162,6 +162,54 @@ export function buildReactiveScopeTerminalsHIR(fn: HIRFunction): void {
162162
}
163163
}
164164
}
165+
166+
/**
167+
* Step 4:
168+
* Fixup the HIR to restore RPO, ensure correct predecessors, and
169+
* renumber instructions. Note that the renumbering instructions
170+
* invalidates scope and identifier ranges, so we fix them in the
171+
* next step.
172+
*/
173+
reversePostorderBlocks(fn.body);
174+
markPredecessors(fn.body);
175+
markInstructionIds(fn.body);
176+
177+
/**
178+
* Step 5:
179+
* Fix scope and identifier ranges to account for renumbered instructions
180+
*/
181+
for (const [, block] of fn.body.blocks) {
182+
for (const instruction of block.instructions) {
183+
for (const lvalue of eachInstructionLValue(instruction)) {
184+
/*
185+
* Any lvalues whose mutable range was a single instruction must have
186+
* started at the current instruction, so update the range to match
187+
* the instruction's new id
188+
*/
189+
if (
190+
lvalue.identifier.mutableRange.end ===
191+
lvalue.identifier.mutableRange.start + 1
192+
) {
193+
lvalue.identifier.mutableRange.start = instruction.id;
194+
lvalue.identifier.mutableRange.end = makeInstructionId(
195+
instruction.id + 1,
196+
);
197+
}
198+
}
199+
}
200+
const terminal = block.terminal;
201+
if (terminal.kind === 'scope' || terminal.kind === 'pruned-scope') {
202+
/*
203+
* Scope ranges should always align to start at the 'scope' terminal
204+
* and end at the first instruction of the fallthrough block
205+
*/
206+
const fallthroughBlock = fn.body.blocks.get(terminal.fallthrough)!;
207+
const firstId =
208+
fallthroughBlock.instructions[0]?.id ?? fallthroughBlock.terminal.id;
209+
terminal.scope.range.start = terminal.id;
210+
terminal.scope.range.end = firstId;
211+
}
212+
}
165213
}
166214

167215
type TerminalRewriteInfo =

0 commit comments

Comments
 (0)