Skip to content

Commit 7b7fac0

Browse files
committed
[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: 5f4fb39 Pull Request resolved: #31171
1 parent 70fb136 commit 7b7fac0

20 files changed

+103
-99
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ export function assertConsistentIdentifiers(fn: HIRFunction): void {
2929
const assignments: Set<IdentifierId> = new Set();
3030
for (const [, block] of fn.body.blocks) {
3131
for (const phi of block.phis) {
32-
validate(identifiers, phi.id);
32+
validate(identifiers, phi.place.identifier);
3333
for (const [, operand] of phi.operands) {
34-
validate(identifiers, operand);
34+
validate(identifiers, operand.identifier);
3535
}
3636
}
3737
for (const instr of block.instructions) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ import {
2020
export function assertValidMutableRanges(fn: HIRFunction): void {
2121
for (const [, block] of fn.body.blocks) {
2222
for (const phi of block.phis) {
23-
visitIdentifier(phi.id);
23+
visitIdentifier(phi.place.identifier);
2424
for (const [, operand] of phi.operands) {
25-
visitIdentifier(operand);
25+
visitIdentifier(operand.identifier);
2626
}
2727
}
2828
for (const instr of block.instructions) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -761,8 +761,8 @@ function _staticInvariantInstructionValueHasLocation(
761761

762762
export type Phi = {
763763
kind: 'Phi';
764-
id: Identifier;
765-
operands: Map<BlockId, Identifier>;
764+
place: Place;
765+
operands: Map<BlockId, Place>;
766766
};
767767

768768
/**

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,20 +84,14 @@ export function mergeConsecutiveBlocks(fn: HIRFunction): void {
8484
id: predecessor.terminal.id,
8585
lvalue: {
8686
kind: 'Identifier',
87-
identifier: phi.id,
87+
identifier: phi.place.identifier,
8888
effect: Effect.ConditionallyMutate,
8989
reactive: false,
9090
loc: GeneratedSource,
9191
},
9292
value: {
9393
kind: 'LoadLocal',
94-
place: {
95-
kind: 'Identifier',
96-
identifier: operand,
97-
effect: Effect.Read,
98-
reactive: false,
99-
loc: GeneratedSource,
100-
},
94+
place: {...operand},
10195
loc: GeneratedSource,
10296
},
10397
loc: GeneratedSource,

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,13 @@ export function printInstruction(instr: ReactiveInstruction): string {
163163

164164
export function printPhi(phi: Phi): string {
165165
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));
169169
items.push(': phi(');
170170
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)}`);
173173
}
174174

175175
items.push(phis.join(', '));

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,7 @@ function collectDependencies(
607607
// Record referenced optional chains in phis
608608
for (const phi of block.phis) {
609609
for (const operand of phi.operands) {
610-
const maybeOptionalChain = temporaries.get(operand[1].id);
610+
const maybeOptionalChain = temporaries.get(operand[1].identifier.id);
611611
if (maybeOptionalChain) {
612612
context.visitDependency(maybeOptionalChain);
613613
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ export function inferAliasForPhis(
1515
for (const [_, block] of func.body.blocks) {
1616
for (const phi of block.phis) {
1717
const isPhiMutatedAfterCreation: boolean =
18-
phi.id.mutableRange.end >
18+
phi.place.identifier.mutableRange.end >
1919
(block.instructions.at(0)?.id ?? block.terminal.id);
2020
if (isPhiMutatedAfterCreation) {
2121
for (const [, operand] of phi.operands) {
22-
aliases.union([phi.id, operand]);
22+
aliases.union([phi.place.identifier, operand.identifier]);
2323
}
2424
}
2525
}

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,19 +116,23 @@ export function inferMutableLifetimes(
116116
for (const [_, block] of func.body.blocks) {
117117
for (const phi of block.phis) {
118118
const isPhiMutatedAfterCreation: boolean =
119-
phi.id.mutableRange.end >
119+
phi.place.identifier.mutableRange.end >
120120
(block.instructions.at(0)?.id ?? block.terminal.id);
121121
if (
122122
inferMutableRangeForStores &&
123123
isPhiMutatedAfterCreation &&
124-
phi.id.mutableRange.start === 0
124+
phi.place.identifier.mutableRange.start === 0
125125
) {
126126
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;
129130
} 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+
),
132136
);
133137
}
134138
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,23 +162,23 @@ export function inferReactivePlaces(fn: HIRFunction): void {
162162
let hasReactiveControl = isReactiveControlledBlock(block.id);
163163

164164
for (const phi of block.phis) {
165-
if (reactiveIdentifiers.isReactiveIdentifier(phi.id)) {
165+
if (reactiveIdentifiers.isReactive(phi.place)) {
166166
// Already marked reactive on a previous pass
167167
continue;
168168
}
169169
let isPhiReactive = false;
170170
for (const [, operand] of phi.operands) {
171-
if (reactiveIdentifiers.isReactiveIdentifier(operand)) {
171+
if (reactiveIdentifiers.isReactive(operand)) {
172172
isPhiReactive = true;
173173
break;
174174
}
175175
}
176176
if (isPhiReactive) {
177-
reactiveIdentifiers.markReactiveIdentifier(phi.id);
177+
reactiveIdentifiers.markReactive(phi.place);
178178
} else {
179179
for (const [pred] of phi.operands) {
180180
if (isReactiveControlledBlock(pred)) {
181-
reactiveIdentifiers.markReactiveIdentifier(phi.id);
181+
reactiveIdentifiers.markReactive(phi.place);
182182
break;
183183
}
184184
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ class InferenceState {
635635
inferPhi(phi: Phi): void {
636636
const values: Set<InstructionValue> = new Set();
637637
for (const [_, operand] of phi.operands) {
638-
const operandValues = this.#variables.get(operand.id);
638+
const operandValues = this.#variables.get(operand.identifier.id);
639639
// This is a backedge that will be handled later by State.merge
640640
if (operandValues === undefined) continue;
641641
for (const v of operandValues) {
@@ -644,7 +644,7 @@ class InferenceState {
644644
}
645645

646646
if (values.size > 0) {
647-
this.#variables.set(phi.id.id, values);
647+
this.#variables.set(phi.place.identifier.id, values);
648648
}
649649
}
650650
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ function applyConstantPropagation(
117117
for (const phi of block.phis) {
118118
let value = evaluatePhi(phi, constants);
119119
if (value !== null) {
120-
constants.set(phi.id.id, value);
120+
constants.set(phi.place.identifier.id, value);
121121
}
122122
}
123123

@@ -167,7 +167,7 @@ function applyConstantPropagation(
167167
function evaluatePhi(phi: Phi, constants: Constants): Constant | null {
168168
let value: Constant | null = null;
169169
for (const [, operand] of phi.operands) {
170-
const operandValue = constants.get(operand.id) ?? null;
170+
const operandValue = constants.get(operand.identifier.id) ?? null;
171171
// did not find a constant, can't constant propogate
172172
if (operandValue === null) {
173173
return null;

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export function deadCodeElimination(fn: HIRFunction): void {
4242
*/
4343
for (const [, block] of fn.body.blocks) {
4444
for (const phi of block.phis) {
45-
if (!state.isIdOrNameUsed(phi.id)) {
45+
if (!state.isIdOrNameUsed(phi.place.identifier)) {
4646
block.phis.delete(phi);
4747
}
4848
}
@@ -159,9 +159,9 @@ function findReferencedIdentifiers(fn: HIRFunction): State {
159159
}
160160
}
161161
for (const phi of block.phis) {
162-
if (state.isIdOrNameUsed(phi.id)) {
162+
if (state.isIdOrNameUsed(phi.place.identifier)) {
163163
for (const [_pred, operand] of phi.operands) {
164-
state.reference(operand);
164+
state.reference(operand.identifier);
165165
}
166166
}
167167
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
removeUnnecessaryTryCatch,
2424
removeUnreachableForUpdates,
2525
} from '../HIR/HIRBuilder';
26-
import {printIdentifier} from '../HIR/PrintHIR';
26+
import {printPlace} from '../HIR/PrintHIR';
2727

2828
/*
2929
* This pass prunes `maybe-throw` terminals for blocks that can provably *never* throw.
@@ -55,7 +55,7 @@ export function pruneMaybeThrows(fn: HIRFunction): void {
5555
loc: GeneratedSource,
5656
description: `Could not find mapping for predecessor bb${predecessor} in block bb${
5757
block.id
58-
} for phi ${printIdentifier(phi.id)}`,
58+
} for phi ${printPlace(phi.place)}`,
5959
suggestions: null,
6060
});
6161
phi.operands.delete(predecessor);

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -281,22 +281,25 @@ export function findDisjointMutableValues(
281281
*/
282282
for (const phi of block.phis) {
283283
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 >
286287
(block.instructions.at(0)?.id ?? block.terminal.id)
287288
) {
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+
);
290293
if (declaration !== undefined) {
291294
operands.push(declaration);
292295
}
293296
for (const [_, phiId] of phi.operands) {
294-
operands.push(phiId);
297+
operands.push(phiId.identifier);
295298
}
296299
scopeIdentifiers.union(operands);
297300
} else if (fn.env.config.enableForest) {
298301
for (const [, phiId] of phi.operands) {
299-
scopeIdentifiers.union([phi.id, phiId]);
302+
scopeIdentifiers.union([phi.place.identifier, phiId.identifier]);
300303
}
301304
}
302305
}

compiler/packages/babel-plugin-react-compiler/src/SSA/EliminateRedundantPhi.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,18 +68,13 @@ export function eliminateRedundantPhi(
6868
// Find any redundant phis
6969
phis: for (const phi of block.phis) {
7070
// 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));
7772
// Find if the phi can be eliminated
7873
let same: Identifier | null = null;
7974
for (const [_, operand] of phi.operands) {
8075
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
8378
) {
8479
/*
8580
* This operand is the same as the phi or is the same as the
@@ -94,7 +89,7 @@ export function eliminateRedundantPhi(
9489
continue phis;
9590
} else {
9691
// First non-phi operand
97-
same = operand;
92+
same = operand.identifier;
9893
}
9994
}
10095
CompilerError.invariant(same !== null, {
@@ -103,7 +98,7 @@ export function eliminateRedundantPhi(
10398
loc: null,
10499
suggestions: null,
105100
});
106-
rewrites.set(phi.id, same);
101+
rewrites.set(phi.place.identifier, same);
107102
block.phis.delete(phi);
108103
}
109104

0 commit comments

Comments
 (0)