@samitouri / QOS-React / commits / 7b7fac073d

[compiler] Represent phis with places rather than identifiers

Summary: The fact that phis are identifiers rather than places is unfortunate in a few cases. In some later analyses, we might wish to know whether a phi is reactive, but we don't have an easy way to do that currently. Most of the changes here is just replacing phi.id with phi.place.identifier and such. Interesting bits are EnterSSA (several functions now take places rather than identifiers, and InferReactivePlaces now needs to mark places as reactive explicitly. ghstack-source-id: 5f4fb396cd86b421008c37832a5735ac40f8806e Pull Request resolved: https://github.com/facebook/react/pull/31171

Mike Vitousek committed Oct 10, 2024 at 12:41 UTC 7b7fac073d1473df839a1caf8d0444c32bf4de49
20 files changed +103 -99
compiler/packages/babel-plugin-react-compiler/src/HIR/AssertConsistentIdentifiers.ts
+2 -2
@@ -29,9 +29,9 @@ export function assertConsistentIdentifiers(fn: HIRFunction): void {
29 const assignments: Set<IdentifierId> = new Set();
30 for (const [, block] of fn.body.blocks) {
31 for (const phi of block.phis) {
32 - validate(identifiers, phi.id);
32 + validate(identifiers, phi.place.identifier);
33 for (const [, operand] of phi.operands) {
34 - validate(identifiers, operand);
34 + validate(identifiers, operand.identifier);
35 }
36 }
37 for (const instr of block.instructions) {
compiler/packages/babel-plugin-react-compiler/src/HIR/AssertValidMutableRanges.ts
+2 -2
@@ -20,9 +20,9 @@ import {
20 export function assertValidMutableRanges(fn: HIRFunction): void {
21 for (const [, block] of fn.body.blocks) {
22 for (const phi of block.phis) {
23 - visitIdentifier(phi.id);
23 + visitIdentifier(phi.place.identifier);
24 for (const [, operand] of phi.operands) {
25 - visitIdentifier(operand);
25 + visitIdentifier(operand.identifier);
26 }
27 }
28 for (const instr of block.instructions) {
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+2 -2
@@ -761,8 +761,8 @@ function _staticInvariantInstructionValueHasLocation(
761
762 export type Phi = {
763 kind: 'Phi';
764 - id: Identifier;
765 - operands: Map<BlockId, Identifier>;
764 + place: Place;
765 + operands: Map<BlockId, Place>;
766 };
767
768 /**
compiler/packages/babel-plugin-react-compiler/src/HIR/MergeConsecutiveBlocks.ts
+2 -8
@@ -84,20 +84,14 @@ export function mergeConsecutiveBlocks(fn: HIRFunction): void {
84 id: predecessor.terminal.id,
85 lvalue: {
86 kind: 'Identifier',
87 - identifier: phi.id,
87 + identifier: phi.place.identifier,
88 effect: Effect.ConditionallyMutate,
89 reactive: false,
90 loc: GeneratedSource,
91 },
92 value: {
93 kind: 'LoadLocal',
94 - place: {
95 - kind: 'Identifier',
96 - identifier: operand,
97 - effect: Effect.Read,
98 - reactive: false,
99 - loc: GeneratedSource,
100 - },
94 + place: {...operand},
95 loc: GeneratedSource,
96 },
97 loc: GeneratedSource,
compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts
+5 -5
@@ -163,13 +163,13 @@ export function printInstruction(instr: ReactiveInstruction): string {
163
164 export function printPhi(phi: Phi): string {
165 const items = [];
166 - items.push(printIdentifier(phi.id));
167 - items.push(printMutableRange(phi.id));
168 - items.push(printType(phi.id.type));
166 + items.push(printPlace(phi.place));
167 + items.push(printMutableRange(phi.place.identifier));
168 + items.push(printType(phi.place.identifier.type));
169 items.push(': phi(');
170 const phis = [];
171 - for (const [blockId, id] of phi.operands) {
172 - phis.push(`bb${blockId}: ${printIdentifier(id)}`);
171 + for (const [blockId, place] of phi.operands) {
172 + phis.push(`bb${blockId}: ${printPlace(place)}`);
173 }
174
175 items.push(phis.join(', '));
compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
+1 -1
@@ -607,7 +607,7 @@ function collectDependencies(
607 // Record referenced optional chains in phis
608 for (const phi of block.phis) {
609 for (const operand of phi.operands) {
610 - const maybeOptionalChain = temporaries.get(operand[1].id);
610 + const maybeOptionalChain = temporaries.get(operand[1].identifier.id);
611 if (maybeOptionalChain) {
612 context.visitDependency(maybeOptionalChain);
613 }
compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasForPhis.ts
+2 -2
@@ -15,11 +15,11 @@ export function inferAliasForPhis(
15 for (const [_, block] of func.body.blocks) {
16 for (const phi of block.phis) {
17 const isPhiMutatedAfterCreation: boolean =
18 - phi.id.mutableRange.end >
18 + phi.place.identifier.mutableRange.end >
19 (block.instructions.at(0)?.id ?? block.terminal.id);
20 if (isPhiMutatedAfterCreation) {
21 for (const [, operand] of phi.operands) {
22 - aliases.union([phi.id, operand]);
22 + aliases.union([phi.place.identifier, operand.identifier]);
23 }
24 }
25 }
compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableLifetimes.ts
+10 -6
@@ -116,19 +116,23 @@ export function inferMutableLifetimes(
116 for (const [_, block] of func.body.blocks) {
117 for (const phi of block.phis) {
118 const isPhiMutatedAfterCreation: boolean =
119 - phi.id.mutableRange.end >
119 + phi.place.identifier.mutableRange.end >
120 (block.instructions.at(0)?.id ?? block.terminal.id);
121 if (
122 inferMutableRangeForStores &&
123 isPhiMutatedAfterCreation &&
124 - phi.id.mutableRange.start === 0
124 + phi.place.identifier.mutableRange.start === 0
125 ) {
126 for (const [, operand] of phi.operands) {
127 - if (phi.id.mutableRange.start === 0) {
128 - phi.id.mutableRange.start = operand.mutableRange.start;
127 + if (phi.place.identifier.mutableRange.start === 0) {
128 + phi.place.identifier.mutableRange.start =
129 + operand.identifier.mutableRange.start;
130 } else {
130 - phi.id.mutableRange.start = makeInstructionId(
131 - Math.min(phi.id.mutableRange.start, operand.mutableRange.start),
131 + phi.place.identifier.mutableRange.start = makeInstructionId(
132 + Math.min(
133 + phi.place.identifier.mutableRange.start,
134 + operand.identifier.mutableRange.start,
135 + ),
136 );
137 }
138 }
compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts
+4 -4
@@ -162,23 +162,23 @@ export function inferReactivePlaces(fn: HIRFunction): void {
162 let hasReactiveControl = isReactiveControlledBlock(block.id);
163
164 for (const phi of block.phis) {
165 - if (reactiveIdentifiers.isReactiveIdentifier(phi.id)) {
165 + if (reactiveIdentifiers.isReactive(phi.place)) {
166 // Already marked reactive on a previous pass
167 continue;
168 }
169 let isPhiReactive = false;
170 for (const [, operand] of phi.operands) {
171 - if (reactiveIdentifiers.isReactiveIdentifier(operand)) {
171 + if (reactiveIdentifiers.isReactive(operand)) {
172 isPhiReactive = true;
173 break;
174 }
175 }
176 if (isPhiReactive) {
177 - reactiveIdentifiers.markReactiveIdentifier(phi.id);
177 + reactiveIdentifiers.markReactive(phi.place);
178 } else {
179 for (const [pred] of phi.operands) {
180 if (isReactiveControlledBlock(pred)) {
181 - reactiveIdentifiers.markReactiveIdentifier(phi.id);
181 + reactiveIdentifiers.markReactive(phi.place);
182 break;
183 }
184 }
compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts
+2 -2
@@ -635,7 +635,7 @@ class InferenceState {
635 inferPhi(phi: Phi): void {
636 const values: Set<InstructionValue> = new Set();
637 for (const [_, operand] of phi.operands) {
638 - const operandValues = this.#variables.get(operand.id);
638 + const operandValues = this.#variables.get(operand.identifier.id);
639 // This is a backedge that will be handled later by State.merge
640 if (operandValues === undefined) continue;
641 for (const v of operandValues) {
@@ -644,7 +644,7 @@ class InferenceState {
644 }
645
646 if (values.size > 0) {
647 - this.#variables.set(phi.id.id, values);
647 + this.#variables.set(phi.place.identifier.id, values);
648 }
649 }
650 }
compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts
+2 -2
@@ -117,7 +117,7 @@ function applyConstantPropagation(
117 for (const phi of block.phis) {
118 let value = evaluatePhi(phi, constants);
119 if (value !== null) {
120 - constants.set(phi.id.id, value);
120 + constants.set(phi.place.identifier.id, value);
121 }
122 }
123
@@ -167,7 +167,7 @@ function applyConstantPropagation(
167 function evaluatePhi(phi: Phi, constants: Constants): Constant | null {
168 let value: Constant | null = null;
169 for (const [, operand] of phi.operands) {
170 - const operandValue = constants.get(operand.id) ?? null;
170 + const operandValue = constants.get(operand.identifier.id) ?? null;
171 // did not find a constant, can't constant propogate
172 if (operandValue === null) {
173 return null;
compiler/packages/babel-plugin-react-compiler/src/Optimization/DeadCodeElimination.ts
+3 -3
@@ -42,7 +42,7 @@ export function deadCodeElimination(fn: HIRFunction): void {
42 */
43 for (const [, block] of fn.body.blocks) {
44 for (const phi of block.phis) {
45 - if (!state.isIdOrNameUsed(phi.id)) {
45 + if (!state.isIdOrNameUsed(phi.place.identifier)) {
46 block.phis.delete(phi);
47 }
48 }
@@ -159,9 +159,9 @@ function findReferencedIdentifiers(fn: HIRFunction): State {
159 }
160 }
161 for (const phi of block.phis) {
162 - if (state.isIdOrNameUsed(phi.id)) {
162 + if (state.isIdOrNameUsed(phi.place.identifier)) {
163 for (const [_pred, operand] of phi.operands) {
164 - state.reference(operand);
164 + state.reference(operand.identifier);
165 }
166 }
167 }
compiler/packages/babel-plugin-react-compiler/src/Optimization/PruneMaybeThrows.ts
+2 -2
@@ -23,7 +23,7 @@ import {
23 removeUnnecessaryTryCatch,
24 removeUnreachableForUpdates,
25 } from '../HIR/HIRBuilder';
26 -import {printIdentifier} from '../HIR/PrintHIR';
26 +import {printPlace} from '../HIR/PrintHIR';
27
28 /*
29 * This pass prunes `maybe-throw` terminals for blocks that can provably *never* throw.
@@ -55,7 +55,7 @@ export function pruneMaybeThrows(fn: HIRFunction): void {
55 loc: GeneratedSource,
56 description: `Could not find mapping for predecessor bb${predecessor} in block bb${
57 block.id
58 - } for phi ${printIdentifier(phi.id)}`,
58 + } for phi ${printPlace(phi.place)}`,
59 suggestions: null,
60 });
61 phi.operands.delete(predecessor);
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts
+9 -6
@@ -281,22 +281,25 @@ export function findDisjointMutableValues(
281 */
282 for (const phi of block.phis) {
283 if (
284 - phi.id.mutableRange.start + 1 !== phi.id.mutableRange.end &&
285 - phi.id.mutableRange.end >
284 + phi.place.identifier.mutableRange.start + 1 !==
285 + phi.place.identifier.mutableRange.end &&
286 + phi.place.identifier.mutableRange.end >
287 (block.instructions.at(0)?.id ?? block.terminal.id)
288 ) {
288 - const operands = [phi.id];
289 - const declaration = declarations.get(phi.id.declarationId);
289 + const operands = [phi.place.identifier];
290 + const declaration = declarations.get(
291 + phi.place.identifier.declarationId,
292 + );
293 if (declaration !== undefined) {
294 operands.push(declaration);
295 }
296 for (const [_, phiId] of phi.operands) {
294 - operands.push(phiId);
297 + operands.push(phiId.identifier);
298 }
299 scopeIdentifiers.union(operands);
300 } else if (fn.env.config.enableForest) {
301 for (const [, phiId] of phi.operands) {
299 - scopeIdentifiers.union([phi.id, phiId]);
302 + scopeIdentifiers.union([phi.place.identifier, phiId.identifier]);
303 }
304 }
305 }
compiler/packages/babel-plugin-react-compiler/src/SSA/EliminateRedundantPhi.ts
+5 -10
@@ -68,18 +68,13 @@ export function eliminateRedundantPhi(
68 // Find any redundant phis
69 phis: for (const phi of block.phis) {
70 // Remap phis in case operands are from eliminated phis
71 - phi.operands = new Map(
72 - Array.from(phi.operands).map(([block, id]) => [
73 - block,
74 - rewrites.get(id) ?? id,
75 - ]),
76 - );
71 + phi.operands.forEach((place, _) => rewritePlace(place, rewrites));
72 // Find if the phi can be eliminated
73 let same: Identifier | null = null;
74 for (const [_, operand] of phi.operands) {
75 if (
81 - (same !== null && operand.id === same.id) ||
82 - operand.id === phi.id.id
76 + (same !== null && operand.identifier.id === same.id) ||
77 + operand.identifier.id === phi.place.identifier.id
78 ) {
79 /*
80 * This operand is the same as the phi or is the same as the
@@ -94,7 +89,7 @@ export function eliminateRedundantPhi(
89 continue phis;
90 } else {
91 // First non-phi operand
97 - same = operand;
92 + same = operand.identifier;
93 }
94 }
95 CompilerError.invariant(same !== null, {
@@ -103,7 +98,7 @@ export function eliminateRedundantPhi(
98 loc: null,
99 suggestions: null,
100 });
106 - rewrites.set(phi.id, same);
101 + rewrites.set(phi.place.identifier, same);
102 block.phis.delete(phi);
103 }
104
compiler/packages/babel-plugin-react-compiler/src/SSA/EnterSSA.ts
+32 -29
@@ -18,7 +18,7 @@ import {
18 Phi,
19 Place,
20 } from '../HIR/HIR';
21 -import {printIdentifier} from '../HIR/PrintHIR';
21 +import {printIdentifier, printPlace} from '../HIR/PrintHIR';
22 import {
23 eachTerminalSuccessor,
24 mapInstructionLValues,
@@ -27,8 +27,8 @@ import {
27 } from '../HIR/visitors';
28
29 type IncompletePhi = {
30 - oldId: Identifier;
31 - newId: Identifier;
30 + oldPlace: Place;
31 + newPlace: Place;
32 };
33
34 type State = {
@@ -122,33 +122,33 @@ class SSABuilder {
122 }
123
124 getPlace(oldPlace: Place): Place {
125 - const newId = this.getIdAt(oldPlace.identifier, this.#current!.id);
125 + const newId = this.getIdAt(oldPlace, this.#current!.id);
126 return {
127 ...oldPlace,
128 identifier: newId,
129 };
130 }
131
132 - getIdAt(oldId: Identifier, blockId: BlockId): Identifier {
132 + getIdAt(oldPlace: Place, blockId: BlockId): Identifier {
133 // check if Place is defined locally
134 const block = this.#blocks.get(blockId)!;
135 const state = this.#states.get(block)!;
136
137 - if (state.defs.has(oldId)) {
138 - return state.defs.get(oldId)!;
137 + if (state.defs.has(oldPlace.identifier)) {
138 + return state.defs.get(oldPlace.identifier)!;
139 }
140
141 if (block.preds.size == 0) {
142 /*
143 * We're at the entry block and haven't found our defintion yet.
144 * console.log(
145 - * `Unable to find "${printIdentifier(
146 - * oldId
145 + * `Unable to find "${printPlace(
146 + * oldPlace
147 * )}" in bb${blockId}, assuming it's a global`
148 * );
149 */
150 - this.#unknown.add(oldId);
151 - return oldId;
150 + this.#unknown.add(oldPlace.identifier);
151 + return oldPlace.identifier;
152 }
153
154 if (this.unsealedPreds.get(block)! > 0) {
@@ -156,52 +156,55 @@ class SSABuilder {
156 * We haven't visited all our predecessors, let's place an incomplete phi
157 * for now.
158 */
159 - const newId = this.makeId(oldId);
160 - state.incompletePhis.push({oldId, newId});
161 - state.defs.set(oldId, newId);
159 + const newId = this.makeId(oldPlace.identifier);
160 + state.incompletePhis.push({
161 + oldPlace,
162 + newPlace: {...oldPlace, identifier: newId},
163 + });
164 + state.defs.set(oldPlace.identifier, newId);
165 return newId;
166 }
167
168 // Only one predecessor, let's check there
169 if (block.preds.size == 1) {
170 const [pred] = block.preds;
168 - const newId = this.getIdAt(oldId, pred);
169 - state.defs.set(oldId, newId);
171 + const newId = this.getIdAt(oldPlace, pred);
172 + state.defs.set(oldPlace.identifier, newId);
173 return newId;
174 }
175
176 // There are multiple predecessors, we may need a phi.
174 - const newId = this.makeId(oldId);
177 + const newId = this.makeId(oldPlace.identifier);
178 /*
179 * Adding a phi may loop back to our block if there is a loop in the CFG. We
180 * update our defs before adding the phi to terminate the recursion rather than
181 * looping infinitely.
182 */
180 - state.defs.set(oldId, newId);
181 - return this.addPhi(block, oldId, newId);
183 + state.defs.set(oldPlace.identifier, newId);
184 + return this.addPhi(block, oldPlace, {...oldPlace, identifier: newId});
185 }
186
184 - addPhi(block: BasicBlock, oldId: Identifier, newId: Identifier): Identifier {
185 - const predDefs: Map<BlockId, Identifier> = new Map();
187 + addPhi(block: BasicBlock, oldPlace: Place, newPlace: Place): Identifier {
188 + const predDefs: Map<BlockId, Place> = new Map();
189 for (const predBlockId of block.preds) {
187 - const predId = this.getIdAt(oldId, predBlockId);
188 - predDefs.set(predBlockId, predId);
190 + const predId = this.getIdAt(oldPlace, predBlockId);
191 + predDefs.set(predBlockId, {...oldPlace, identifier: predId});
192 }
193
194 const phi: Phi = {
195 kind: 'Phi',
193 - id: newId,
196 + place: newPlace,
197 operands: predDefs,
198 };
199
200 block.phis.add(phi);
198 - return newId;
201 + return newPlace.identifier;
202 }
203
204 fixIncompletePhis(block: BasicBlock): void {
205 const state = this.#states.get(block)!;
206 for (const phi of state.incompletePhis) {
204 - this.addPhi(block, phi.oldId, phi.newId);
207 + this.addPhi(block, phi.oldPlace, phi.newPlace);
208 }
209 }
210
@@ -223,9 +226,9 @@ class SSABuilder {
226
227 for (const incompletePhi of state.incompletePhis) {
228 text.push(
226 - ` iphi \$${printIdentifier(
227 - incompletePhi.newId,
228 - )} = \$${printIdentifier(incompletePhi.oldId)}`,
229 + ` iphi \$${printPlace(
230 + incompletePhi.newPlace,
231 + )} = \$${printPlace(incompletePhi.oldPlace)}`,
232 );
233 }
234 }
compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts
+3 -3
@@ -69,7 +69,7 @@ export function inferTypes(func: HIRFunction): void {
69 function apply(func: HIRFunction, unifier: Unifier): void {
70 for (const [_, block] of func.body.blocks) {
71 for (const phi of block.phis) {
72 - phi.id.type = unifier.get(phi.id.type);
72 + phi.place.identifier.type = unifier.get(phi.place.identifier.type);
73 }
74 for (const instr of block.instructions) {
75 for (const operand of eachInstructionLValue(instr)) {
@@ -127,9 +127,9 @@ function* generate(
127 const returnTypes: Array<Type> = [];
128 for (const [_, block] of func.body.blocks) {
129 for (const phi of block.phis) {
130 - yield equation(phi.id.type, {
130 + yield equation(phi.place.identifier.type, {
131 kind: 'Phi',
132 - operands: [...phi.operands.values()].map(id => id.type),
132 + operands: [...phi.operands.values()].map(id => id.identifier.type),
133 });
134 }
135
compiler/packages/babel-plugin-react-compiler/src/TypeInference/PropagatePhiTypes.ts
+8 -5
@@ -62,21 +62,24 @@ export function propagatePhiTypes(fn: HIRFunction): void {
62 * We also don't propagate scopes for named variables, to preserve compatibility
63 * with previous LeaveSSA behavior.
64 */
65 - if (phi.id.type.kind !== 'Type' || phi.id.name !== null) {
65 + if (
66 + phi.place.identifier.type.kind !== 'Type' ||
67 + phi.place.identifier.name !== null
68 + ) {
69 continue;
70 }
71 let type: Type | null = null;
72 for (const [, operand] of phi.operands) {
73 if (type === null) {
71 - type = operand.type;
72 - } else if (!typeEquals(type, operand.type)) {
74 + type = operand.identifier.type;
75 + } else if (!typeEquals(type, operand.identifier.type)) {
76 type = null;
77 break;
78 }
79 }
80 if (type !== null) {
78 - phi.id.type = type;
79 - propagated.add(phi.id.id);
81 + phi.place.identifier.type = type;
82 + propagated.add(phi.place.identifier.id);
83 }
84 }
85 for (const instr of block.instructions) {
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateHooksUsage.ts
+4 -3
@@ -198,11 +198,12 @@ export function validateHooksUsage(fn: HIRFunction): void {
198 for (const [, block] of fn.body.blocks) {
199 for (const phi of block.phis) {
200 let kind: Kind =
201 - phi.id.name !== null && isHookName(phi.id.name.value)
201 + phi.place.identifier.name !== null &&
202 + isHookName(phi.place.identifier.name.value)
203 ? Kind.PotentialHook
204 : Kind.Local;
205 for (const [, operand] of phi.operands) {
205 - const operandKind = valueKinds.get(operand.id);
206 + const operandKind = valueKinds.get(operand.identifier.id);
207 /*
208 * NOTE: we currently skip operands whose value is unknown
209 * (which can only occur for functions with loops), we may
@@ -213,7 +214,7 @@ export function validateHooksUsage(fn: HIRFunction): void {
214 kind = joinKinds(kind, operandKind);
215 }
216 }
216 - valueKinds.set(phi.id.id, kind);
217 + valueKinds.set(phi.place.identifier.id, kind);
218 }
219 for (const instr of block.instructions) {
220 switch (instr.value.kind) {
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts
+3 -2
@@ -209,10 +209,11 @@ function validateNoRefAccessInRenderImpl(
209 for (const [, block] of fn.body.blocks) {
210 for (const phi of block.phis) {
211 env.set(
212 - phi.id.id,
212 + phi.place.identifier.id,
213 joinRefAccessTypes(
214 ...Array(...phi.operands.values()).map(
215 - operand => env.get(operand.id) ?? ({kind: 'None'} as const),
215 + operand =>
216 + env.get(operand.identifier.id) ?? ({kind: 'None'} as const),
217 ),
218 ),
219 );