Skip to content

Commit 512d6f4

Browse files
committed
Update on "[compiler] Stay in SSA form through entire pipeline"
This PR updates to use SSA form through the entire compilation pipeline. This means that in both HIR form and ReactiveFunction form, `Identifier` instances map 1:1 to `IdentifierId` values. If two identifiers have the same IdentifierId, they are the same instance. What this means is that all our passes can use this more precise information to determine if two particular identifiers are not just the same variable, but the same SSA "version" of that variable. However, some parts of our analysis really care about program variables as opposed to SSA versions, and were relying on LeaveSSA to reset identifiers such that all Identifier instances for a particular program variable would have the same IdentifierId (though not necessarily the same Identifier instance). With LeaveSSA removed, those analysis passes can now use DeclarationId instead to uniquely identify a program variable. Note that this PR surfaces some opportunties to improve edge-cases around reassigned values being declared/reassigned/depended-upon across multiple scopes. Several passes could/should use IdentifierId to more precisely identify exactly which values are accessed - for example, a scope that reassigns `x` but doesn't use `x` prior to reassignment doesn't have to take a dependency on `x`. But today we take a dependnecy. My approach for these cases was to add a "TODO LeaveSSA" comment with notes and the name of the fixture demonstrating the difference, but to intentionally preserve the existing behavior (generally, switching to use DeclarationId when IdentifierId would have been more precise). Beyond updating passes to use DeclarationId instead of Identifier/IdentifierId, the other change here is to extract out the remaining necessary bits of LeaveSSA into a new pass that rewrites InstructionKind (const/let/reassign/etc) based on whether a value is actually const or has reassignments and should be let. [ghstack-poisoned]
2 parents 8559ed8 + f585898 commit 512d6f4

File tree

4 files changed

+6
-15
lines changed

4 files changed

+6
-15
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1213,7 +1213,7 @@ export type ValidIdentifierName = string & {
12131213
[opaqueValidIdentifierName]: 'ValidIdentifierName';
12141214
};
12151215

1216-
export function makeTemporary(
1216+
export function makeTemporaryIdentifier(
12171217
id: IdentifierId,
12181218
loc: SourceLocation,
12191219
): Identifier {

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

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import {
2828
makeDeclarationId,
2929
makeIdentifierName,
3030
makeInstructionId,
31-
makeTemporary,
31+
makeTemporaryIdentifier,
3232
makeType,
3333
} from './HIR';
3434
import {printInstruction} from './PrintHIR';
@@ -184,7 +184,7 @@ export default class HIRBuilder {
184184

185185
makeTemporary(loc: SourceLocation): Identifier {
186186
const id = this.nextIdentifierId;
187-
return makeTemporary(id, loc);
187+
return makeTemporaryIdentifier(id, loc);
188188
}
189189

190190
#resolveBabelBinding(
@@ -891,18 +891,9 @@ export function createTemporaryPlace(
891891
env: Environment,
892892
loc: SourceLocation,
893893
): Place {
894-
const id = env.nextIdentifierId;
895894
return {
896895
kind: 'Identifier',
897-
identifier: {
898-
id,
899-
declarationId: makeDeclarationId(id),
900-
mutableRange: {start: makeInstructionId(0), end: makeInstructionId(0)},
901-
name: null,
902-
scope: null,
903-
type: makeType(),
904-
loc,
905-
},
896+
identifier: makeTemporaryIdentifier(env.nextIdentifierId, loc),
906897
reactive: false,
907898
effect: Effect.Unknown,
908899
loc: GeneratedSource,

fixtures/flight/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"version": "0.1.0",
55
"private": true,
66
"devEngines": {
7-
"node": "20.x || 21.x"
7+
"node": "20.x || 22.x"
88
},
99
"dependencies": {
1010
"@babel/core": "^7.16.0",

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@
103103
"yargs": "^15.3.1"
104104
},
105105
"devEngines": {
106-
"node": "16.x || 18.x || 20.x || 21.x"
106+
"node": "16.x || 18.x || 20.x || 22.x"
107107
},
108108
"jest": {
109109
"testRegex": "/scripts/jest/dont-run-jest-directly\\.js$"

0 commit comments

Comments
 (0)