@samitouri / QOS-React / commits / f987f1a057

[compiler] Maintain RPO and unique 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: 2a99df02ac9d125b202cae369e2dc4dccefb0625 Pull Request resolved: https://github.com/facebook/react/pull/30399

Joe Savona committed Jul 23, 2024 at 10:07 UTC f987f1a05737dae85350f0d24937f95b26494f58
2 files changed +57 -8
compiler/packages/babel-plugin-react-compiler/src/HIR/AssertTerminalBlocksExist.ts
+1
@@ -39,6 +39,7 @@ export function assertTerminalPredsExist(fn: HIRFunction): void {
39 [...eachTerminalSuccessor(predBlock.terminal)].includes(block.id),
40 {
41 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}`,
43 loc: GeneratedSource,
44 },
45 );
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts
+56 -8
@@ -9,10 +9,17 @@ import {
9 GotoVariant,
10 HIRFunction,
11 InstructionId,
12 + makeInstructionId,
13 ReactiveScope,
14 ReactiveScopeTerminal,
15 ScopeId,
16 } from './HIR';
17 +import {
18 + markInstructionIds,
19 + markPredecessors,
20 + reversePostorderBlocks,
21 +} from './HIRBuilder';
22 +import {eachInstructionLValue} from './visitors';
23
24 /**
25 * This pass assumes that all program blocks are properly nested with respect to fallthroughs
@@ -142,16 +149,9 @@ export function buildReactiveScopeTerminalsHIR(fn: HIRFunction): void {
149
150 /**
151 * Step 3:
145 - * Repoint preds and phis when they refer to a rewritten block.
152 + * Repoint phis when they refer to a rewritten block.
153 */
154 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 - }
155 for (const phi of block.phis) {
156 for (const [originalId, value] of phi.operands) {
157 const newId = rewrittenFinalBlocks.get(originalId);
@@ -162,6 +162,54 @@ export function buildReactiveScopeTerminalsHIR(fn: HIRFunction): void {
162 }
163 }
164 }
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 + }
213 }
214
215 type TerminalRewriteInfo =