@samitouri / QOS-React / commits / 925c20a206

[compiler] Add fallthrough to branch terminal

Branch terminals didn't have a fallthrough because they correspond to an outer terminal (optional, logical, etc) that has the "real" fallthrough. But understanding how branch terminals correspond to these outer terminals requires knowing the branch fallthrough. For example, `foo?.bar?.baz` creates terminals along the lines of: ``` bb0: optional fallthrough=bb4 bb1: optional fallthrough=bb3 bb2: ... branch ... (fallthrough=bb3) ... bb3: ... branch ... (fallthrough=bb4) ... bb4: ... ``` Without a fallthrough on `branch` terminals, it's unclear that the optional from bb0 has its branch node in bb3. With the fallthroughs, we can see look for a branch with the same fallthrough as the outer optional terminal to match them up. ghstack-source-id: d48c6232899864716eef71798a278b487d30eafc Pull Request resolved: https://github.com/facebook/react/pull/30814

Joe Savona committed Aug 28, 2024 at 10:52 UTC 925c20a20674254391b7752aa216ec417c8f52a3
6 files changed +77 -11
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
+10 -4
@@ -607,6 +607,7 @@ function lowerStatement(
607 ),
608 consequent: bodyBlock,
609 alternate: continuationBlock.id,
610 + fallthrough: continuationBlock.id,
611 id: makeInstructionId(0),
612 loc: stmt.node.loc ?? GeneratedSource,
613 },
@@ -656,16 +657,13 @@ function lowerStatement(
657 },
658 conditionalBlock,
659 );
659 - /*
660 - * The conditional block is empty and exists solely as conditional for
661 - * (re)entering or exiting the loop
662 - */
660 const test = lowerExpressionToTemporary(builder, stmt.get('test'));
661 const terminal: BranchTerminal = {
662 kind: 'branch',
663 test,
664 consequent: loopBlock,
665 alternate: continuationBlock.id,
666 + fallthrough: conditionalBlock.id,
667 id: makeInstructionId(0),
668 loc: stmt.node.loc ?? GeneratedSource,
669 };
@@ -975,6 +973,7 @@ function lowerStatement(
973 test,
974 consequent: loopBlock,
975 alternate: continuationBlock.id,
976 + fallthrough: conditionalBlock.id,
977 id: makeInstructionId(0),
978 loc,
979 };
@@ -1118,6 +1117,7 @@ function lowerStatement(
1117 consequent: loopBlock,
1118 alternate: continuationBlock.id,
1119 loc: stmt.node.loc ?? GeneratedSource,
1120 + fallthrough: continuationBlock.id,
1121 },
1122 continuationBlock,
1123 );
@@ -1203,6 +1203,7 @@ function lowerStatement(
1203 test,
1204 consequent: loopBlock,
1205 alternate: continuationBlock.id,
1206 + fallthrough: continuationBlock.id,
1207 loc: stmt.node.loc ?? GeneratedSource,
1208 },
1209 continuationBlock,
@@ -1800,6 +1801,7 @@ function lowerExpression(
1801 test: {...testPlace},
1802 consequent: consequentBlock,
1803 alternate: alternateBlock,
1804 + fallthrough: continuationBlock.id,
1805 id: makeInstructionId(0),
1806 loc: exprLoc,
1807 },
@@ -1878,6 +1880,7 @@ function lowerExpression(
1880 test: {...leftPlace},
1881 consequent,
1882 alternate,
1883 + fallthrough: continuationBlock.id,
1884 id: makeInstructionId(0),
1885 loc: exprLoc,
1886 },
@@ -2611,6 +2614,7 @@ function lowerOptionalMemberExpression(
2614 test: {...object},
2615 consequent: consequent.id,
2616 alternate,
2617 + fallthrough: continuationBlock.id,
2618 id: makeInstructionId(0),
2619 loc,
2620 };
@@ -2750,6 +2754,7 @@ function lowerOptionalCallExpression(
2754 test: {...testPlace},
2755 consequent: consequent.id,
2756 alternate,
2757 + fallthrough: continuationBlock.id,
2758 id: makeInstructionId(0),
2759 loc,
2760 };
@@ -4025,6 +4030,7 @@ function lowerAssignment(
4030 test: {...test},
4031 consequent,
4032 alternate,
4033 + fallthrough: continuationBlock.id,
4034 id: makeInstructionId(0),
4035 loc,
4036 },
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+1 -1
@@ -491,7 +491,7 @@ export type BranchTerminal = {
491 alternate: BlockId;
492 id: InstructionId;
493 loc: SourceLocation;
494 - fallthrough?: never;
494 + fallthrough: BlockId;
495 };
496
497 export type SwitchTerminal = {
compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts
+3 -1
@@ -660,11 +660,13 @@ export function mapTerminalSuccessors(
660 case 'branch': {
661 const consequent = fn(terminal.consequent);
662 const alternate = fn(terminal.alternate);
663 + const fallthrough = fn(terminal.fallthrough);
664 return {
665 kind: 'branch',
666 test: terminal.test,
667 consequent,
668 alternate,
669 + fallthrough,
670 id: makeInstructionId(0),
671 loc: terminal.loc,
672 };
@@ -883,7 +885,6 @@ export function terminalHasFallthrough<
885 >(terminal: T): terminal is U {
886 switch (terminal.kind) {
887 case 'maybe-throw':
886 - case 'branch':
888 case 'goto':
889 case 'return':
890 case 'throw':
@@ -892,6 +893,7 @@ export function terminalHasFallthrough<
893 const _: undefined = terminal.fallthrough;
894 return false;
895 }
896 + case 'branch':
897 case 'try':
898 case 'do-while':
899 case 'for-of':
compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts
+53 -2
@@ -42,6 +42,7 @@ type IdentifierSidemap = {
42 react: Set<IdentifierId>;
43 maybeDepsLists: Map<IdentifierId, Array<Place>>;
44 maybeDeps: Map<IdentifierId, ManualMemoDependency>;
45 + optionals: Set<IdentifierId>;
46 };
47
48 /**
@@ -52,6 +53,7 @@ type IdentifierSidemap = {
53 export function collectMaybeMemoDependencies(
54 value: InstructionValue,
55 maybeDeps: Map<IdentifierId, ManualMemoDependency>,
56 + optional: boolean,
57 ): ManualMemoDependency | null {
58 switch (value.kind) {
59 case 'LoadGlobal': {
@@ -69,7 +71,7 @@ export function collectMaybeMemoDependencies(
71 return {
72 root: object.root,
73 // TODO: determine if the access is optional
72 - path: [...object.path, {property: value.property, optional: false}],
74 + path: [...object.path, {property: value.property, optional}],
75 };
76 }
77 break;
@@ -162,7 +164,11 @@ function collectTemporaries(
164 break;
165 }
166 }
165 - const maybeDep = collectMaybeMemoDependencies(value, sidemap.maybeDeps);
167 + const maybeDep = collectMaybeMemoDependencies(
168 + value,
169 + sidemap.maybeDeps,
170 + sidemap.optionals.has(lvalue.identifier.id),
171 + );
172 // We don't expect named lvalues during this pass (unlike ValidatePreservingManualMemo)
173 if (maybeDep != null) {
174 sidemap.maybeDeps.set(lvalue.identifier.id, maybeDep);
@@ -338,12 +344,14 @@ export function dropManualMemoization(func: HIRFunction): void {
344 func.env.config.validatePreserveExistingMemoizationGuarantees ||
345 func.env.config.validateNoSetStateInRender ||
346 func.env.config.enablePreserveExistingMemoizationGuarantees;
347 + const optionals = findOptionalPlaces(func);
348 const sidemap: IdentifierSidemap = {
349 functions: new Map(),
350 manualMemos: new Map(),
351 react: new Set(),
352 maybeDeps: new Map(),
353 maybeDepsLists: new Map(),
354 + optionals,
355 };
356 let nextManualMemoId = 0;
357
@@ -476,3 +484,46 @@ export function dropManualMemoization(func: HIRFunction): void {
484 }
485 }
486 }
487 +
488 +function findOptionalPlaces(fn: HIRFunction): Set<IdentifierId> {
489 + const optionals = new Set<IdentifierId>();
490 + for (const [, block] of fn.body.blocks) {
491 + if (block.terminal.kind === 'optional') {
492 + const optionalTerminal = block.terminal;
493 + let testBlock = fn.body.blocks.get(block.terminal.test)!;
494 + loop: while (true) {
495 + const terminal = testBlock.terminal;
496 + switch (terminal.kind) {
497 + case 'branch': {
498 + if (terminal.fallthrough === optionalTerminal.fallthrough) {
499 + // found it
500 + const consequent = fn.body.blocks.get(terminal.consequent)!;
501 + const last = consequent.instructions.at(-1);
502 + if (last !== undefined && last.value.kind === 'StoreLocal') {
503 + optionals.add(last.value.value.identifier.id);
504 + }
505 + break loop;
506 + } else {
507 + testBlock = fn.body.blocks.get(terminal.fallthrough)!;
508 + }
509 + break;
510 + }
511 + case 'optional':
512 + case 'logical':
513 + case 'sequence':
514 + case 'ternary': {
515 + testBlock = fn.body.blocks.get(terminal.fallthrough)!;
516 + break;
517 + }
518 + default: {
519 + CompilerError.invariant(false, {
520 + reason: `Unexpected terminal in optional`,
521 + loc: terminal.loc,
522 + });
523 + }
524 + }
525 + }
526 + }
527 + }
528 + return optionals;
529 +}
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignReactiveScopesToBlockScopesHIR.ts
+1 -1
@@ -140,7 +140,7 @@ export function alignReactiveScopesToBlockScopesHIR(fn: HIRFunction): void {
140 }
141
142 const fallthrough = terminalFallthrough(terminal);
143 - if (fallthrough !== null) {
143 + if (fallthrough !== null && terminal.kind !== 'branch') {
144 /*
145 * Any currently active scopes that overlaps the block-fallthrough range
146 * need their range extended to at least the first instruction of the
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts
+9 -2
@@ -167,7 +167,10 @@ function compareDeps(
167
168 let isSubpath = true;
169 for (let i = 0; i < Math.min(inferred.path.length, source.path.length); i++) {
170 - if (inferred.path[i].property !== source.path[i].property) {
170 + if (
171 + inferred.path[i].property !== source.path[i].property ||
172 + inferred.path[i].optional !== source.path[i].optional
173 + ) {
174 isSubpath = false;
175 break;
176 }
@@ -339,7 +342,11 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
342 return null;
343 }
344 default: {
342 - const dep = collectMaybeMemoDependencies(value, this.temporaries);
345 + const dep = collectMaybeMemoDependencies(
346 + value,
347 + this.temporaries,
348 + false,
349 + );
350 if (value.kind === 'StoreLocal' || value.kind === 'StoreContext') {
351 const storeTarget = value.lvalue.place;
352 state.manualMemoState?.decls.add(