Skip to content

Commit bfb7d22

Browse files
josephsavonajosephsavona
committed
[wip][compiler] Stop rewriting IdentifierId in LeaveSSA
ghstack-source-id: 1b4da81 Pull Request resolved: #30573
1 parent 3682b79 commit bfb7d22

12 files changed

+314
-95
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
465465
break;
466466
}
467467
case 'DeclareContext': {
468-
value = `DeclareContext ${instrValue.lvalue.kind} ${printPlace(
468+
value = `DeclareContext (${instrValue.lvalue.kind}) ${instrValue.lvalue.kind} ${printPlace(
469469
instrValue.lvalue.place,
470470
)}`;
471471
break;
@@ -833,6 +833,7 @@ export function printPlace(place: Place): string {
833833
}
834834

835835
export function printIdentifier(id: Identifier): string {
836+
// return `${printName(id.name)}\$${id.id}${Number(id.declarationId) !== id.id ? `_d${id.declarationId}` : ''}${printScope(id.scope)}`;
836837
return `${printName(id.name)}\$${id.id}${printScope(id.scope)}`;
837838
}
838839

‎compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {Environment, EnvironmentConfig, ExternalFunction} from '../HIR';
1818
import {
1919
ArrayPattern,
2020
BlockId,
21+
DeclarationId,
2122
GeneratedSource,
2223
Identifier,
2324
IdentifierId,
@@ -309,9 +310,9 @@ function codegenReactiveFunction(
309310
): Result<CodegenFunction, CompilerError> {
310311
for (const param of fn.params) {
311312
if (param.kind === 'Identifier') {
312-
cx.temp.set(param.identifier.id, null);
313+
cx.temp.set(param.identifier.declarationId, null);
313314
} else {
314-
cx.temp.set(param.place.identifier.id, null);
315+
cx.temp.set(param.place.identifier.declarationId, null);
315316
}
316317
}
317318

@@ -392,7 +393,7 @@ class Context {
392393
env: Environment;
393394
fnName: string;
394395
#nextCacheIndex: number = 0;
395-
#declarations: Set<IdentifierId> = new Set();
396+
#declarations: Set<DeclarationId> = new Set();
396397
temp: Temporaries;
397398
errors: CompilerError = new CompilerError();
398399
objectMethods: Map<IdentifierId, ObjectMethod> = new Map();
@@ -418,11 +419,11 @@ class Context {
418419
}
419420

420421
declare(identifier: Identifier): void {
421-
this.#declarations.add(identifier.id);
422+
this.#declarations.add(identifier.declarationId);
422423
}
423424

424425
hasDeclared(identifier: Identifier): boolean {
425-
return this.#declarations.has(identifier.id);
426+
return this.#declarations.has(identifier.declarationId);
426427
}
427428

428429
synthesizeName(name: string): ValidIdentifierName {
@@ -1147,7 +1148,7 @@ function codegenTerminal(
11471148
let catchParam = null;
11481149
if (terminal.handlerBinding !== null) {
11491150
catchParam = convertIdentifier(terminal.handlerBinding.identifier);
1150-
cx.temp.set(terminal.handlerBinding.identifier.id, null);
1151+
cx.temp.set(terminal.handlerBinding.identifier.declarationId, null);
11511152
}
11521153
return t.tryStatement(
11531154
codegenBlock(cx, terminal.block),
@@ -1205,7 +1206,7 @@ function codegenInstructionNullable(
12051206
kind !== InstructionKind.Reassign &&
12061207
place.identifier.name === null
12071208
) {
1208-
cx.temp.set(place.identifier.id, null);
1209+
cx.temp.set(place.identifier.declarationId, null);
12091210
}
12101211
const isDeclared = cx.hasDeclared(place.identifier);
12111212
hasReasign ||= isDeclared;
@@ -1261,7 +1262,7 @@ function codegenInstructionNullable(
12611262
);
12621263
if (instr.lvalue !== null) {
12631264
if (instr.value.kind !== 'StoreContext') {
1264-
cx.temp.set(instr.lvalue.identifier.id, expr);
1265+
cx.temp.set(instr.lvalue.identifier.declarationId, expr);
12651266
return null;
12661267
} else {
12671268
// Handle chained reassignments for context variables
@@ -1530,7 +1531,7 @@ function createCallExpression(
15301531
}
15311532
}
15321533

1533-
type Temporaries = Map<IdentifierId, t.Expression | t.JSXText | null>;
1534+
type Temporaries = Map<DeclarationId, t.Expression | t.JSXText | null>;
15341535

15351536
function codegenLabel(id: BlockId): string {
15361537
return `bb${id}`;
@@ -1549,7 +1550,7 @@ function codegenInstruction(
15491550
}
15501551
if (instr.lvalue.identifier.name === null) {
15511552
// temporary
1552-
cx.temp.set(instr.lvalue.identifier.id, value);
1553+
cx.temp.set(instr.lvalue.identifier.declarationId, value);
15531554
return t.emptyStatement();
15541555
} else {
15551556
const expressionValue = convertValueToExpression(value);
@@ -2498,7 +2499,7 @@ function codegenPlaceToExpression(cx: Context, place: Place): t.Expression {
24982499
}
24992500

25002501
function codegenPlace(cx: Context, place: Place): t.Expression | t.JSXText {
2501-
let tmp = cx.temp.get(place.identifier.id);
2502+
let tmp = cx.temp.get(place.identifier.declarationId);
25022503
if (tmp != null) {
25032504
return tmp;
25042505
}

‎compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
import {
9+
DeclarationId,
910
Destructure,
1011
Environment,
1112
IdentifierId,
@@ -17,6 +18,7 @@ import {
1718
ReactiveStatement,
1819
promoteTemporary,
1920
} from '../HIR';
21+
import {createTemporaryPlace} from '../HIR/HIRBuilder';
2022
import {eachPatternOperand, mapPatternOperands} from '../HIR/visitors';
2123
import {
2224
ReactiveFunctionTransform,
@@ -82,7 +84,7 @@ export function extractScopeDeclarationsFromDestructuring(
8284

8385
class State {
8486
env: Environment;
85-
declared: Set<IdentifierId> = new Set();
87+
declared: Set<DeclarationId> = new Set();
8688

8789
constructor(env: Environment) {
8890
this.env = env;
@@ -92,7 +94,7 @@ class State {
9294
class Visitor extends ReactiveFunctionTransform<State> {
9395
override visitScope(scope: ReactiveScopeBlock, state: State): void {
9496
for (const [, declaration] of scope.scope.declarations) {
95-
state.declared.add(declaration.identifier.id);
97+
state.declared.add(declaration.identifier.declarationId);
9698
}
9799
this.traverseScope(scope, state);
98100
}
@@ -131,7 +133,7 @@ function transformDestructuring(
131133
let reassigned: Set<IdentifierId> = new Set();
132134
let hasDeclaration = false;
133135
for (const place of eachPatternOperand(destructure.lvalue.pattern)) {
134-
const isDeclared = state.declared.has(place.identifier.id);
136+
const isDeclared = state.declared.has(place.identifier.declarationId);
135137
if (isDeclared) {
136138
reassigned.add(place.identifier.id);
137139
}
@@ -150,15 +152,7 @@ function transformDestructuring(
150152
if (!reassigned.has(place.identifier.id)) {
151153
return place;
152154
}
153-
const tempId = state.env.nextIdentifierId;
154-
const temporary = {
155-
...place,
156-
identifier: {
157-
...place.identifier,
158-
id: tempId,
159-
name: null, // overwritten below
160-
},
161-
};
155+
const temporary = createTemporaryPlace(state.env, place.loc);
162156
promoteTemporary(temporary.identifier);
163157
renamed.set(place, temporary);
164158
return temporary;

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import {CompilerError, SourceLocation} from '..';
99
import {Environment} from '../HIR';
1010
import {
11+
DeclarationId,
1112
GeneratedSource,
1213
HIRFunction,
1314
Identifier,
@@ -257,21 +258,34 @@ export function findDisjointMutableValues(
257258
fn: HIRFunction,
258259
): DisjointSet<Identifier> {
259260
const scopeIdentifiers = new DisjointSet<Identifier>();
261+
262+
const declarations = new Map<DeclarationId, Identifier>();
263+
function declareIdentifier(lvalue: Place): void {
264+
if (!declarations.has(lvalue.identifier.declarationId)) {
265+
declarations.set(lvalue.identifier.declarationId, lvalue.identifier);
266+
}
267+
}
268+
260269
for (const [_, block] of fn.body.blocks) {
261270
/*
262271
* If a phi is mutated after creation, then we need to alias all of its operands such that they
263272
* are assigned to the same scope.
264273
*/
265274
for (const phi of block.phis) {
266275
if (
267-
// The phi was reset because it was not mutated after creation
268276
phi.id.mutableRange.start + 1 !== phi.id.mutableRange.end &&
269277
phi.id.mutableRange.end >
270278
(block.instructions.at(0)?.id ?? block.terminal.id)
271279
) {
272-
for (const [, phiId] of phi.operands) {
273-
scopeIdentifiers.union([phi.id, phiId]);
280+
const operands = [phi.id];
281+
const declaration = declarations.get(phi.id.declarationId);
282+
if (declaration !== undefined) {
283+
operands.push(declaration);
284+
}
285+
for (const [_, phiId] of phi.operands) {
286+
operands.push(phiId);
274287
}
288+
scopeIdentifiers.union(operands);
275289
} else if (fn.env.config.enableForest) {
276290
for (const [, phiId] of phi.operands) {
277291
scopeIdentifiers.union([phi.id, phiId]);
@@ -286,9 +300,15 @@ export function findDisjointMutableValues(
286300
operands.push(instr.lvalue!.identifier);
287301
}
288302
if (
303+
instr.value.kind === 'DeclareLocal' ||
304+
instr.value.kind === 'DeclareContext'
305+
) {
306+
declareIdentifier(instr.value.lvalue.place);
307+
} else if (
289308
instr.value.kind === 'StoreLocal' ||
290309
instr.value.kind === 'StoreContext'
291310
) {
311+
declareIdentifier(instr.value.lvalue.place);
292312
if (
293313
instr.value.lvalue.place.identifier.mutableRange.end >
294314
instr.value.lvalue.place.identifier.mutableRange.start + 1
@@ -303,6 +323,7 @@ export function findDisjointMutableValues(
303323
}
304324
} else if (instr.value.kind === 'Destructure') {
305325
for (const place of eachPatternOperand(instr.value.lvalue.pattern)) {
326+
declareIdentifier(place);
306327
if (
307328
place.identifier.mutableRange.end >
308329
place.identifier.mutableRange.start + 1

‎compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
import {CompilerError} from '../CompilerError';
99
import {GeneratedSource} from '../HIR';
1010
import {
11+
DeclarationId,
1112
Identifier,
12-
IdentifierId,
1313
InstructionId,
1414
Place,
1515
PrunedReactiveScopeBlock,
@@ -24,7 +24,6 @@ import {ReactiveFunctionVisitor, visitReactiveFunction} from './visitors';
2424

2525
class Visitor extends ReactiveFunctionVisitor<State> {
2626
override visitScope(scopeBlock: ReactiveScopeBlock, state: State): void {
27-
this.traverseScope(scopeBlock, state);
2827
for (const dep of scopeBlock.scope.dependencies) {
2928
const {identifier} = dep;
3029
if (identifier.name == null) {
@@ -43,21 +42,23 @@ class Visitor extends ReactiveFunctionVisitor<State> {
4342
promoteIdentifier(declaration.identifier, state);
4443
}
4544
}
45+
this.traverseScope(scopeBlock, state);
4646
}
4747

4848
override visitPrunedScope(
4949
scopeBlock: PrunedReactiveScopeBlock,
5050
state: State,
5151
): void {
52-
this.traversePrunedScope(scopeBlock, state);
5352
for (const [, declaration] of scopeBlock.scope.declarations) {
5453
if (
5554
declaration.identifier.name == null &&
56-
state.pruned.get(declaration.identifier.id)?.usedOutsideScope === true
55+
state.pruned.get(declaration.identifier.declarationId)
56+
?.usedOutsideScope === true
5757
) {
5858
promoteIdentifier(declaration.identifier, state);
5959
}
6060
}
61+
this.traversePrunedScope(scopeBlock, state);
6162
}
6263

6364
override visitParam(place: Place, state: State): void {
@@ -93,11 +94,38 @@ class Visitor extends ReactiveFunctionVisitor<State> {
9394
}
9495
}
9596

96-
type JsxExpressionTags = Set<IdentifierId>;
97+
class Visitor2 extends ReactiveFunctionVisitor<State> {
98+
override visitPlace(_id: InstructionId, place: Place, state: State): void {
99+
if (
100+
place.identifier.name === null &&
101+
state.promoted.has(place.identifier.declarationId)
102+
) {
103+
promoteIdentifier(place.identifier, state);
104+
}
105+
}
106+
override visitLValue(
107+
_id: InstructionId,
108+
_lvalue: Place,
109+
_state: State,
110+
): void {
111+
this.visitPlace(_id, _lvalue, _state);
112+
}
113+
override visitReactiveFunctionValue(
114+
_id: InstructionId,
115+
_dependencies: Array<Place>,
116+
fn: ReactiveFunction,
117+
state: State,
118+
): void {
119+
visitReactiveFunction(fn, this, state);
120+
}
121+
}
122+
123+
type JsxExpressionTags = Set<DeclarationId>;
97124
type State = {
98125
tags: JsxExpressionTags;
126+
promoted: Set<DeclarationId>;
99127
pruned: Map<
100-
IdentifierId,
128+
DeclarationId,
101129
{activeScopes: Array<ScopeId>; usedOutsideScope: boolean}
102130
>; // true if referenced within another scope, false if only accessed outside of scopes
103131
};
@@ -108,9 +136,9 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor<State> {
108136
override visitPlace(_id: InstructionId, place: Place, state: State): void {
109137
if (
110138
this.activeScopes.length !== 0 &&
111-
state.pruned.has(place.identifier.id)
139+
state.pruned.has(place.identifier.declarationId)
112140
) {
113-
const prunedPlace = state.pruned.get(place.identifier.id)!;
141+
const prunedPlace = state.pruned.get(place.identifier.declarationId)!;
114142
if (prunedPlace.activeScopes.indexOf(this.activeScopes.at(-1)!) === -1) {
115143
prunedPlace.usedOutsideScope = true;
116144
}
@@ -124,16 +152,16 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor<State> {
124152
): void {
125153
this.traverseValue(id, value, state);
126154
if (value.kind === 'JsxExpression' && value.tag.kind === 'Identifier') {
127-
state.tags.add(value.tag.identifier.id);
155+
state.tags.add(value.tag.identifier.declarationId);
128156
}
129157
}
130158

131159
override visitPrunedScope(
132160
scopeBlock: PrunedReactiveScopeBlock,
133161
state: State,
134162
): void {
135-
for (const [id] of scopeBlock.scope.declarations) {
136-
state.pruned.set(id, {
163+
for (const [_id, decl] of scopeBlock.scope.declarations) {
164+
state.pruned.set(decl.identifier.declarationId, {
137165
activeScopes: [...this.activeScopes],
138166
usedOutsideScope: false,
139167
});
@@ -151,6 +179,7 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor<State> {
151179
export function promoteUsedTemporaries(fn: ReactiveFunction): void {
152180
const state: State = {
153181
tags: new Set(),
182+
promoted: new Set(),
154183
pruned: new Map(),
155184
};
156185
visitReactiveFunction(fn, new CollectPromotableTemporaries(), state);
@@ -161,6 +190,7 @@ export function promoteUsedTemporaries(fn: ReactiveFunction): void {
161190
}
162191
}
163192
visitReactiveFunction(fn, new Visitor(), state);
193+
visitReactiveFunction(fn, new Visitor2(), state);
164194
}
165195

166196
function promoteIdentifier(identifier: Identifier, state: State): void {
@@ -171,9 +201,10 @@ function promoteIdentifier(identifier: Identifier, state: State): void {
171201
loc: GeneratedSource,
172202
suggestions: null,
173203
});
174-
if (state.tags.has(identifier.id)) {
204+
if (state.tags.has(identifier.declarationId)) {
175205
promoteTemporaryJsxTag(identifier);
176206
} else {
177207
promoteTemporary(identifier);
178208
}
209+
state.promoted.add(identifier.declarationId);
179210
}

0 commit comments

Comments
 (0)