diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts index 7c1fb54ea8058..961e0312ef5c6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts @@ -184,6 +184,7 @@ type TerminalRewriteInfo = | { kind: 'StartScope'; blockId: BlockId; + dependencyId: BlockId; fallthroughId: BlockId; instrId: InstructionId; scope: ReactiveScope; @@ -208,12 +209,14 @@ function pushStartScopeTerminal( scope: ReactiveScope, context: ScopeTraversalContext, ): void { + const dependencyId = context.env.nextBlockId; const blockId = context.env.nextBlockId; const fallthroughId = context.env.nextBlockId; context.rewrites.push({ kind: 'StartScope', blockId, fallthroughId, + dependencyId, instrId: scope.range.start, scope, }); @@ -255,10 +258,13 @@ type RewriteContext = { * instr1, instr2, instr3, instr4, [[ original terminal ]] * Rewritten: * bb0: - * instr1, [[ scope start block=bb1]] + * instr1, [[ scope start dependencies=bb1 block=bb2]] * bb1: - * instr2, instr3, [[ scope end goto=bb2 ]] + * [[ empty, filled in in PropagateScopeDependenciesHIR ]] + * goto bb2 * bb2: + * instr2, instr3, [[ scope end goto=bb3 ]] + * bb3: * instr4, [[ original terminal ]] */ function handleRewrite( @@ -272,6 +278,7 @@ function handleRewrite( ? { kind: 'scope', fallthrough: terminalInfo.fallthroughId, + dependencies: terminalInfo.dependencyId, block: terminalInfo.blockId, scope: terminalInfo.scope, id: terminalInfo.instrId, @@ -298,7 +305,28 @@ function handleRewrite( context.nextPreds = new Set([currBlockId]); context.nextBlockId = terminalInfo.kind === 'StartScope' - ? terminalInfo.blockId + ? terminalInfo.dependencyId : terminalInfo.fallthroughId; context.instrSliceIdx = idx; + + if (terminalInfo.kind === 'StartScope') { + const currBlockId = context.nextBlockId; + context.rewrites.push({ + kind: context.source.kind, + id: currBlockId, + instructions: [], + preds: context.nextPreds, + // Only the first rewrite should reuse source block phis + phis: new Set(), + terminal: { + kind: 'goto', + variant: GotoVariant.Break, + block: terminal.block, + id: terminalInfo.instrId, + loc: GeneratedSource, + }, + }); + context.nextPreds = new Set([currBlockId]); + context.nextBlockId = terminalInfo.blockId; + } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts index a422570fffead..7d35d9e3c62ac 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -194,7 +194,10 @@ type PropertyPathNode = class PropertyPathRegistry { roots: Map = new Map(); - getOrCreateIdentifier(identifier: Identifier): PropertyPathNode { + getOrCreateIdentifier( + identifier: Identifier, + reactive: boolean, + ): PropertyPathNode { /** * Reads from a statically scoped variable are always safe in JS, * with the exception of TDZ (not addressed by this pass). @@ -208,12 +211,19 @@ class PropertyPathRegistry { optionalProperties: new Map(), fullPath: { identifier, + reactive, path: [], }, hasOptional: false, parent: null, }; this.roots.set(identifier.id, rootNode); + } else { + CompilerError.invariant(reactive === rootNode.fullPath.reactive, { + reason: + '[HoistablePropertyLoads] Found inconsistencies in `reactive` flag after ', + loc: GeneratedSource, + }); } return rootNode; } @@ -231,6 +241,7 @@ class PropertyPathRegistry { parent: parent, fullPath: { identifier: parent.fullPath.identifier, + reactive: parent.fullPath.reactive, path: parent.fullPath.path.concat(entry), }, hasOptional: parent.hasOptional || entry.optional, @@ -246,7 +257,7 @@ class PropertyPathRegistry { * so all subpaths of a PropertyLoad should already exist * (e.g. a.b is added before a.b.c), */ - let currNode = this.getOrCreateIdentifier(n.identifier); + let currNode = this.getOrCreateIdentifier(n.identifier, n.reactive); if (n.path.length === 0) { return currNode; } @@ -268,10 +279,11 @@ function getMaybeNonNullInInstruction( instr: InstructionValue, context: CollectHoistablePropertyLoadsContext, ): PropertyPathNode | null { - let path = null; + let path: ReactiveScopeDependency | null = null; if (instr.kind === 'PropertyLoad') { path = context.temporaries.get(instr.object.identifier.id) ?? { identifier: instr.object.identifier, + reactive: instr.object.reactive, path: [], }; } else if (instr.kind === 'Destructure') { @@ -334,7 +346,7 @@ function collectNonNullsInBlocks( ) { const identifier = fn.params[0].identifier; knownNonNullIdentifiers.add( - context.registry.getOrCreateIdentifier(identifier), + context.registry.getOrCreateIdentifier(identifier, true), ); } const nodes = new Map(); @@ -565,9 +577,11 @@ function reduceMaybeOptionalChains( changed = false; for (const original of optionalChainNodes) { - let {identifier, path: origPath} = original.fullPath; - let currNode: PropertyPathNode = - registry.getOrCreateIdentifier(identifier); + let {identifier, path: origPath, reactive} = original.fullPath; + let currNode: PropertyPathNode = registry.getOrCreateIdentifier( + identifier, + reactive, + ); for (let i = 0; i < origPath.length; i++) { const entry = origPath[i]; // If the base is known to be non-null, replace with a non-optional load diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts index 2b7c9f2134e71..e4a7d14570a93 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts @@ -106,7 +106,7 @@ export type OptionalChainSidemap = { hoistableObjects: ReadonlyMap; }; -type OptionalTraversalContext = { +export type OptionalTraversalContext = { currFn: HIRFunction; blocks: ReadonlyMap; @@ -227,7 +227,7 @@ function matchOptionalTestBlock( * property loads. If any part of the optional chain is not hoistable, returns * null. */ -function traverseOptionalBlock( +export function traverseOptionalBlock( optional: TBasicBlock, context: OptionalTraversalContext, outerAlternate: BlockId | null, @@ -282,6 +282,7 @@ function traverseOptionalBlock( ); baseObject = { identifier: maybeTest.instructions[0].value.place.identifier, + reactive: maybeTest.instructions[0].value.place.reactive, path, }; test = maybeTest.terminal; @@ -383,6 +384,7 @@ function traverseOptionalBlock( ); const load = { identifier: baseObject.identifier, + reactive: baseObject.reactive, path: [ ...baseObject.path, { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts index 07f44a59a26de..b83cf9677cb99 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts @@ -24,8 +24,9 @@ export class ReactiveScopeDependencyTreeHIR { * `identifier.path`, or `identifier?.path` is in this map, it is safe to * evaluate (non-optional) PropertyLoads from. */ - #hoistableObjects: Map = new Map(); - #deps: Map = new Map(); + #hoistableObjects: Map = + new Map(); + #deps: Map = new Map(); /** * @param hoistableObjects a set of paths from which we can safely evaluate @@ -34,9 +35,10 @@ export class ReactiveScopeDependencyTreeHIR { * duplicates when traversing the CFG. */ constructor(hoistableObjects: Iterable) { - for (const {path, identifier} of hoistableObjects) { + for (const {path, identifier, reactive} of hoistableObjects) { let currNode = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot( identifier, + reactive, this.#hoistableObjects, path.length > 0 && path[0].optional ? 'Optional' : 'NonNull', ); @@ -69,7 +71,8 @@ export class ReactiveScopeDependencyTreeHIR { static #getOrCreateRoot( identifier: Identifier, - roots: Map>, + reactive: boolean, + roots: Map & {reactive: boolean}>, defaultAccessType: T, ): TreeNode { // roots can always be accessed unconditionally in JS @@ -78,9 +81,16 @@ export class ReactiveScopeDependencyTreeHIR { if (rootNode === undefined) { rootNode = { properties: new Map(), + reactive, accessType: defaultAccessType, }; roots.set(identifier, rootNode); + } else { + CompilerError.invariant(reactive === rootNode.reactive, { + reason: '[DeriveMinimalDependenciesHIR] Conflicting reactive root flag', + description: `Identifier ${printIdentifier(identifier)}`, + loc: GeneratedSource, + }); } return rootNode; } @@ -91,9 +101,10 @@ export class ReactiveScopeDependencyTreeHIR { * safe-to-evaluate subpath */ addDependency(dep: ReactiveScopeDependency): void { - const {identifier, path} = dep; + const {identifier, reactive, path} = dep; let depCursor = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot( identifier, + reactive, this.#deps, PropertyAccessType.UnconditionalAccess, ); @@ -171,7 +182,13 @@ export class ReactiveScopeDependencyTreeHIR { deriveMinimalDependencies(): Set { const results = new Set(); for (const [rootId, rootNode] of this.#deps.entries()) { - collectMinimalDependenciesInSubtree(rootNode, rootId, [], results); + collectMinimalDependenciesInSubtree( + rootNode, + rootNode.reactive, + rootId, + [], + results, + ); } return results; @@ -293,25 +310,24 @@ type HoistableNode = TreeNode<'Optional' | 'NonNull'>; type DependencyNode = TreeNode; /** - * TODO: this is directly pasted from DeriveMinimalDependencies. Since we no - * longer have conditionally accessed nodes, we can simplify - * * Recursively calculates minimal dependencies in a subtree. * @param node DependencyNode representing a dependency subtree. * @returns a minimal list of dependencies in this subtree. */ function collectMinimalDependenciesInSubtree( node: DependencyNode, + reactive: boolean, rootIdentifier: Identifier, path: Array, results: Set, ): void { if (isDependency(node.accessType)) { - results.add({identifier: rootIdentifier, path}); + results.add({identifier: rootIdentifier, reactive, path}); } else { for (const [childName, childNode] of node.properties) { collectMinimalDependenciesInSubtree( childNode, + reactive, rootIdentifier, [ ...path, diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index e124caee7a23f..fb7e84b11604a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -62,12 +62,14 @@ export type ReactiveFunction = { export type ReactiveScopeBlock = { kind: 'scope'; + dependencyInstructions: Array; scope: ReactiveScope; instructions: ReactiveBlock; }; export type PrunedReactiveScopeBlock = { kind: 'pruned-scope'; + dependencyInstructions: Array; scope: ReactiveScope; instructions: ReactiveBlock; }; @@ -614,6 +616,7 @@ export type MaybeThrowTerminal = { export type ReactiveScopeTerminal = { kind: 'scope'; fallthrough: BlockId; + dependencies: BlockId; block: BlockId; scope: ReactiveScope; id: InstructionId; @@ -623,6 +626,7 @@ export type ReactiveScopeTerminal = { export type PrunedScopeTerminal = { kind: 'pruned-scope'; fallthrough: BlockId; + dependencies: BlockId; block: BlockId; scope: ReactiveScope; id: InstructionId; @@ -1453,9 +1457,10 @@ export type ReactiveScope = { range: MutableRange; /** - * The inputs to this reactive scope + * Note the dependencies of a reactive scope are tracked in HIR and + * ReactiveFunction */ - dependencies: ReactiveScopeDependencies; + dependencies: Array; /** * The set of values produced by this scope. This may be empty @@ -1506,6 +1511,18 @@ export type DependencyPathEntry = {property: string; optional: boolean}; export type DependencyPath = Array; export type ReactiveScopeDependency = { identifier: Identifier; + /** + * Reflects whether the base identifier is reactive. Note that some reactive + * objects may have non-reactive properties, but we do not currently track + * this. + * + * ```js + * // Technically, result[0] is reactive and result[1] is not. + * // Currently, both dependencies would be marked as reactive. + * const result = useState(); + * ``` + */ + reactive: boolean; path: DependencyPath; }; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index c3b46c6ad3475..3e9dec30e5462 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -6,7 +6,6 @@ */ import generate from '@babel/generator'; -import {printReactiveFunction} from '..'; import {CompilerError} from '../CompilerError'; import {printReactiveScopeSummary} from '../ReactiveScopes/PrintReactiveFunction'; import DisjointSet from '../Utils/DisjointSet'; @@ -287,13 +286,13 @@ export function printTerminal(terminal: Terminal): Array | string { case 'scope': { value = `[${terminal.id}] Scope ${printReactiveScopeSummary( terminal.scope, - )} block=bb${terminal.block} fallthrough=bb${terminal.fallthrough}`; + )} dependencies=bb${terminal.dependencies} block=bb${terminal.block} fallthrough=bb${terminal.fallthrough}`; break; } case 'pruned-scope': { value = `[${terminal.id}] Scope ${printReactiveScopeSummary( terminal.scope, - )} block=bb${terminal.block} fallthrough=bb${terminal.fallthrough}`; + )} dependencies=bb${terminal.dependencies} block=bb${terminal.block} fallthrough=bb${terminal.fallthrough}`; break; } case 'try': { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index 4cb84870a8a33..13926adbbd7ba 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -22,6 +22,18 @@ import { TInstruction, FunctionExpression, ObjectMethod, + BlockId, + makeTemporaryIdentifier, + Effect, + OptionalTerminal, + TBasicBlock, + ReactiveScopeTerminal, + GotoVariant, + PrunedScopeTerminal, + ReactiveInstruction, + ReactiveValue, + ReactiveScopeBlock, + PrunedReactiveScopeBlock, } from './HIR'; import { collectHoistablePropertyLoads, @@ -38,7 +50,21 @@ import {Stack, empty} from '../Utils/Stack'; import {CompilerError} from '../CompilerError'; import {Iterable_some} from '../Utils/utils'; import {ReactiveScopeDependencyTreeHIR} from './DeriveMinimalDependenciesHIR'; -import {collectOptionalChainSidemap} from './CollectOptionalChainDependencies'; +import { + collectOptionalChainSidemap, + OptionalTraversalContext, + traverseOptionalBlock, +} from './CollectOptionalChainDependencies'; +import {Environment} from './Environment'; +import HIRBuilder, { + fixScopeAndIdentifierRanges, + markInstructionIds, + markPredecessors, + reversePostorderBlocks, +} from './HIRBuilder'; +import {lowerValueToTemporary} from './BuildHIR'; +import {printDependency} from '../ReactiveScopes/PrintReactiveFunction'; +import {printPlace} from './PrintHIR'; export function propagateScopeDependenciesHIR(fn: HIRFunction): void { const usedOutsideDeclaringScope = @@ -65,8 +91,10 @@ export function propagateScopeDependenciesHIR(fn: HIRFunction): void { /** * Derive the minimal set of hoistable dependencies for each scope. */ + const minimalDeps = new Map>(); for (const [scope, deps] of scopeDeps) { if (deps.length === 0) { + minimalDeps.set(scope, new Set()); continue; } @@ -93,19 +121,616 @@ export function propagateScopeDependenciesHIR(fn: HIRFunction): void { * Step 3: Reduce dependencies to a minimal set. */ const candidates = tree.deriveMinimalDependencies(); + const dependencies = new Set(); for (const candidateDep of candidates) { if ( !Iterable_some( - scope.dependencies, + dependencies, existingDep => existingDep.identifier.declarationId === candidateDep.identifier.declarationId && areEqualPaths(existingDep.path, candidateDep.path), ) ) - scope.dependencies.add(candidateDep); + dependencies.add(candidateDep); + } + minimalDeps.set(scope, dependencies); + } + + let changed = false; + /** + * Step 4: inject dependencies + */ + for (const [_, {terminal}] of fn.body.blocks) { + if (terminal.kind !== 'scope' && terminal.kind !== 'pruned-scope') { + continue; + } + const scope = terminal.scope; + const deps = minimalDeps.get(scope); + if (deps == null || deps.size === 0) { + continue; + } + writeScopeDependencies(terminal, deps, fn); + changed = true; + } + + if (changed) { + /** + * Step 5: + * Fix scope and identifier ranges to account for renumbered instructions + */ + reversePostorderBlocks(fn.body); + markPredecessors(fn.body); + markInstructionIds(fn.body); + + fixScopeAndIdentifierRanges(fn.body); + } + + // Sanity check + { + for (const [scope, deps] of minimalDeps) { + const checkedDeps = readScopeDependencies(fn, scope.id); + CompilerError.invariant(checkedDeps != null, { + reason: '[Rewrite] Cannot find scope dep when reading', + loc: scope.loc, + }); + CompilerError.invariant(checkedDeps.size === deps.size, { + reason: '[Rewrite] non matching sizes when reading', + description: `scopeId=${scope.id} deps=${[...deps].map(printDependency)} checkedDeps=${[...checkedDeps].map(printDependency)}`, + loc: scope.loc, + }); + label: for (const dep of deps) { + for (const checkedDep of checkedDeps) { + if ( + dep.identifier === checkedDep.identifier && + areEqualPaths(dep.path, checkedDep.path) + ) { + continue label; + } + } + CompilerError.invariant(false, { + reason: + '[Rewrite] could not find match for dependency when re-reading', + description: `${printDependency(dep)}`, + loc: scope.loc, + }); + } + } + } +} + +function writeNonOptionalDependency( + dep: ReactiveScopeDependency, + env: Environment, + builder: HIRBuilder, +): Identifier { + const loc = dep.identifier.loc; + let last: Identifier = makeTemporaryIdentifier(env.nextIdentifierId, loc); + builder.push({ + lvalue: { + identifier: last, + kind: 'Identifier', + effect: Effect.Mutate, + reactive: dep.reactive, + loc, + }, + value: { + kind: 'LoadLocal', + place: { + identifier: dep.identifier, + kind: 'Identifier', + effect: Effect.Freeze, + reactive: dep.reactive, + loc, + }, + loc, + }, + id: makeInstructionId(1), + loc: loc, + }); + + for (const path of dep.path) { + const next = makeTemporaryIdentifier(env.nextIdentifierId, loc); + builder.push({ + lvalue: { + identifier: next, + kind: 'Identifier', + effect: Effect.Mutate, + reactive: dep.reactive, + loc, + }, + value: { + kind: 'PropertyLoad', + object: { + identifier: last, + kind: 'Identifier', + effect: Effect.Freeze, + reactive: dep.reactive, + loc, + }, + property: path.property, + loc, + }, + id: makeInstructionId(1), + loc: loc, + }); + last = next; + } + return last; +} + +function writeScopeDependencies( + terminal: ReactiveScopeTerminal | PrunedScopeTerminal, + deps: Set, + fn: HIRFunction, +): void { + const scopeDepBlock = fn.body.blocks.get(terminal.dependencies); + + CompilerError.invariant(scopeDepBlock != null, { + reason: 'Expected nonnull scopeDepBlock', + loc: terminal.loc, + }); + CompilerError.invariant( + scopeDepBlock.instructions.length === 0 && + scopeDepBlock.terminal.kind === 'goto' && + scopeDepBlock.terminal.block === terminal.block, + { + reason: 'Expected scope.dependencies to be a goto block', + loc: terminal.loc, + }, + ); + const builder = new HIRBuilder(fn.env, { + entryBlockKind: 'value', + }); + + for (const dep of deps) { + if (dep.path.every(path => !path.optional)) { + const last = writeNonOptionalDependency(dep, fn.env, builder); + terminal.scope.dependencies.push({ + kind: 'Identifier', + identifier: last, + effect: Effect.Freeze, + reactive: dep.reactive, + loc: GeneratedSource, + }); + } + } + + // Write all optional chaining deps + for (const dep of deps) { + if (!dep.path.every(path => !path.optional)) { + const last = writeOptional( + dep.path.length - 1, + dep, + builder, + terminal, + null, + ); + terminal.scope.dependencies.push({ + kind: 'Identifier', + identifier: last, + effect: Effect.Freeze, + reactive: dep.reactive, + loc: GeneratedSource, + }); + } + } + + // Placeholder terminal for HIRBuilder (goto leads to block outside of that nested HIR) + const lastBlockId = builder.terminate( + { + kind: 'return', + value: { + kind: 'Identifier', + identifier: makeTemporaryIdentifier( + fn.env.nextIdentifierId, + GeneratedSource, + ), + effect: Effect.Freeze, + loc: GeneratedSource, + reactive: true, + }, + loc: GeneratedSource, + id: makeInstructionId(0), + }, + null, + ); + + const dependenciesHIR = builder.build(); + for (const [id, block] of dependenciesHIR.blocks) { + fn.body.blocks.set(id, block); + } + // Shouldn't be needed with RPO / removeUnreachableBlocks + fn.body.blocks.delete(terminal.dependencies); + terminal.dependencies = dependenciesHIR.entry; + fn.body.blocks.get(lastBlockId)!.terminal = scopeDepBlock.terminal; +} + +function writeOptional( + idx: number, + dep: ReactiveScopeDependency, + builder: HIRBuilder, + terminal: ReactiveScopeTerminal | PrunedScopeTerminal, + parentAlternate: BlockId | null, +): Identifier { + const env = builder.environment; + CompilerError.invariant( + idx >= 0 && !dep.path.slice(0, idx + 1).every(path => !path.optional), + { + reason: '[WriteOptional] Expected optional path', + description: `${idx} ${printDependency(dep)}`, + loc: GeneratedSource, + }, + ); + const continuationBlock = builder.reserve(builder.currentBlockKind()); + const consequent = builder.reserve('value'); + + const returnPlace: Place = { + kind: 'Identifier', + identifier: makeTemporaryIdentifier(env.nextIdentifierId, GeneratedSource), + effect: Effect.Mutate, + reactive: dep.reactive, + loc: GeneratedSource, + }; + + let alternate; + if (parentAlternate != null) { + alternate = parentAlternate; + } else { + /** + * Make outermost alternate block + * $N = Primitive undefined + * $M = StoreLocal $OptionalResult = $N + * goto fallthrough + */ + alternate = builder.enter('value', () => { + const temp = lowerValueToTemporary(builder, { + kind: 'Primitive', + value: undefined, + loc: GeneratedSource, + }); + lowerValueToTemporary(builder, { + kind: 'StoreLocal', + lvalue: {kind: InstructionKind.Const, place: {...returnPlace}}, + value: {...temp}, + type: null, + loc: GeneratedSource, + }); + return { + kind: 'goto', + variant: GotoVariant.Break, + block: continuationBlock.id, + id: makeInstructionId(0), + loc: GeneratedSource, + }; + }); + } + + let testIdentifier: Identifier | null = null; + const testBlock = builder.enter('value', () => { + const firstOptional = dep.path.findIndex(path => path.optional); + if (idx === firstOptional) { + // Lower test block + testIdentifier = writeNonOptionalDependency( + { + identifier: dep.identifier, + reactive: dep.reactive, + path: dep.path.slice(0, idx), + }, + env, + builder, + ); + } else { + testIdentifier = writeOptional( + idx - 1, + dep, + builder, + terminal, + alternate, + ); + } + + return { + kind: 'branch', + test: { + identifier: testIdentifier, + effect: Effect.Freeze, + kind: 'Identifier', + loc: GeneratedSource, + reactive: dep.reactive, + }, + consequent: consequent.id, + alternate, + id: makeInstructionId(0), + loc: GeneratedSource, + fallthrough: continuationBlock.id, + }; + }); + + CompilerError.invariant(testIdentifier !== null, { + reason: 'Satisfy type checker', + description: null, + loc: null, + suggestions: null, + }); + + builder.enterReserved(consequent, () => { + CompilerError.invariant(testIdentifier !== null, { + reason: 'Satisfy type checker', + description: null, + loc: null, + suggestions: null, + }); + + const tmpConsequent = lowerValueToTemporary(builder, { + kind: 'PropertyLoad', + object: { + identifier: testIdentifier, + kind: 'Identifier', + effect: Effect.Freeze, + reactive: dep.reactive, + loc: GeneratedSource, + }, + property: dep.path[idx].property, + loc: GeneratedSource, + }); + lowerValueToTemporary(builder, { + kind: 'StoreLocal', + lvalue: {kind: InstructionKind.Const, place: {...returnPlace}}, + value: {...tmpConsequent}, + type: null, + loc: GeneratedSource, + }); + return { + kind: 'goto', + variant: GotoVariant.Break, + block: continuationBlock.id, + id: makeInstructionId(0), + loc: GeneratedSource, + }; + }); + builder.terminateWithContinuation( + { + kind: 'optional', + optional: dep.path[idx].optional, + test: testBlock, + fallthrough: continuationBlock.id, + id: makeInstructionId(0), + loc: GeneratedSource, + }, + continuationBlock, + ); + + return returnPlace.identifier; +} + +export function readScopeDependencies( + fn: HIRFunction, + scope: ScopeId, +): Set { + for (const [_, {terminal}] of fn.body.blocks) { + if (terminal.kind !== 'scope' && terminal.kind !== 'pruned-scope') { + continue; + } + if (terminal.scope.id !== scope) { + continue; } + const temporaries = new Map(); + const context: OptionalTraversalContext = { + currFn: fn, + blocks: fn.body.blocks, + seenOptionals: new Set(), + processedInstrsInOptional: new Set(), + temporariesReadInOptional: temporaries, + hoistableObjects: new Map(), + }; + /** + * read all dependencies between scope and block + */ + let work = terminal.dependencies; + while (true) { + const block = fn.body.blocks.get(work)!; + for (const {lvalue, value} of block.instructions) { + if (value.kind === 'LoadLocal') { + temporaries.set(lvalue.identifier.id, { + identifier: value.place.identifier, + reactive: value.place.reactive, + path: [], + }); + } else if (value.kind === 'PropertyLoad') { + const source = temporaries.get(value.object.identifier.id)!; + temporaries.set(lvalue.identifier.id, { + identifier: source.identifier, + reactive: source.reactive, + path: [...source.path, {property: value.property, optional: false}], + }); + } + } + + if (block.terminal.kind === 'optional') { + traverseOptionalBlock( + block as TBasicBlock, + context, + null, + ); + work = block.terminal.fallthrough; + } else { + CompilerError.invariant( + block.terminal.kind === 'goto' && + block.terminal.block === terminal.block, + { + reason: 'unexpected terminal', + description: `kind: ${block.terminal.kind}`, + loc: block.terminal.loc, + }, + ); + break; + } + } + + const scopeOwnDependencies = new Set(); + for (const dep of terminal.scope.dependencies) { + const reactiveScopeDependency = temporaries.get(dep.identifier.id)!; + CompilerError.invariant(reactiveScopeDependency != null, { + reason: 'Expected dependency to be found', + description: `${printPlace(dep)}`, + loc: terminal.scope.loc, + }); + scopeOwnDependencies.add(reactiveScopeDependency); + } + return scopeOwnDependencies; + } + CompilerError.invariant(false, { + reason: 'Expected scope to be found', + loc: GeneratedSource, + }); +} + +function assertNonNull(value: T | null | undefined): T { + if (value == null) { + throw new Error('Expected nonnull value'); } + return value; +} + +function _helperInstr( + instr: ReactiveInstruction, + sidemap: Map, +): void { + const value = _helperValue(instr.value, sidemap); + if (instr.lvalue != null) { + sidemap.set(instr.lvalue.identifier.id, value); + } +} +function _helperValue( + instr: ReactiveValue, + sidemap: Map, +): ReactiveScopeDependency { + if (instr.kind === 'LoadLocal') { + const base = sidemap.get(instr.place.identifier.id); + if (base != null) { + return base; + } else { + return { + identifier: instr.place.identifier, + reactive: instr.place.reactive, + path: [], + }; + } + } else if (instr.kind === 'PropertyLoad') { + const base = assertNonNull(sidemap.get(instr.object.identifier.id)); + return { + identifier: base.identifier, + reactive: base.reactive, + path: [...base.path, {property: instr.property, optional: false}], + }; + } else if (instr.kind === 'SequenceExpression') { + for (const inner of instr.instructions) { + _helperInstr(inner, sidemap); + } + return _helperValue(instr.value, sidemap); + } else if (instr.kind === 'OptionalExpression') { + const value = _helperValue(instr.value, sidemap); + CompilerError.invariant( + value.path.length > 0 && !value.path.at(-1)!.optional, + { + reason: 'Expected optional chain to be nonempty', + loc: instr.loc, + }, + ); + return { + ...value, + path: [ + ...value.path.slice(0, -1), + {property: value.path.at(-1)!.property, optional: instr.optional}, + ], + }; + } + CompilerError.invariant(false, { + reason: 'Unexpected value kind', + description: instr.kind, + loc: instr.loc, + }); +} + +export function readScopeDependenciesRHIR( + scopeBlock: ReactiveScopeBlock | PrunedReactiveScopeBlock, +): Map { + const sidemap = new Map(); + for (const instr of scopeBlock.dependencyInstructions) { + _helperInstr(instr.instruction, sidemap); + } + return new Map( + scopeBlock.scope.dependencies.map(place => { + return [place, assertNonNull(sidemap.get(place.identifier.id))]; + }), + ); +} + +/** + * Note: this only handles simple pruning i.e. deleting dependency entries, + * not more complex rewrites such as truncating property paths. + */ +export function scopeDependenciesDCE( + scopeBlock: ReactiveScopeBlock | PrunedReactiveScopeBlock, +): void { + const usage = new Map(); + for (const { + instruction: {lvalue, value}, + } of scopeBlock.dependencyInstructions) { + if (lvalue == null) { + continue; + } + switch (value.kind) { + case 'LoadLocal': { + usage.set(lvalue.identifier.id, null); + break; + } + case 'PropertyLoad': { + usage.set(lvalue.identifier.id, value.object.identifier.id); + break; + } + case 'OptionalExpression': { + usage.set(lvalue.identifier.id, null); + break; + } + default: { + CompilerError.invariant(false, { + reason: 'Unexpected value kind', + description: value.kind, + loc: value.loc, + }); + } + } + } + + const notUsed = new Set(usage.keys()); + const seen = new Set(); + for (const {identifier} of scopeBlock.scope.dependencies) { + let curr: IdentifierId | undefined | null = identifier.id; + while (curr != null) { + CompilerError.invariant(!seen.has(curr), { + reason: 'infinite loop', + loc: GeneratedSource, + }); + notUsed.delete(curr); + seen.add(curr); + curr = usage.get(curr); + } + } + /** + * Remove unused instructions in place + */ + let j = 0; + for (let i = 0; i < scopeBlock.dependencyInstructions.length; i++) { + const instr = scopeBlock.dependencyInstructions[i].instruction; + if (instr.lvalue != null && !notUsed.has(instr.lvalue.identifier.id)) { + scopeBlock.dependencyInstructions[j] = + scopeBlock.dependencyInstructions[i]; + j++; + } + } + scopeBlock.dependencyInstructions.length = j; } function findTemporariesUsedOutsideDeclaringScope( @@ -301,6 +926,7 @@ function collectTemporariesSidemapImpl( ) { temporaries.set(lvalue.identifier.id, { identifier: value.place.identifier, + reactive: value.place.reactive, path: [], }); } @@ -354,11 +980,13 @@ function getProperty( if (resolvedDependency == null) { property = { identifier: object.identifier, + reactive: object.reactive, path: [{property: propertyName, optional}], }; } else { property = { identifier: resolvedDependency.identifier, + reactive: resolvedDependency.reactive, path: [...resolvedDependency.path, {property: propertyName, optional}], }; } @@ -514,6 +1142,7 @@ class Context { this.visitDependency( this.#temporaries.get(place.identifier.id) ?? { identifier: place.identifier, + reactive: place.reactive, path: [], }, ); @@ -574,6 +1203,7 @@ class Context { ) { maybeDependency = { identifier: maybeDependency.identifier, + reactive: maybeDependency.reactive, path: [], }; } @@ -595,7 +1225,11 @@ class Context { identifier => identifier.declarationId === place.identifier.declarationId, ) && - this.#checkValidDependency({identifier: place.identifier, path: []}) + this.#checkValidDependency({ + identifier: place.identifier, + reactive: place.reactive, + path: [], + }) ) { currentScope.reassignments.add(place.identifier); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts index 49ff3c256e016..3247771fb2f52 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/visitors.ts @@ -860,11 +860,13 @@ export function mapTerminalSuccessors( } case 'scope': case 'pruned-scope': { + const dependencies = fn(terminal.dependencies); const block = fn(terminal.block); const fallthrough = fn(terminal.fallthrough); return { kind: terminal.kind, scope: terminal.scope, + dependencies, block, fallthrough, id: makeInstructionId(0), @@ -1017,7 +1019,7 @@ export function* eachTerminalSuccessor(terminal: Terminal): Iterable { } case 'scope': case 'pruned-scope': { - yield terminal.block; + yield terminal.dependencies; break; } case 'unreachable': @@ -1068,6 +1070,13 @@ export function mapTerminalOperands( } break; } + case 'scope': + case 'pruned-scope': { + for (let i = 0; i < terminal.scope.dependencies.length; i++) { + terminal.scope.dependencies[i] = fn(terminal.scope.dependencies[i]); + } + break; + } case 'maybe-throw': case 'sequence': case 'label': @@ -1081,9 +1090,7 @@ export function mapTerminalOperands( case 'for-in': case 'goto': case 'unreachable': - case 'unsupported': - case 'scope': - case 'pruned-scope': { + case 'unsupported': { // no-op break; } @@ -1127,6 +1134,13 @@ export function* eachTerminalOperand(terminal: Terminal): Iterable { } break; } + case 'scope': + case 'pruned-scope': { + for (const dep of terminal.scope.dependencies) { + yield dep; + } + break; + } case 'maybe-throw': case 'sequence': case 'label': @@ -1140,9 +1154,7 @@ export function* eachTerminalOperand(terminal: Terminal): Iterable { case 'for-in': case 'goto': case 'unreachable': - case 'unsupported': - case 'scope': - case 'pruned-scope': { + case 'unsupported': { // no-op break; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts index 8c9823765bbb6..07518059bb477 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts @@ -22,6 +22,7 @@ import { fixScopeAndIdentifierRanges, markInstructionIds, } from '../HIR/HIRBuilder'; +import {readScopeDependencies} from '../HIR/PropagateScopeDependenciesHIR'; import {eachInstructionOperand, eachTerminalOperand} from '../HIR/visitors'; import {getOrInsertWith} from '../Utils/utils'; @@ -71,10 +72,11 @@ export function inferEffectDependencies(fn: HIRFunction): void { block.terminal.kind === 'scope' || block.terminal.kind === 'pruned-scope' ) { + // TODO: check const scopeBlock = fn.body.blocks.get(block.terminal.block)!; scopeInfos.set(block.terminal.scope.id, { pruned: block.terminal.kind === 'pruned-scope', - deps: block.terminal.scope.dependencies, + deps: readScopeDependencies(fn, block.terminal.scope.id), hasSingleInstr: scopeBlock.instructions.length === 1 && scopeBlock.terminal.kind === 'goto' && diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts index 344949b020a99..19361e195a73b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts @@ -21,6 +21,7 @@ import { import {PostDominator} from '../HIR/Dominator'; import { eachInstructionLValue, + eachInstructionOperand, eachInstructionValueOperand, eachTerminalOperand, } from '../HIR/visitors'; @@ -188,7 +189,7 @@ export function inferReactivePlaces(fn: HIRFunction): void { let hasReactiveInput = false; /* * NOTE: we want to mark all operands as reactive or not, so we - * avoid short-circuting here + * avoid short-circuiting here */ for (const operand of eachInstructionValueOperand(value)) { const reactive = reactiveIdentifiers.isReactive(operand); @@ -264,6 +265,36 @@ export function inferReactivePlaces(fn: HIRFunction): void { } } } while (reactiveIdentifiers.snapshot()); + + function propagateReactivityToInnerFunctions( + fn: HIRFunction, + isOutermost: boolean, + ): void { + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + if (!isOutermost) { + for (const operand of eachInstructionOperand(instr)) { + reactiveIdentifiers.isReactive(operand); + } + } + if ( + instr.value.kind === 'ObjectMethod' || + instr.value.kind === 'FunctionExpression' + ) { + propagateReactivityToInnerFunctions( + instr.value.loweredFunc.func, + false, + ); + } + } + if (!isOutermost) { + for (const operand of eachTerminalOperand(block.terminal)) { + reactiveIdentifiers.isReactive(operand); + } + } + } + } + propagateReactivityToInnerFunctions(fn, true); } /* diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts index 9ffc64864f397..999d9d1a9cd70 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts @@ -386,12 +386,10 @@ export function inlineJsxTransform( if (block.terminal.kind === 'scope') { const scope = block.terminal.scope; - for (const dep of scope.dependencies) { - dep.identifier = handleIdentifier( - dep.identifier, - inlinedJsxDeclarations, - ); - } + /** + * Note that scope dependencies don't need to be renamed explicitly + * as they will be visited when traversing scope terminal successors. + */ for (const [origId, decl] of [...scope.declarations]) { const newDecl = handleIdentifier( diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/BuildReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/BuildReactiveFunction.ts index 03e06dd0a2408..02b9731c48a62 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/BuildReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/BuildReactiveFunction.ts @@ -17,11 +17,16 @@ import { SourceLocation, } from '../HIR'; import { + areEqualPaths, + GeneratedSource, HIRFunction, + PrunedReactiveScopeBlock, ReactiveBreakTerminal, ReactiveContinueTerminal, ReactiveFunction, + ReactiveInstructionStatement, ReactiveLogicalValue, + ReactiveScopeBlock, ReactiveSequenceValue, ReactiveTerminalStatement, ReactiveTerminalTargetKind, @@ -29,7 +34,12 @@ import { ReactiveValue, Terminal, } from '../HIR/HIR'; +import { + readScopeDependencies, + readScopeDependenciesRHIR, +} from '../HIR/PropagateScopeDependenciesHIR'; import {assertExhaustive} from '../Utils/utils'; +import {printDependency} from './PrintReactiveFunction'; /* * Converts from HIR (lower-level CFG) to ReactiveFunction, a tree representation @@ -38,7 +48,7 @@ import {assertExhaustive} from '../Utils/utils'; * labels for *all* terminals: see PruneUnusedLabels which removes unnecessary labels. */ export function buildReactiveFunction(fn: HIRFunction): ReactiveFunction { - const cx = new Context(fn.body); + const cx = new Context(fn); const driver = new Driver(cx); const body = driver.traverseBlock(cx.block(fn.body.entry)); return { @@ -816,16 +826,65 @@ class Driver { } else { block = this.traverseBlock(this.cx.ir.blocks.get(terminal.block)!); } + { + const scheduleId = this.cx.schedule(terminal.block, 'if'); + scheduleIds.push(scheduleId); + this.cx.scopeFallthroughs.add(terminal.block); + } + CompilerError.invariant(!this.cx.isScheduled(terminal.dependencies), { + reason: `Unexpected 'scope' where the dependencies block is already scheduled`, + loc: terminal.loc, + }); + const dependencies: Array = + this.traverseBlock(this.cx.ir.blocks.get(terminal.dependencies)!).map( + dep => { + CompilerError.invariant(dep.kind === 'instruction', { + reason: '[BuildReactiveFunction] Expected reactive instruction', + loc: GeneratedSource, + }); + return dep; + }, + ); this.cx.unscheduleAll(scheduleIds); - blockValue.push({ + const scopeBlock: ReactiveScopeBlock | PrunedReactiveScopeBlock = { kind: terminal.kind, + dependencyInstructions: dependencies, instructions: block, scope: terminal.scope, - }); + }; + blockValue.push(scopeBlock); if (fallthroughId !== null) { this.visitBlock(this.cx.ir.blocks.get(fallthroughId)!, blockValue); } + /** + * Sanity check: check that HIR and RHIR dependencies match + */ + const hirDepsSanityCheck = readScopeDependencies( + this.cx.fn, + terminal.scope.id, + ); + const rhirDeps = readScopeDependenciesRHIR(scopeBlock); + CompilerError.invariant(hirDepsSanityCheck.size === rhirDeps.size, { + reason: `Mismatch in dependencies`, + loc: terminal.loc, + }); + outer: for (const hirDep of hirDepsSanityCheck) { + for (const [, rhirDep] of rhirDeps) { + if ( + hirDep.identifier === rhirDep.identifier && + areEqualPaths(hirDep.path, rhirDep.path) + ) { + continue outer; + } + } + CompilerError.invariant(false, { + reason: + '[BuildReactiveFunction] Rewrite sanity check: mismatching scope dependencies', + description: `No match found for ${printDependency(hirDep)}. Candidates are ${[...rhirDeps.values()].map(printDependency)}`, + loc: GeneratedSource, + }); + } break; } @@ -1187,6 +1246,7 @@ class Driver { loc: SourceLocation, ): ReactiveTerminalStatement | null { const target = this.cx.getBreakTarget(block); + // console.log(target); if (target === null) { CompilerError.invariant(false, { reason: 'Expected a break target', @@ -1243,6 +1303,7 @@ class Driver { } class Context { + fn: HIRFunction; ir: HIR; #nextScheduleId: number = 0; @@ -1273,8 +1334,9 @@ class Context { */ #controlFlowStack: Array = []; - constructor(ir: HIR) { - this.ir = ir; + constructor(fn: HIRFunction) { + this.fn = fn; + this.ir = fn.body; } block(id: BlockId): BasicBlock { @@ -1405,6 +1467,7 @@ class Context { * breaking to the last break point, which is where control will transfer * implicitly */ + // console.log(this.#controlFlowStack[i]); type = 'implicit'; } else { // breaking somewhere else requires an explicit break diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 93b70bd7d1da3..0c4d1e9b7e391 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -32,10 +32,10 @@ import { ReactiveBlock, ReactiveFunction, ReactiveInstruction, + ReactiveInstructionStatement, ReactiveScope, ReactiveScopeBlock, ReactiveScopeDeclaration, - ReactiveScopeDependency, ReactiveTerminal, ReactiveValue, SourceLocation, @@ -53,6 +53,7 @@ import {buildReactiveFunction} from './BuildReactiveFunction'; import {SINGLE_CHILD_FBT_TAGS} from './MemoizeFbtAndMacroOperandsInSameScope'; import {ReactiveFunctionVisitor, visitReactiveFunction} from './visitors'; import {ReactFunctionType} from '../HIR/Environment'; +import generate from '@babel/generator'; export const MEMO_CACHE_SENTINEL = 'react.memo_cache_sentinel'; export const EARLY_RETURN_SENTINEL = 'react.early_return_sentinel'; @@ -509,7 +510,13 @@ function codegenBlockNoReset( } case 'scope': { const temp = new Map(cx.temp); - codegenReactiveScope(cx, statements, item.scope, item.instructions); + codegenReactiveScope( + cx, + statements, + item.scope, + item.instructions, + item.dependencyInstructions, + ); cx.temp = temp; break; } @@ -568,6 +575,7 @@ function codegenReactiveScope( statements: Array, scope: ReactiveScope, block: ReactiveBlock, + dependencyInstructions: Array, ): void { const cacheStoreStatements: Array = []; const cacheLoadStatements: Array = []; @@ -580,9 +588,35 @@ function codegenReactiveScope( const changeExpressionComments: Array = []; const outputComments: Array = []; - for (const dep of [...scope.dependencies].sort(compareScopeDependency)) { + /** + * Step 1: Dependency instructions should codegen their expressions into + * the `Context.Temporaries` map. + */ + for (const instr of dependencyInstructions) { + const result = codegenInstructionNullable(cx, instr.instruction); + CompilerError.invariant(result == null, { + reason: 'Expected dependency instructions to be inlined', + loc: instr.instruction.loc, + }); + } + const sortedDependencies: Array<[string, t.Expression]> = scope.dependencies + .map<[string, t.Expression]>(dep => { + const dependency = codegenPlace(cx, dep); + CompilerError.invariant(dependency.type !== 'JSXText', { + reason: 'Expected dependency to not be JSXText', + loc: GeneratedSource, + }); + return [generate(dependency).code, dependency]; + }) + .sort(([aName], [bName]) => { + if (aName < bName) return -1; + else if (aName > bName) return 1; + else return 0; + }); + + for (const [code, dep] of sortedDependencies) { const index = cx.nextCacheIndex; - changeExpressionComments.push(printDependencyComment(dep)); + changeExpressionComments.push(code); const comparison = t.binaryExpression( '!==', t.memberExpression( @@ -590,7 +624,7 @@ function codegenReactiveScope( t.numericLiteral(index), true, ), - codegenDependency(cx, dep), + dep, ); if (cx.env.config.enableChangeVariableCodegen) { @@ -617,7 +651,7 @@ function codegenReactiveScope( t.numericLiteral(index), true, ), - codegenDependency(cx, dep), + t.cloneNode(dep, true), ), ), ); @@ -1410,17 +1444,6 @@ function codegenForInit( } } -function printDependencyComment(dependency: ReactiveScopeDependency): string { - const identifier = convertIdentifier(dependency.identifier); - let name = identifier.name; - if (dependency.path !== null) { - for (const path of dependency.path) { - name += `.${path.property}`; - } - } - return name; -} - function printDelimitedCommentList( items: Array, finalCompletion: string, @@ -1445,29 +1468,6 @@ function printDelimitedCommentList( return output.join(''); } -function codegenDependency( - cx: Context, - dependency: ReactiveScopeDependency, -): t.Expression { - let object: t.Expression = convertIdentifier(dependency.identifier); - if (dependency.path.length !== 0) { - const hasOptional = dependency.path.some(path => path.optional); - for (const path of dependency.path) { - if (hasOptional) { - object = t.optionalMemberExpression( - object, - t.identifier(path.property), - false, - path.optional, - ); - } else { - object = t.memberExpression(object, t.identifier(path.property)); - } - } - } - return object; -} - function withLoc) => t.Node>( fn: T, ): ( @@ -2576,30 +2576,6 @@ function convertIdentifier(identifier: Identifier): t.Identifier { return t.identifier(identifier.name.value); } -function compareScopeDependency( - a: ReactiveScopeDependency, - b: ReactiveScopeDependency, -): number { - CompilerError.invariant( - a.identifier.name?.kind === 'named' && b.identifier.name?.kind === 'named', - { - reason: '[Codegen] Expected named identifier for dependency', - loc: a.identifier.loc, - }, - ); - const aName = [ - a.identifier.name.value, - ...a.path.map(entry => `${entry.optional ? '?' : ''}${entry.property}`), - ].join('.'); - const bName = [ - b.identifier.name.value, - ...b.path.map(entry => `${entry.optional ? '?' : ''}${entry.property}`), - ].join('.'); - if (aName < bName) return -1; - else if (aName > bName) return 1; - else return 0; -} - function compareScopeDeclaration( a: ReactiveScopeDeclaration, b: ReactiveScopeDeclaration, diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/FlattenReactiveLoopsHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/FlattenReactiveLoopsHIR.ts index 66044ae534f41..ca58e140fcc17 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/FlattenReactiveLoopsHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/FlattenReactiveLoopsHIR.ts @@ -31,14 +31,16 @@ export function flattenReactiveLoopsHIR(fn: HIRFunction): void { } case 'scope': { if (activeLoops.length !== 0) { - block.terminal = { + const newTerminal: PrunedScopeTerminal = { kind: 'pruned-scope', block: terminal.block, + dependencies: terminal.dependencies, fallthrough: terminal.fallthrough, id: terminal.id, loc: terminal.loc, scope: terminal.scope, - } as PrunedScopeTerminal; + }; + block.terminal = newTerminal; } break; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/FlattenScopesWithHooksOrUseHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/FlattenScopesWithHooksOrUseHIR.ts index 103923a2e4d5e..326fb61f9be7a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/FlattenScopesWithHooksOrUseHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/FlattenScopesWithHooksOrUseHIR.ts @@ -83,28 +83,50 @@ export function flattenScopesWithHooksOrUseHIR(fn: HIRFunction): void { body.terminal.kind === 'goto' && body.terminal.block === terminal.fallthrough ) { + /** + * Note that scope blocks are unique in that they represent two nested + * goto-labels. We entirely remove dependency blocks here to simplify + * rewrite logic + */ + const dependencyBlock = fn.body.blocks.get(terminal.dependencies); + CompilerError.invariant( + dependencyBlock != null && + dependencyBlock.instructions.length === 0 && + dependencyBlock.terminal.kind === 'goto' && + dependencyBlock.terminal.block === terminal.block, + { + reason: `Expected scope dependency block to have no instructions and goto scope block`, + loc: terminal.loc, + }, + ); + fn.body.blocks.delete(terminal.dependencies); + const scopeBlock = fn.body.blocks.get(terminal.block)!; + scopeBlock.preds = new Set([block.id]); /* * This was a scope just for a hook call, which doesn't need memoization. * flatten it away. We rely on the PrunedUnusedLabel step to do the actual * flattening */ - block.terminal = { + const newTerminal: LabelTerminal = { kind: 'label', block: terminal.block, fallthrough: terminal.fallthrough, id: terminal.id, loc: terminal.loc, - } as LabelTerminal; + }; + block.terminal = newTerminal; continue; } - block.terminal = { + const newTerminal: PrunedScopeTerminal = { kind: 'pruned-scope', block: terminal.block, + dependencies: terminal.dependencies, fallthrough: terminal.fallthrough, id: terminal.id, loc: terminal.loc, scope: terminal.scope, - } as PrunedScopeTerminal; + }; + block.terminal = newTerminal; } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts index 1f104d8592fab..58cd583a8baf0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -107,7 +107,7 @@ export function inferReactiveScopeVariables(fn: HIRFunction): void { scope = { id: fn.env.nextScopeId, range: identifier.mutableRange, - dependencies: new Set(), + dependencies: [], declarations: new Map(), reassignments: new Set(), earlyReturnValue: null, diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts index 08d2212d86b95..1d002bf4f8580 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts @@ -28,6 +28,7 @@ import { BuiltInJsxId, BuiltInObjectId, } from '../HIR/ObjectShape'; +import {readScopeDependenciesRHIR} from '../HIR/PropagateScopeDependenciesHIR'; import {eachInstructionLValue} from '../HIR/visitors'; import {assertExhaustive, Iterable_some} from '../Utils/utils'; import {printReactiveScopeSummary} from './PrintReactiveFunction'; @@ -119,21 +120,29 @@ class FindLastUsageVisitor extends ReactiveFunctionVisitor { class Transform extends ReactiveFunctionTransform { lastUsage: Map; + cache: Map = new Map(); constructor(lastUsage: Map) { super(); this.lastUsage = lastUsage; } + dependency(scopeBlock: ReactiveScopeBlock): ReactiveScopeDependencies { + let dependencies = this.cache.get(scopeBlock); + if (dependencies == null) { + dependencies = new Set(readScopeDependenciesRHIR(scopeBlock).values()); + this.cache.set(scopeBlock, dependencies); + } + return dependencies; + } + override transformScope( scopeBlock: ReactiveScopeBlock, state: ReactiveScopeDependencies | null, ): Transformed { - this.visitScope(scopeBlock, scopeBlock.scope.dependencies); - if ( - state !== null && - areEqualDependencies(state, scopeBlock.scope.dependencies) - ) { + const dependencies = this.dependency(scopeBlock); + this.visitScope(scopeBlock, dependencies); + if (state !== null && areEqualDependencies(state, dependencies)) { return {kind: 'replace-many', value: scopeBlock.instructions}; } else { return {kind: 'keep'}; @@ -260,7 +269,7 @@ class Transform extends ReactiveFunctionTransform ({ identifier: declaration.identifier, + reactive: false, path: [], })), ), - next.scope.dependencies, + nextDeps, ) || - (next.scope.dependencies.size !== 0 && - [...next.scope.dependencies].every( + (nextDeps.size !== 0 && + [...nextDeps].every( dep => isAlwaysInvalidatingType(dep.identifier.type) && Iterable_some( @@ -537,7 +548,7 @@ function areEqualDependencies( * *never* change and it's also eligible for merging. */ function scopeIsEligibleForMerging(scopeBlock: ReactiveScopeBlock): boolean { - if (scopeBlock.scope.dependencies.size === 0) { + if (scopeBlock.scope.dependencies.length === 0) { /* * Regardless of the type of value produced, if the scope has no dependencies * then its value will never change. diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts index 259fc06486888..45f51d6310a39 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts @@ -64,11 +64,7 @@ export function printReactiveScopeSummary(scope: ReactiveScope): string { items.push('scope'); items.push(`@${scope.id}`); items.push(`[${scope.range.start}:${scope.range.end}]`); - items.push( - `dependencies=[${Array.from(scope.dependencies) - .map(dep => printDependency(dep)) - .join(', ')}]`, - ); + items.push(`dependencies=[${scope.dependencies.map(printPlace).join(',')}]`); items.push( `declarations=[${Array.from(scope.declarations) .map(([, decl]) => @@ -96,6 +92,8 @@ export function writeReactiveBlock( block: ReactiveScopeBlock, ): void { writer.writeLine(`${printReactiveScopeSummary(block.scope)} {`); + writeReactiveInstructions(writer, block.dependencyInstructions); + writer.writeLine('} /* - end dependencies - */ {'); writeReactiveInstructions(writer, block.instructions); writer.writeLine('}'); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts index 12cbde01b20ff..bd1b4ae0813e0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts @@ -26,18 +26,13 @@ import { } from '../HIR/HIR'; import {ReactiveFunctionVisitor, visitReactiveFunction} from './visitors'; import {eachInstructionValueLValue, eachPatternOperand} from '../HIR/visitors'; +import {scopeDependenciesDCE} from '../HIR/PropagateScopeDependenciesHIR'; /** * Phase 2: Promote identifiers which are used in a place that requires a named variable. */ class PromoteTemporaries extends ReactiveFunctionVisitor { override visitScope(scopeBlock: ReactiveScopeBlock, state: State): void { - for (const dep of scopeBlock.scope.dependencies) { - const {identifier} = dep; - if (identifier.name == null) { - promoteIdentifier(identifier, state); - } - } /* * This is technically optional. We could prune ReactiveScopes * whose outputs are not used in another computation or return @@ -50,7 +45,29 @@ class PromoteTemporaries extends ReactiveFunctionVisitor { promoteIdentifier(declaration.identifier, state); } } - this.traverseScope(scopeBlock, state); + /** + * Run DCE to remove dependency instructions that are no longer used due to + * pruning passes. + */ + scopeDependenciesDCE(scopeBlock); + /** + * Traverse into scope dependency instructions to promote references to + * unnamed outer identifiers. + */ + CompilerError.invariant(state.scopeDepContext == null, { + reason: 'PromoteTemporaries: unexpected nested scopeDepContext', + loc: GeneratedSource, + }); + this.visitBlock(scopeBlock.dependencyInstructions, { + ...state, + scopeDepContext: { + declarations: new Set(), + dependencies: new Set( + [...scopeBlock.scope.dependencies].map(dep => dep.identifier), + ), + }, + }); + this.visitBlock(scopeBlock.instructions, state); } override visitPrunedScope( @@ -75,11 +92,33 @@ class PromoteTemporaries extends ReactiveFunctionVisitor { } } + override visitLValue(_id: InstructionId, lvalue: Place, state: State): void { + // Track lvalues from within scope dependency blocks to avoid promoting them. + state.scopeDepContext?.declarations.add(lvalue.identifier.declarationId); + } + override visitValue( id: InstructionId, value: ReactiveValue, state: State, ): void { + if ( + state.scopeDepContext && + (value.kind === 'LoadLocal' || value.kind === 'LoadContext') + ) { + /** + * Scope dependency LoadLocal sources (defined by instructions external to + * that scope) should be promoted, as scope boundaries represent + * re-ordering barriers + */ + const identifier = value.place.identifier; + if ( + !state.scopeDepContext.declarations.has(identifier.declarationId) && + identifier.name === null + ) { + promoteIdentifier(identifier, state); + } + } this.traverseValue(id, value, state); if (value.kind === 'FunctionExpression' || value.kind === 'ObjectMethod') { this.visitHirFunction(value.loweredFunc.func, state); @@ -177,6 +216,10 @@ type State = { DeclarationId, {activeScopes: Array; usedOutsideScope: boolean} >; // true if referenced within another scope, false if only accessed outside of scopes + scopeDepContext: { + declarations: Set; + dependencies: ReadonlySet; + } | null; }; /** @@ -427,6 +470,7 @@ export function promoteUsedTemporaries(fn: ReactiveFunction): void { tags: new Set(), promoted: new Set(), pruned: new Map(), + scopeDepContext: null, }; visitReactiveFunction(fn, new CollectPromotableTemporaries(), state); for (const operand of fn.params) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneAlwaysInvalidatingScopes.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneAlwaysInvalidatingScopes.ts index b470271cf00b9..e7c5a7c1657bd 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneAlwaysInvalidatingScopes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneAlwaysInvalidatingScopes.ts @@ -13,6 +13,7 @@ import { ReactiveScopeBlock, ReactiveStatement, } from '../HIR'; +import {readScopeDependenciesRHIR} from '../HIR/PropagateScopeDependenciesHIR'; /** * Some instructions will *always* produce a new value, and unless memoized will *always* @@ -87,7 +88,8 @@ class Transform extends ReactiveFunctionTransform { ): Transformed { this.visitScope(scopeBlock, true); - for (const dep of scopeBlock.scope.dependencies) { + const scopeDeps = readScopeDependenciesRHIR(scopeBlock); + for (const [, dep] of scopeDeps) { if (this.unmemoizedValues.has(dep.identifier)) { /* * This scope depends on an always-invalidating value so the scope will always invalidate: @@ -107,6 +109,7 @@ class Transform extends ReactiveFunctionTransform { kind: 'replace', value: { kind: 'pruned-scope', + dependencyInstructions: scopeBlock.dependencyInstructions, scope: scopeBlock.scope, instructions: scopeBlock.instructions, }, diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneInitializationDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneInitializationDependencies.ts index 2a9d0b9793d9f..3bac6f0853919 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneInitializationDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneInitializationDependencies.ts @@ -21,6 +21,7 @@ import { isUseRefType, isUseStateType, } from '../HIR'; +import {readScopeDependenciesRHIR} from '../HIR/PropagateScopeDependenciesHIR'; import {eachCallArgument, eachInstructionLValue} from '../HIR/visitors'; import DisjointSet from '../Utils/DisjointSet'; import {assertExhaustive} from '../Utils/utils'; @@ -177,14 +178,22 @@ class Visitor extends ReactiveFunctionVisitor { ].map(id => this.map.get(id) ?? 'Unknown'), ); super.visitScope(scope, state); - [...scope.scope.dependencies].forEach(ident => { + // TODO + const scopeDeps = readScopeDependenciesRHIR(scope); + [...scopeDeps].forEach(([place, dep]) => { let target: undefined | IdentifierId = - this.aliases.find(ident.identifier.id) ?? ident.identifier.id; - ident.path.forEach(token => { + this.aliases.find(dep.identifier.id) ?? dep.identifier.id; + dep.path.forEach(token => { target &&= this.paths.get(target)?.get(token.property); }); if (target && this.map.get(target) === 'Create') { - scope.scope.dependencies.delete(ident); + const idx = scope.scope.dependencies.indexOf(place); + CompilerError.invariant(idx !== -1, { + reason: 'Expected dependency to be found', + loc: place.loc, + }); + + scope.scope.dependencies.splice(idx, 1); } }); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts index 9bdf15aeb5343..d905146ec4dbc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts @@ -95,13 +95,19 @@ class Visitor extends ReactiveFunctionVisitor { state: ReactiveIdentifiers, ): void { this.traverseScope(scopeBlock, state); - for (const dep of scopeBlock.scope.dependencies) { - const isReactive = state.has(dep.identifier.id); - if (!isReactive) { - scopeBlock.scope.dependencies.delete(dep); + let j = 0; + for (let i = 0; i < scopeBlock.scope.dependencies.length; i++) { + const isReactive = state.has( + scopeBlock.scope.dependencies[i].identifier.id, + ); + if (isReactive) { + scopeBlock.scope.dependencies[j] = scopeBlock.scope.dependencies[i]; + j++; } } - if (scopeBlock.scope.dependencies.size !== 0) { + scopeBlock.scope.dependencies.length = j; + + if (scopeBlock.scope.dependencies.length !== 0) { /** * If any of a scope's dependencies are reactive, then all of its * outputs will re-evaluate whenever those dependencies change. diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneUnusedScopes.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneUnusedScopes.ts index fdcf0065297a8..e4d9ecd9d541d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneUnusedScopes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneUnusedScopes.ts @@ -56,6 +56,7 @@ class Transform extends ReactiveFunctionTransform { value: { kind: 'pruned-scope', scope: scopeBlock.scope, + dependencyInstructions: scopeBlock.dependencyInstructions, instructions: scopeBlock.instructions, }, }; diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/visitors.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/visitors.ts index 4ad05aa302ac1..499c9601a7a4f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/visitors.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/visitors.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import {CompilerError} from '..'; import { HIRFunction, InstructionId, @@ -185,6 +186,20 @@ export class ReactiveFunctionVisitor { this.traverseScope(scope, state); } traverseScope(scope: ReactiveScopeBlock, state: TState): void { + this.visitBlock(scope.dependencyInstructions, state); + const lastDependencyInstruction = scope.dependencyInstructions.at(-1); + let lastInstructionId: InstructionId | null = null; + if (lastDependencyInstruction !== undefined) { + lastInstructionId = lastDependencyInstruction.instruction.id; + } + for (const dep of scope.scope.dependencies) { + CompilerError.invariant(lastInstructionId !== null, { + reason: + '[ReactiveFunction] Expected at least one dependency instruction.', + loc: scope.scope.loc, + }); + this.visitPlace(lastInstructionId, dep, state); + } this.visitBlock(scope.instructions, state); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateMemoizedEffectDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateMemoizedEffectDependencies.ts index 364e78ae6b68d..f871b03db0eb6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateMemoizedEffectDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateMemoizedEffectDependencies.ts @@ -17,6 +17,7 @@ import { isUseInsertionEffectHookType, isUseLayoutEffectHookType, } from '../HIR'; +import {readScopeDependenciesRHIR} from '../HIR/PropagateScopeDependenciesHIR'; import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables'; import { ReactiveFunctionVisitor, @@ -72,7 +73,7 @@ class Visitor extends ReactiveFunctionVisitor { * memoized, allowing a transitive memoization check. */ let areDependenciesMemoized = true; - for (const dep of scopeBlock.scope.dependencies) { + for (const [, dep] of readScopeDependenciesRHIR(scopeBlock)) { if (isUnmemoized(dep.identifier, this.scopes)) { areDependenciesMemoized = false; break; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts index 28be6c166d2e4..974a82f2af90a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts @@ -24,6 +24,7 @@ import { SourceLocation, } from '../HIR'; import {printIdentifier, printManualMemoDependency} from '../HIR/PrintHIR'; +import {readScopeDependenciesRHIR} from '../HIR/PropagateScopeDependenciesHIR'; import {eachInstructionValueOperand} from '../HIR/visitors'; import {collectMaybeMemoDependencies} from '../Inference/DropManualMemoization'; import { @@ -404,7 +405,8 @@ class Visitor extends ReactiveFunctionVisitor { state.manualMemoState != null && state.manualMemoState.depsFromSource != null ) { - for (const dep of scopeBlock.scope.dependencies) { + const deps = readScopeDependenciesRHIR(scopeBlock).values(); + for (const dep of deps) { validateInferredDep( dep, this.temporaries,