From 2500367a26f1552f2426312c61dde10c84851bfd Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 23 Sep 2024 17:36:25 -0400 Subject: [PATCH 1/3] Update [ghstack-poisoned] --- .../src/HIR/CollectHoistablePropertyLoads.ts | 180 +++++++++--------- 1 file changed, 86 insertions(+), 94 deletions(-) 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 87d1e9e8b25e0..ae8e8cd59b2f7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -84,98 +84,81 @@ export function collectHoistablePropertyLoads( export type BlockInfo = { block: BasicBlock; - assumedNonNullObjects: ReadonlySet; + assumedNonNullObjects: ReadonlySet; }; /** * Tree data structure to dedupe property loads (e.g. a.b.c) * and make computing sets intersections simpler. */ -type RootNode = { - properties: Map; - parent: null; - // Recorded to make later computations simpler - fullPath: ReactiveScopeDependency; - root: IdentifierId; +type DedupedNode = { + key: string; + dep: ReactiveScopeDependency; }; - -type PropertyLoadNode = - | { - properties: Map; - parent: PropertyLoadNode; - fullPath: ReactiveScopeDependency; +function depToKey(dep: ReactiveScopeDependency): string { + let key = dep.identifier.id.toString(); + for (let path of dep.path) { + if (path.optional) { + key += ' ?. ' + path.property; + } else { + key += ' . ' + path.property; } - | RootNode; - -class Tree { - roots: Map = new Map(); - - getOrCreateRoot(identifier: Identifier): PropertyLoadNode { - /** - * Reads from a statically scoped variable are always safe in JS, - * with the exception of TDZ (not addressed by this pass). - */ - let rootNode = this.roots.get(identifier.id); - - if (rootNode === undefined) { - rootNode = { - root: identifier.id, - properties: new Map(), - fullPath: { - identifier, - path: [], - }, - parent: null, - }; - this.roots.set(identifier.id, rootNode); - } - return rootNode; } + return key; +} +function findSuperpaths( + match: ReactiveScopeDependency, + nodes: Set, +): Set { + const key = depToKey(match) + ' '; + const result = new Set(); - static #getOrCreateProperty( - node: PropertyLoadNode, - property: string, - ): PropertyLoadNode { - let child = node.properties.get(property); - if (child == null) { - child = { - properties: new Map(), - parent: node, - fullPath: { - identifier: node.fullPath.identifier, - path: node.fullPath.path.concat([{property, optional: false}]), - }, - }; - node.properties.set(property, child); + for (const n of nodes) { + if (n.key.startsWith(key)) { + result.add(n); } - return child; } + return result; +} - getPropertyLoadNode(n: ReactiveScopeDependency): PropertyLoadNode { - /** - * We add ReactiveScopeDependencies according to instruction ordering, - * so all subpaths of a PropertyLoad should already exist - * (e.g. a.b is added before a.b.c), - */ - let currNode = this.getOrCreateRoot(n.identifier); - if (n.path.length === 0) { - return currNode; +class DedupeMap { + nodes: Map = new Map(); + + getOrCreateIdentifier(id: Identifier): DedupedNode { + const key = id.id.toString(); + let result = this.nodes.get(key); + if (result != null) { + return result; } - for (let i = 0; i < n.path.length - 1; i++) { - currNode = assertNonNull(currNode.properties.get(n.path[i].property)); + result = { + key, + dep: {identifier: id, path: []}, + }; + this.nodes.set(key, result); + return result; + } + getOrCreateProperty(dep: ReactiveScopeDependency): DedupedNode { + const key = depToKey(dep); + let result = this.nodes.get(key); + if (result != null) { + return result; } - - return Tree.#getOrCreateProperty(currNode, n.path.at(-1)!.property); + result = { + key, + dep, + }; + this.nodes.set(key, result); + return result; } } function pushPropertyLoadNode( - node: PropertyLoadNode, + node: DedupedNode, instrId: InstructionId, knownImmutableIdentifiers: Set, - result: Set, + result: Set, ): void { - const object = node.fullPath.identifier; + const object = node.dep.identifier; /** * Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges * are not valid with respect to current instruction id numbering. @@ -192,13 +175,9 @@ function pushPropertyLoadNode( inRange({id: instrId}, object.scope.range); if ( !isMutableAtInstr || - knownImmutableIdentifiers.has(node.fullPath.identifier.id) + knownImmutableIdentifiers.has(node.dep.identifier.id) ) { - let curr: PropertyLoadNode | null = node; - while (curr != null) { - result.add(curr); - curr = curr.parent; - } + result.add(node); } } @@ -206,7 +185,7 @@ function collectNonNullsInBlocks( fn: HIRFunction, temporaries: ReadonlyMap, ): ReadonlyMap { - const tree = new Tree(); + const tree = new DedupeMap(); /** * Due to current limitations of mutable range inference, there are edge cases in * which we infer known-immutable values (e.g. props or hook params) to have a @@ -227,7 +206,7 @@ function collectNonNullsInBlocks( * Known non-null objects such as functional component props can be safely * read from any block. */ - const knownNonNullIdentifiers = new Set(); + const knownNonNullIdentifiers = new Set(); if ( fn.env.config.enablePropagateDepsInHIR === 'enabled_with_optimizations' && fn.fnType === 'Component' && @@ -235,20 +214,38 @@ function collectNonNullsInBlocks( fn.params[0].kind === 'Identifier' ) { const identifier = fn.params[0].identifier; - knownNonNullIdentifiers.add(tree.getOrCreateRoot(identifier)); + knownNonNullIdentifiers.add(tree.getOrCreateIdentifier(identifier)); } const nodes = new Map(); for (const [_, block] of fn.body.blocks) { - const assumedNonNullObjects = new Set( - knownNonNullIdentifiers, - ); + const assumedNonNullObjects = new Set(knownNonNullIdentifiers); + + nodes.set(block.id, { + block, + assumedNonNullObjects, + }); + for (const phi of block.phis) { + const source = temporaries.get(phi.id.id); + if (source) { + const propertyNode = tree.getOrCreateProperty(source); + pushPropertyLoadNode( + propertyNode, + // TODO double check which instr id + block.instructions.length > 0 + ? block.instructions[0].id + : block.terminal.id, + knownImmutableIdentifiers, + assumedNonNullObjects, + ); + } + } for (const instr of block.instructions) { if (instr.value.kind === 'PropertyLoad') { const source = temporaries.get(instr.value.object.identifier.id) ?? { identifier: instr.value.object.identifier, path: [], }; - const propertyNode = tree.getPropertyLoadNode(source); + const propertyNode = tree.getOrCreateProperty(source); pushPropertyLoadNode( propertyNode, instr.id, @@ -263,7 +260,7 @@ function collectNonNullsInBlocks( const sourceNode = temporaries.get(source); if (sourceNode != null) { pushPropertyLoadNode( - tree.getPropertyLoadNode(sourceNode), + tree.getOrCreateProperty(sourceNode), instr.id, knownImmutableIdentifiers, assumedNonNullObjects, @@ -277,7 +274,7 @@ function collectNonNullsInBlocks( const sourceNode = temporaries.get(source); if (sourceNode != null) { pushPropertyLoadNode( - tree.getPropertyLoadNode(sourceNode), + tree.getOrCreateProperty(sourceNode), instr.id, knownImmutableIdentifiers, assumedNonNullObjects, @@ -285,11 +282,6 @@ function collectNonNullsInBlocks( } } } - - nodes.set(block.id, { - block, - assumedNonNullObjects, - }); } return nodes; } @@ -319,7 +311,7 @@ function propagateNonNull( nodeId: BlockId, direction: 'forward' | 'backward', traversalState: Map, - nonNullObjectsByBlock: Map>, + nonNullObjectsByBlock: Map>, ): boolean { /** * Avoid re-visiting computed or currently active nodes, which can @@ -390,8 +382,8 @@ function propagateNonNull( changed ||= prevObjects.size !== newObjects.size; return changed; } - const fromEntry = new Map>(); - const fromExit = new Map>(); + const fromEntry = new Map>(); + const fromExit = new Map>(); for (const [blockId, blockInfo] of nodes) { fromEntry.set(blockId, blockInfo.assumedNonNullObjects); fromExit.set(blockId, blockInfo.assumedNonNullObjects); @@ -456,7 +448,7 @@ function propagateNonNull( } } -function assertNonNull, U>( +export function assertNonNull, U>( value: T | null | undefined, source?: string, ): T { From 71aa14e02cc4a7cc234c66b847cf1f8f4814afb5 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Wed, 25 Sep 2024 18:54:22 -0400 Subject: [PATCH 2/3] Update [ghstack-poisoned] --- .../src/HIR/CollectHoistablePropertyLoads.ts | 228 ++++++++---------- .../src/HIR/DeriveMinimalDependenciesHIR.ts | 25 +- .../src/HIR/HIR.ts | 18 +- 3 files changed, 131 insertions(+), 140 deletions(-) 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 ae8e8cd59b2f7..82a2707e69f88 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -4,6 +4,7 @@ import {Set_intersect, Set_union, getOrInsertDefault} from '../Utils/utils'; import { BasicBlock, BlockId, + DependencyPathEntry, GeneratedSource, HIRFunction, Identifier, @@ -66,8 +67,10 @@ export function collectHoistablePropertyLoads( fn: HIRFunction, temporaries: ReadonlyMap, ): ReadonlyMap { - const nodes = collectNonNullsInBlocks(fn, temporaries); - propagateNonNull(fn, nodes); + const tree = new Tree(); + + const nodes = collectNonNullsInBlocks(fn, temporaries, tree); + propagateNonNull(fn, nodes, tree); const nodesKeyedByScopeId = new Map(); for (const [_, block] of fn.body.blocks) { @@ -84,81 +87,104 @@ export function collectHoistablePropertyLoads( export type BlockInfo = { block: BasicBlock; - assumedNonNullObjects: ReadonlySet; + assumedNonNullObjects: ReadonlySet; }; /** * Tree data structure to dedupe property loads (e.g. a.b.c) * and make computing sets intersections simpler. */ -type DedupedNode = { - key: string; - dep: ReactiveScopeDependency; +type RootNode = { + properties: Map; + parent: null; + // Recorded to make later computations simpler + fullPath: ReactiveScopeDependency; + root: IdentifierId; }; -function depToKey(dep: ReactiveScopeDependency): string { - let key = dep.identifier.id.toString(); - for (let path of dep.path) { - if (path.optional) { - key += ' ?. ' + path.property; - } else { - key += ' . ' + path.property; + +type PropertyLoadNode = + | { + properties: Map; + parent: PropertyLoadNode; + fullPath: ReactiveScopeDependency; } - } - return key; -} -function findSuperpaths( - match: ReactiveScopeDependency, - nodes: Set, -): Set { - const key = depToKey(match) + ' '; - const result = new Set(); + | RootNode; + +class Tree { + roots: Map = new Map(); + + getOrCreateIdentifier(identifier: Identifier): PropertyLoadNode { + /** + * Reads from a statically scoped variable are always safe in JS, + * with the exception of TDZ (not addressed by this pass). + */ + let rootNode = this.roots.get(identifier.id); - for (const n of nodes) { - if (n.key.startsWith(key)) { - result.add(n); + if (rootNode === undefined) { + rootNode = { + root: identifier.id, + properties: new Map(), + fullPath: { + identifier, + path: [], + }, + parent: null, + }; + this.roots.set(identifier.id, rootNode); } + return rootNode; } - return result; -} - -class DedupeMap { - nodes: Map = new Map(); - getOrCreateIdentifier(id: Identifier): DedupedNode { - const key = id.id.toString(); - let result = this.nodes.get(key); - if (result != null) { - return result; + static getOrCreatePropertyEntry( + parent: PropertyLoadNode, + entry: DependencyPathEntry, + ): PropertyLoadNode { + if (entry.optional) { + CompilerError.throwTodo({ + reason: 'handle optional nodes', + loc: GeneratedSource, + }); + } + let child = parent.properties.get(entry.property); + if (child == null) { + child = { + properties: new Map(), + parent: parent, + fullPath: { + identifier: parent.fullPath.identifier, + path: parent.fullPath.path.concat(entry), + }, + }; + parent.properties.set(entry.property, child); } - result = { - key, - dep: {identifier: id, path: []}, - }; - this.nodes.set(key, result); - return result; + return child; } - getOrCreateProperty(dep: ReactiveScopeDependency): DedupedNode { - const key = depToKey(dep); - let result = this.nodes.get(key); - if (result != null) { - return result; + + getOrCreateProperty(n: ReactiveScopeDependency): PropertyLoadNode { + /** + * We add ReactiveScopeDependencies according to instruction ordering, + * 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); + if (n.path.length === 0) { + return currNode; + } + for (let i = 0; i < n.path.length - 1; i++) { + currNode = Tree.getOrCreatePropertyEntry(currNode, n.path[i]); } - result = { - key, - dep, - }; - this.nodes.set(key, result); - return result; + + return Tree.getOrCreatePropertyEntry(currNode, n.path.at(-1)!); } } function pushPropertyLoadNode( - node: DedupedNode, + node: PropertyLoadNode, instrId: InstructionId, knownImmutableIdentifiers: Set, - result: Set, + result: Set, ): void { - const object = node.dep.identifier; + const object = node.fullPath.identifier; /** * Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges * are not valid with respect to current instruction id numbering. @@ -175,7 +201,7 @@ function pushPropertyLoadNode( inRange({id: instrId}, object.scope.range); if ( !isMutableAtInstr || - knownImmutableIdentifiers.has(node.dep.identifier.id) + knownImmutableIdentifiers.has(node.fullPath.identifier.id) ) { result.add(node); } @@ -184,8 +210,8 @@ function pushPropertyLoadNode( function collectNonNullsInBlocks( fn: HIRFunction, temporaries: ReadonlyMap, + tree: Tree, ): ReadonlyMap { - const tree = new DedupeMap(); /** * Due to current limitations of mutable range inference, there are edge cases in * which we infer known-immutable values (e.g. props or hook params) to have a @@ -206,7 +232,7 @@ function collectNonNullsInBlocks( * Known non-null objects such as functional component props can be safely * read from any block. */ - const knownNonNullIdentifiers = new Set(); + const knownNonNullIdentifiers = new Set(); if ( fn.env.config.enablePropagateDepsInHIR === 'enabled_with_optimizations' && fn.fnType === 'Component' && @@ -218,27 +244,9 @@ function collectNonNullsInBlocks( } const nodes = new Map(); for (const [_, block] of fn.body.blocks) { - const assumedNonNullObjects = new Set(knownNonNullIdentifiers); - - nodes.set(block.id, { - block, - assumedNonNullObjects, - }); - for (const phi of block.phis) { - const source = temporaries.get(phi.id.id); - if (source) { - const propertyNode = tree.getOrCreateProperty(source); - pushPropertyLoadNode( - propertyNode, - // TODO double check which instr id - block.instructions.length > 0 - ? block.instructions[0].id - : block.terminal.id, - knownImmutableIdentifiers, - assumedNonNullObjects, - ); - } - } + const assumedNonNullObjects = new Set( + knownNonNullIdentifiers, + ); for (const instr of block.instructions) { if (instr.value.kind === 'PropertyLoad') { const source = temporaries.get(instr.value.object.identifier.id) ?? { @@ -282,6 +290,11 @@ function collectNonNullsInBlocks( } } } + + nodes.set(block.id, { + block, + assumedNonNullObjects, + }); } return nodes; } @@ -289,6 +302,7 @@ function collectNonNullsInBlocks( function propagateNonNull( fn: HIRFunction, nodes: ReadonlyMap, + tree: Tree, ): void { const blockSuccessors = new Map>(); const terminalPreds = new Set(); @@ -311,7 +325,6 @@ function propagateNonNull( nodeId: BlockId, direction: 'forward' | 'backward', traversalState: Map, - nonNullObjectsByBlock: Map>, ): boolean { /** * Avoid re-visiting computed or currently active nodes, which can @@ -342,7 +355,6 @@ function propagateNonNull( pred, direction, traversalState, - nonNullObjectsByBlock, ); changed ||= neighborChanged; } @@ -371,38 +383,37 @@ function propagateNonNull( const neighborAccesses = Set_intersect( Array.from(neighbors) .filter(n => traversalState.get(n) === 'done') - .map(n => assertNonNull(nonNullObjectsByBlock.get(n))), + .map(n => assertNonNull(nodes.get(n)).assumedNonNullObjects), ); - const prevObjects = assertNonNull(nonNullObjectsByBlock.get(nodeId)); - const newObjects = Set_union(prevObjects, neighborAccesses); + const prevObjects = assertNonNull(nodes.get(nodeId)).assumedNonNullObjects; + const mergedObjects = Set_union(prevObjects, neighborAccesses); - nonNullObjectsByBlock.set(nodeId, newObjects); + assertNonNull(nodes.get(nodeId)).assumedNonNullObjects = mergedObjects; traversalState.set(nodeId, 'done'); - changed ||= prevObjects.size !== newObjects.size; + changed ||= prevObjects.size === mergedObjects.size; return changed; } - const fromEntry = new Map>(); - const fromExit = new Map>(); - for (const [blockId, blockInfo] of nodes) { - fromEntry.set(blockId, blockInfo.assumedNonNullObjects); - fromExit.set(blockId, blockInfo.assumedNonNullObjects); - } const traversalState = new Map(); const reversedBlocks = [...fn.body.blocks]; reversedBlocks.reverse(); - let i = 0; let changed; + let i = 0; do { i++; + CompilerError.invariant(i++ < 100, { + reason: + '[CollectHoistablePropertyLoads] fixed point iteration did not terminate after 1000 loops', + loc: GeneratedSource, + }); + changed = false; for (const [blockId] of fn.body.blocks) { const forwardChanged = recursivelyPropagateNonNull( blockId, 'forward', traversalState, - fromEntry, ); changed ||= forwardChanged; } @@ -412,40 +423,11 @@ function propagateNonNull( blockId, 'backward', traversalState, - fromExit, ); changed ||= backwardChanged; } traversalState.clear(); } while (changed); - - /** - * TODO: validate against meta internal code, then remove in future PR. - * Currently cannot come up with a case that requires fixed-point iteration. - */ - CompilerError.invariant(i <= 2, { - reason: 'require fixed-point iteration', - description: `#iterations = ${i}`, - loc: GeneratedSource, - }); - - CompilerError.invariant( - fromEntry.size === fromExit.size && fromEntry.size === nodes.size, - { - reason: - 'bad sizes after calculating fromEntry + fromExit ' + - `${fromEntry.size} ${fromExit.size} ${nodes.size}`, - loc: GeneratedSource, - }, - ); - - for (const [id, node] of nodes) { - const assumedNonNullObjects = Set_union( - assertNonNull(fromEntry.get(id)), - assertNonNull(fromExit.get(id)), - ); - node.assumedNonNullObjects = assumedNonNullObjects; - } } export function assertNonNull, U>( 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 ecc1844b006aa..f2bb0b31f05c9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts @@ -19,16 +19,17 @@ const ENABLE_DEBUG_INVARIANTS = true; export class ReactiveScopeDependencyTreeHIR { #roots: Map = new Map(); - #getOrCreateRoot(identifier: Identifier, isNonNull: boolean): DependencyNode { + #getOrCreateRoot( + identifier: Identifier, + accessType: PropertyAccessType, + ): DependencyNode { // roots can always be accessed unconditionally in JS let rootNode = this.#roots.get(identifier); if (rootNode === undefined) { rootNode = { properties: new Map(), - accessType: isNonNull - ? PropertyAccessType.NonNullAccess - : PropertyAccessType.Access, + accessType, }; this.#roots.set(identifier, rootNode); } @@ -37,7 +38,7 @@ export class ReactiveScopeDependencyTreeHIR { addDependency(dep: ReactiveScopePropertyDependency): void { const {path} = dep; - let currNode = this.#getOrCreateRoot(dep.identifier, false); + let currNode = this.#getOrCreateRoot(dep.identifier, MIN_ACCESS_TYPE); const accessType = PropertyAccessType.Access; @@ -45,8 +46,11 @@ export class ReactiveScopeDependencyTreeHIR { for (const property of path) { // all properties read 'on the way' to a dependency are marked as 'access' - let currChild = getOrMakeProperty(currNode, property.property); - currChild.accessType = merge(currChild.accessType, accessType); + let currChild = makeOrMergeProperty( + currNode, + property.property, + accessType, + ); currNode = currChild; } @@ -251,17 +255,20 @@ function printSubtree( return results; } -function getOrMakeProperty( +function makeOrMergeProperty( node: DependencyNode, property: string, + accessType: PropertyAccessType, ): DependencyNode { let child = node.properties.get(property); if (child == null) { child = { properties: new Map(), - accessType: MIN_ACCESS_TYPE, + accessType, }; node.properties.set(property, child); + } else { + child.accessType = merge(child.accessType, accessType); } return child; } 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 30408ab032b35..615ec18feb962 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -874,13 +874,7 @@ export type InstructionValue = }; loc: SourceLocation; } - | { - kind: 'StoreLocal'; - lvalue: LValue; - value: Place; - type: t.FlowType | t.TSType | null; - loc: SourceLocation; - } + | StoreLocal | { kind: 'StoreContext'; lvalue: { @@ -1123,6 +1117,13 @@ export type Primitive = { export type JSXText = {kind: 'JSXText'; value: string; loc: SourceLocation}; +export type StoreLocal = { + kind: 'StoreLocal'; + lvalue: LValue; + value: Place; + type: t.FlowType | t.TSType | null; + loc: SourceLocation; +}; export type PropertyLoad = { kind: 'PropertyLoad'; object: Place; @@ -1496,7 +1497,8 @@ export type ReactiveScopeDeclaration = { scope: ReactiveScope; // the scope in which the variable was originally declared }; -export type DependencyPath = Array<{property: string; optional: boolean}>; +export type DependencyPathEntry = {property: string; optional: boolean}; +export type DependencyPath = Array; export type ReactiveScopeDependency = { identifier: Identifier; path: DependencyPath; From 48a9df2f8a5e0ab304738dbee16248eb48574ad7 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Wed, 25 Sep 2024 19:15:55 -0400 Subject: [PATCH 3/3] Update [ghstack-poisoned] --- .../src/HIR/CollectHoistablePropertyLoads.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) 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 82a2707e69f88..f41dd91662d4a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -70,7 +70,7 @@ export function collectHoistablePropertyLoads( const tree = new Tree(); const nodes = collectNonNullsInBlocks(fn, temporaries, tree); - propagateNonNull(fn, nodes, tree); + propagateNonNull(fn, nodes); const nodesKeyedByScopeId = new Map(); for (const [_, block] of fn.body.blocks) { @@ -302,7 +302,6 @@ function collectNonNullsInBlocks( function propagateNonNull( fn: HIRFunction, nodes: ReadonlyMap, - tree: Tree, ): void { const blockSuccessors = new Map>(); const terminalPreds = new Set(); @@ -391,7 +390,7 @@ function propagateNonNull( assertNonNull(nodes.get(nodeId)).assumedNonNullObjects = mergedObjects; traversalState.set(nodeId, 'done'); - changed ||= prevObjects.size === mergedObjects.size; + changed ||= prevObjects.size !== mergedObjects.size; return changed; } const traversalState = new Map(); @@ -401,10 +400,9 @@ function propagateNonNull( let changed; let i = 0; do { - i++; CompilerError.invariant(i++ < 100, { reason: - '[CollectHoistablePropertyLoads] fixed point iteration did not terminate after 1000 loops', + '[CollectHoistablePropertyLoads] fixed point iteration did not terminate after 100 loops', loc: GeneratedSource, });