Skip to content

Commit 6011c1e

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

File tree

9 files changed

+246
-75
lines changed

9 files changed

+246
-75
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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/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
}

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

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import {CompilerError} from '../CompilerError';
99
import {
1010
BlockId,
11+
DeclarationId,
1112
GeneratedSource,
1213
Identifier,
1314
IdentifierId,
@@ -76,9 +77,9 @@ type TemporariesUsedOutsideDefiningScope = {
7677
* tracks all relevant temporary declarations (currently LoadLocal and PropertyLoad)
7778
* and the scope where they are defined
7879
*/
79-
declarations: Map<IdentifierId, ScopeId>;
80+
declarations: Map<DeclarationId, ScopeId>;
8081
// temporaries used outside of their defining scope
81-
usedOutsideDeclaringScope: Set<IdentifierId>;
82+
usedOutsideDeclaringScope: Set<DeclarationId>;
8283
};
8384
class FindPromotedTemporaries extends ReactiveFunctionVisitor<TemporariesUsedOutsideDefiningScope> {
8485
scopes: Array<ScopeId> = [];
@@ -107,7 +108,10 @@ class FindPromotedTemporaries extends ReactiveFunctionVisitor<TemporariesUsedOut
107108
case 'LoadLocal':
108109
case 'LoadContext':
109110
case 'PropertyLoad': {
110-
state.declarations.set(instruction.lvalue.identifier.id, scope);
111+
state.declarations.set(
112+
instruction.lvalue.identifier.declarationId,
113+
scope,
114+
);
111115
break;
112116
}
113117
default: {
@@ -121,18 +125,20 @@ class FindPromotedTemporaries extends ReactiveFunctionVisitor<TemporariesUsedOut
121125
place: Place,
122126
state: TemporariesUsedOutsideDefiningScope,
123127
): void {
124-
const declaringScope = state.declarations.get(place.identifier.id);
128+
const declaringScope = state.declarations.get(
129+
place.identifier.declarationId,
130+
);
125131
if (declaringScope === undefined) {
126132
return;
127133
}
128134
if (this.scopes.indexOf(declaringScope) === -1) {
129135
// Declaring scope is not active === used outside declaring scope
130-
state.usedOutsideDeclaringScope.add(place.identifier.id);
136+
state.usedOutsideDeclaringScope.add(place.identifier.declarationId);
131137
}
132138
}
133139
}
134140

135-
type DeclMap = Map<IdentifierId, Decl>;
141+
type DeclMap = Map<DeclarationId, Decl>;
136142
type Decl = {
137143
id: InstructionId;
138144
scope: Stack<ScopeTraversalState>;
@@ -280,7 +286,7 @@ class PoisonState {
280286
}
281287

282288
class Context {
283-
#temporariesUsedOutsideScope: Set<IdentifierId>;
289+
#temporariesUsedOutsideScope: Set<DeclarationId>;
284290
#declarations: DeclMap = new Map();
285291
#reassignments: Map<Identifier, Decl> = new Map();
286292
// Reactive dependencies used in the current reactive scope.
@@ -307,7 +313,7 @@ class Context {
307313
#scopes: Stack<ScopeTraversalState> = empty();
308314
poisonState: PoisonState = new PoisonState(new Set(), new Set(), false);
309315

310-
constructor(temporariesUsedOutsideScope: Set<IdentifierId>) {
316+
constructor(temporariesUsedOutsideScope: Set<DeclarationId>) {
311317
this.#temporariesUsedOutsideScope = temporariesUsedOutsideScope;
312318
}
313319

@@ -377,7 +383,9 @@ class Context {
377383
}
378384

379385
isUsedOutsideDeclaringScope(place: Place): boolean {
380-
return this.#temporariesUsedOutsideScope.has(place.identifier.id);
386+
return this.#temporariesUsedOutsideScope.has(
387+
place.identifier.declarationId,
388+
);
381389
}
382390

383391
/*
@@ -440,8 +448,8 @@ class Context {
440448
* on itself.
441449
*/
442450
declare(identifier: Identifier, decl: Decl): void {
443-
if (!this.#declarations.has(identifier.id)) {
444-
this.#declarations.set(identifier.id, decl);
451+
if (!this.#declarations.has(identifier.declarationId)) {
452+
this.#declarations.set(identifier.declarationId, decl);
445453
}
446454
this.#reassignments.set(identifier, decl);
447455
}
@@ -533,7 +541,7 @@ class Context {
533541
*/
534542
const currentDeclaration =
535543
this.#reassignments.get(identifier) ??
536-
this.#declarations.get(identifier.id);
544+
this.#declarations.get(identifier.declarationId);
537545
const currentScope = this.currentScope.value?.value;
538546
return (
539547
currentScope != null &&
@@ -599,7 +607,7 @@ class Context {
599607
* (all other decls e.g. `let x;` should be initialized in BuildHIR)
600608
*/
601609
const originalDeclaration = this.#declarations.get(
602-
maybeDependency.identifier.id,
610+
maybeDependency.identifier.declarationId,
603611
);
604612
if (
605613
originalDeclaration !== undefined &&

0 commit comments

Comments
 (0)