Skip to content

Commit

Permalink
[wip][compiler] Stop rewriting IdentifierId in LeaveSSA
Browse files Browse the repository at this point in the history 
ghstack-source-id: e54a9d44a89a11cd9d65939da17222d85be05712
Pull Request resolved: #30573
  • Loading branch information
@josephsavona
josephsavona committed Aug 1, 2024
1 parent 3682b79 commit 656a7da
Show file tree
Hide file tree
Showing 11 changed files with 281 additions and 91 deletions.
3 changes: 2 additions & 1 deletion compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
break;
}
case 'DeclareContext': {
value = `DeclareContext ${instrValue.lvalue.kind} ${printPlace(
value = `DeclareContext (${instrValue.lvalue.kind}) ${instrValue.lvalue.kind} ${printPlace(
instrValue.lvalue.place,
)}`;
break;
Expand Down Expand Up @@ -833,6 +833,7 @@ export function printPlace(place: Place): string {
}

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

Expand Down
23 changes: 12 additions & 11 deletions compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {Environment, EnvironmentConfig, ExternalFunction} from '../HIR';
import {
ArrayPattern,
BlockId,
DeclarationId,
GeneratedSource,
Identifier,
IdentifierId,
Expand Down Expand Up @@ -309,9 +310,9 @@ function codegenReactiveFunction(
): Result<CodegenFunction, CompilerError> {
for (const param of fn.params) {
if (param.kind === 'Identifier') {
cx.temp.set(param.identifier.id, null);
cx.temp.set(param.identifier.declarationId, null);
} else {
cx.temp.set(param.place.identifier.id, null);
cx.temp.set(param.place.identifier.declarationId, null);
}
}

Expand Down Expand Up @@ -392,7 +393,7 @@ class Context {
env: Environment;
fnName: string;
#nextCacheIndex: number = 0;
#declarations: Set<IdentifierId> = new Set();
#declarations: Set<DeclarationId> = new Set();
temp: Temporaries;
errors: CompilerError = new CompilerError();
objectMethods: Map<IdentifierId, ObjectMethod> = new Map();
Expand All @@ -418,11 +419,11 @@ class Context {
}

declare(identifier: Identifier): void {
this.#declarations.add(identifier.id);
this.#declarations.add(identifier.declarationId);
}

hasDeclared(identifier: Identifier): boolean {
return this.#declarations.has(identifier.id);
return this.#declarations.has(identifier.declarationId);
}

synthesizeName(name: string): ValidIdentifierName {
Expand Down Expand Up @@ -1147,7 +1148,7 @@ function codegenTerminal(
let catchParam = null;
if (terminal.handlerBinding !== null) {
catchParam = convertIdentifier(terminal.handlerBinding.identifier);
cx.temp.set(terminal.handlerBinding.identifier.id, null);
cx.temp.set(terminal.handlerBinding.identifier.declarationId, null);
}
return t.tryStatement(
codegenBlock(cx, terminal.block),
Expand Down Expand Up @@ -1205,7 +1206,7 @@ function codegenInstructionNullable(
kind !== InstructionKind.Reassign &&
place.identifier.name === null
) {
cx.temp.set(place.identifier.id, null);
cx.temp.set(place.identifier.declarationId, null);
}
const isDeclared = cx.hasDeclared(place.identifier);
hasReasign ||= isDeclared;
Expand Down Expand Up @@ -1261,7 +1262,7 @@ function codegenInstructionNullable(
);
if (instr.lvalue !== null) {
if (instr.value.kind !== 'StoreContext') {
cx.temp.set(instr.lvalue.identifier.id, expr);
cx.temp.set(instr.lvalue.identifier.declarationId, expr);
return null;
} else {
// Handle chained reassignments for context variables
Expand Down Expand Up @@ -1530,7 +1531,7 @@ function createCallExpression(
}
}

type Temporaries = Map<IdentifierId, t.Expression | t.JSXText | null>;
type Temporaries = Map<DeclarationId, t.Expression | t.JSXText | null>;

function codegenLabel(id: BlockId): string {
return `bb${id}`;
Expand All @@ -1549,7 +1550,7 @@ function codegenInstruction(
}
if (instr.lvalue.identifier.name === null) {
// temporary
cx.temp.set(instr.lvalue.identifier.id, value);
cx.temp.set(instr.lvalue.identifier.declarationId, value);
return t.emptyStatement();
} else {
const expressionValue = convertValueToExpression(value);
Expand Down Expand Up @@ -2498,7 +2499,7 @@ function codegenPlaceToExpression(cx: Context, place: Place): t.Expression {
}

function codegenPlace(cx: Context, place: Place): t.Expression | t.JSXText {
let tmp = cx.temp.get(place.identifier.id);
let tmp = cx.temp.get(place.identifier.declarationId);
if (tmp != null) {
return tmp;
}
Expand Down
18 changes: 6 additions & 12 deletions ...bel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import {
DeclarationId,
Destructure,
Environment,
IdentifierId,
Expand All @@ -17,6 +18,7 @@ import {
ReactiveStatement,
promoteTemporary,
} from '../HIR';
import {createTemporaryPlace} from '../HIR/HIRBuilder';
import {eachPatternOperand, mapPatternOperands} from '../HIR/visitors';
import {
ReactiveFunctionTransform,
Expand Down Expand Up @@ -82,7 +84,7 @@ export function extractScopeDeclarationsFromDestructuring(

class State {
env: Environment;
declared: Set<IdentifierId> = new Set();
declared: Set<DeclarationId> = new Set();

constructor(env: Environment) {
this.env = env;
Expand All @@ -92,7 +94,7 @@ class State {
class Visitor extends ReactiveFunctionTransform<State> {
override visitScope(scope: ReactiveScopeBlock, state: State): void {
for (const [, declaration] of scope.scope.declarations) {
state.declared.add(declaration.identifier.id);
state.declared.add(declaration.identifier.declarationId);
}
this.traverseScope(scope, state);
}
Expand Down Expand Up @@ -131,7 +133,7 @@ function transformDestructuring(
let reassigned: Set<IdentifierId> = new Set();
let hasDeclaration = false;
for (const place of eachPatternOperand(destructure.lvalue.pattern)) {
const isDeclared = state.declared.has(place.identifier.id);
const isDeclared = state.declared.has(place.identifier.declarationId);
if (isDeclared) {
reassigned.add(place.identifier.id);
}
Expand All @@ -150,15 +152,7 @@ function transformDestructuring(
if (!reassigned.has(place.identifier.id)) {
return place;
}
const tempId = state.env.nextIdentifierId;
const temporary = {
...place,
identifier: {
...place.identifier,
id: tempId,
name: null, // overwritten below
},
};
const temporary = createTemporaryPlace(state.env, place.loc);
promoteTemporary(temporary.identifier);
renamed.set(place, temporary);
return temporary;
Expand Down
55 changes: 43 additions & 12 deletions compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
import {CompilerError} from '../CompilerError';
import {GeneratedSource} from '../HIR';
import {
DeclarationId,
Identifier,
IdentifierId,
InstructionId,
Place,
PrunedReactiveScopeBlock,
Expand All @@ -24,7 +24,6 @@ import {ReactiveFunctionVisitor, visitReactiveFunction} from './visitors';

class Visitor extends ReactiveFunctionVisitor<State> {
override visitScope(scopeBlock: ReactiveScopeBlock, state: State): void {
this.traverseScope(scopeBlock, state);
for (const dep of scopeBlock.scope.dependencies) {
const {identifier} = dep;
if (identifier.name == null) {
Expand All @@ -43,21 +42,23 @@ class Visitor extends ReactiveFunctionVisitor<State> {
promoteIdentifier(declaration.identifier, state);
}
}
this.traverseScope(scopeBlock, state);
}

override visitPrunedScope(
scopeBlock: PrunedReactiveScopeBlock,
state: State,
): void {
this.traversePrunedScope(scopeBlock, state);
for (const [, declaration] of scopeBlock.scope.declarations) {
if (
declaration.identifier.name == null &&
state.pruned.get(declaration.identifier.id)?.usedOutsideScope === true
state.pruned.get(declaration.identifier.declarationId)
?.usedOutsideScope === true
) {
promoteIdentifier(declaration.identifier, state);
}
}
this.traversePrunedScope(scopeBlock, state);
}

override visitParam(place: Place, state: State): void {
Expand Down Expand Up @@ -93,11 +94,38 @@ class Visitor extends ReactiveFunctionVisitor<State> {
}
}

type JsxExpressionTags = Set<IdentifierId>;
class Visitor2 extends ReactiveFunctionVisitor<State> {
override visitPlace(_id: InstructionId, place: Place, state: State): void {
if (
place.identifier.name === null &&
state.promoted.has(place.identifier.declarationId)
) {
promoteIdentifier(place.identifier, state);
}
}
override visitLValue(
_id: InstructionId,
_lvalue: Place,
_state: State,
): void {
this.visitPlace(_id, _lvalue, _state);
}
override visitReactiveFunctionValue(
_id: InstructionId,
_dependencies: Array<Place>,
fn: ReactiveFunction,
state: State,
): void {
visitReactiveFunction(fn, this, state);
}
}

type JsxExpressionTags = Set<DeclarationId>;
type State = {
tags: JsxExpressionTags;
promoted: Set<DeclarationId>;
pruned: Map<
IdentifierId,
DeclarationId,
{activeScopes: Array<ScopeId>; usedOutsideScope: boolean}
>; // true if referenced within another scope, false if only accessed outside of scopes
};
Expand All @@ -108,9 +136,9 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor<State> {
override visitPlace(_id: InstructionId, place: Place, state: State): void {
if (
this.activeScopes.length !== 0 &&
state.pruned.has(place.identifier.id)
state.pruned.has(place.identifier.declarationId)
) {
const prunedPlace = state.pruned.get(place.identifier.id)!;
const prunedPlace = state.pruned.get(place.identifier.declarationId)!;
if (prunedPlace.activeScopes.indexOf(this.activeScopes.at(-1)!) === -1) {
prunedPlace.usedOutsideScope = true;
}
Expand All @@ -124,16 +152,16 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor<State> {
): void {
this.traverseValue(id, value, state);
if (value.kind === 'JsxExpression' && value.tag.kind === 'Identifier') {
state.tags.add(value.tag.identifier.id);
state.tags.add(value.tag.identifier.declarationId);
}
}

override visitPrunedScope(
scopeBlock: PrunedReactiveScopeBlock,
state: State,
): void {
for (const [id] of scopeBlock.scope.declarations) {
state.pruned.set(id, {
for (const [_id, decl] of scopeBlock.scope.declarations) {
state.pruned.set(decl.identifier.declarationId, {
activeScopes: [...this.activeScopes],
usedOutsideScope: false,
});
Expand All @@ -151,6 +179,7 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor<State> {
export function promoteUsedTemporaries(fn: ReactiveFunction): void {
const state: State = {
tags: new Set(),
promoted: new Set(),
pruned: new Map(),
};
visitReactiveFunction(fn, new CollectPromotableTemporaries(), state);
Expand All @@ -161,6 +190,7 @@ export function promoteUsedTemporaries(fn: ReactiveFunction): void {
}
}
visitReactiveFunction(fn, new Visitor(), state);
visitReactiveFunction(fn, new Visitor2(), state);
}

function promoteIdentifier(identifier: Identifier, state: State): void {
Expand All @@ -171,9 +201,10 @@ function promoteIdentifier(identifier: Identifier, state: State): void {
loc: GeneratedSource,
suggestions: null,
});
if (state.tags.has(identifier.id)) {
if (state.tags.has(identifier.declarationId)) {
promoteTemporaryJsxTag(identifier);
} else {
promoteTemporary(identifier);
}
state.promoted.add(identifier.declarationId);
}
Loading

0 comments on commit 656a7da

Please sign in to comment.