Skip to content

Commit 08d6ad7

Browse files
committed
[compiler][ez] Clean up duplicate code in propagateScopeDeps
Clean up duplicate checks for when to skip processing values as dependencies / hoistable temporaries.
1 parent 031230d commit 08d6ad7

File tree

1 file changed

+58
-24
lines changed

1 file changed

+58
-24
lines changed

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

Lines changed: 58 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,7 @@ class Context {
358358

359359
#temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>;
360360
#temporariesUsedOutsideScope: ReadonlySet<DeclarationId>;
361+
#processedInstrsInOptional: ReadonlySet<Instruction | Terminal>;
361362

362363
/**
363364
* Tracks the traversal state. See Context.declare for explanation of why this
@@ -368,9 +369,11 @@ class Context {
368369
constructor(
369370
temporariesUsedOutsideScope: ReadonlySet<DeclarationId>,
370371
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
372+
processedInstrsInOptional: ReadonlySet<Instruction | Terminal>,
371373
) {
372374
this.#temporariesUsedOutsideScope = temporariesUsedOutsideScope;
373375
this.#temporaries = temporaries;
376+
this.#processedInstrsInOptional = processedInstrsInOptional;
374377
}
375378

376379
enterScope(scope: ReactiveScope): void {
@@ -574,22 +577,49 @@ class Context {
574577
currentScope.reassignments.add(place.identifier);
575578
}
576579
}
580+
enterInnerFn<T>(cb: () => T): T {
581+
const wasInInnerFn = this.inInnerFn;
582+
this.inInnerFn = true;
583+
const result = cb();
584+
this.inInnerFn = wasInInnerFn;
585+
return result;
586+
}
587+
588+
/**
589+
* Skip dependencies that are subexpressions of other dependencies. e.g. if a
590+
* dependency is tracked in the temporaries sidemap, it can be added at
591+
* site-of-use
592+
*/
593+
isDeferredDependency(
594+
instr:
595+
| {kind: HIRValue.Instruction; value: Instruction}
596+
| {kind: HIRValue.Terminal; value: Terminal},
597+
): boolean {
598+
return (
599+
this.#processedInstrsInOptional.has(instr.value) ||
600+
(instr.kind === HIRValue.Instruction &&
601+
this.#temporaries.has(instr.value.lvalue.identifier.id))
602+
);
603+
}
604+
}
605+
enum HIRValue {
606+
Instruction = 1,
607+
Terminal,
577608
}
578609

579610
function handleInstruction(instr: Instruction, context: Context): void {
580611
const {id, value, lvalue} = instr;
581-
if (value.kind === 'LoadLocal') {
582-
if (
583-
value.place.identifier.name === null ||
584-
lvalue.identifier.name !== null ||
585-
context.isUsedOutsideDeclaringScope(lvalue)
586-
) {
587-
context.visitOperand(value.place);
588-
}
589-
} else if (value.kind === 'PropertyLoad') {
590-
if (context.isUsedOutsideDeclaringScope(lvalue)) {
591-
context.visitProperty(value.object, value.property, false);
592-
}
612+
context.declare(lvalue.identifier, {
613+
id,
614+
scope: context.currentScope,
615+
});
616+
if (
617+
context.isDeferredDependency({kind: HIRValue.Instruction, value: instr})
618+
) {
619+
return;
620+
}
621+
if (value.kind === 'PropertyLoad') {
622+
context.visitProperty(value.object, value.property, false);
593623
} else if (value.kind === 'StoreLocal') {
594624
context.visitOperand(value.value);
595625
if (value.lvalue.kind === InstructionKind.Reassign) {
@@ -632,11 +662,6 @@ function handleInstruction(instr: Instruction, context: Context): void {
632662
context.visitOperand(operand);
633663
}
634664
}
635-
636-
context.declare(lvalue.identifier, {
637-
id,
638-
scope: context.currentScope,
639-
});
640665
}
641666

642667
function collectDependencies(
@@ -645,7 +670,11 @@ function collectDependencies(
645670
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
646671
processedInstrsInOptional: ReadonlySet<Instruction | Terminal>,
647672
): Map<ReactiveScope, Array<ReactiveScopeDependency>> {
648-
const context = new Context(usedOutsideDeclaringScope, temporaries);
673+
const context = new Context(
674+
usedOutsideDeclaringScope,
675+
temporaries,
676+
processedInstrsInOptional,
677+
);
649678

650679
for (const param of fn.params) {
651680
if (param.kind === 'Identifier') {
@@ -694,16 +723,21 @@ function collectDependencies(
694723
/**
695724
* Recursively visit the inner function to extract dependencies there
696725
*/
697-
const wasInInnerFn = context.inInnerFn;
698-
context.inInnerFn = true;
699-
handleFunction(instr.value.loweredFunc.func);
700-
context.inInnerFn = wasInInnerFn;
701-
} else if (!processedInstrsInOptional.has(instr)) {
726+
const innerFn = instr.value.loweredFunc.func;
727+
context.enterInnerFn(() => {
728+
handleFunction(innerFn);
729+
});
730+
} else {
702731
handleInstruction(instr, context);
703732
}
704733
}
705734

706-
if (!processedInstrsInOptional.has(block.terminal)) {
735+
if (
736+
!context.isDeferredDependency({
737+
kind: HIRValue.Terminal,
738+
value: block.terminal,
739+
})
740+
) {
707741
for (const place of eachTerminalOperand(block.terminal)) {
708742
context.visitOperand(place);
709743
}

0 commit comments

Comments
 (0)