From 86e2edfa879f4cbd06a1f6bbaf0c8a6ab08cf59b Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 15 Dec 2023 13:47:23 -0800 Subject: [PATCH] Prune memoize instructions in codegen The previous PR introduced `memoize` instructions whose lvalues aren't used, but which can't be pruned by DCE due to pipeline ordering. Here we change to make memoize an instruction intended for its side effects only, and prune during codegen. --- .../babel-plugin-react-forget/src/HIR/HIR.ts | 4 ++++ .../src/Inference/DropManualMemoization.ts | 12 +++++------- .../src/Inference/InferReferenceEffects.ts | 10 +++++----- .../src/ReactiveScopes/CodegenReactiveFunction.ts | 7 +++---- .../src/ReactiveScopes/PruneNonEscapingScopes.ts | 2 +- .../src/TypeInference/InferTypes.ts | 7 ++----- ...ariable-preserve-memoization-guarantees.expect.md | 10 +++------- ...d-later-preserve-memoization-guarantees.expect.md | 6 +++--- 8 files changed, 26 insertions(+), 32 deletions(-) diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts index 3f4ee9857b..8bf6f6634d 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts @@ -849,6 +849,10 @@ export type InstructionValue = * Represents semantic information from useMemo/useCallback that the developer * has indicated a particular value should be memoized. This value is ignored * unless the TODO flag is enabled. + * + * NOTE: the Memoize instruction is intended for side-effects only, and is pruned + * during codegen. It can't be pruned during DCE because we need to preserve the + * instruction so it can be visible in InferReferenceEffects. */ | { kind: "Memoize"; value: Place; loc: SourceLocation } /* diff --git a/compiler/packages/babel-plugin-react-forget/src/Inference/DropManualMemoization.ts b/compiler/packages/babel-plugin-react-forget/src/Inference/DropManualMemoization.ts index 8e3f29b46e..e83595da7d 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Inference/DropManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Inference/DropManualMemoization.ts @@ -137,9 +137,6 @@ export function dropManualMemoization(func: HIRFunction): void { * the original lvalue for the result of the Memoize instruction, so that * we don't have to rewrite subsequent instructions. */ - const lvalue = instr.lvalue; - const temp = createTemporaryPlace(func.env); - instr.lvalue = { ...temp }; nextInstructions = nextInstructions ?? block.instructions.slice(0, i); @@ -148,10 +145,10 @@ export function dropManualMemoization(func: HIRFunction): void { for (const operand of eachInstructionValueOperand( functionExpression )) { - const operandLValue = createTemporaryPlace(func.env); + const temp = createTemporaryPlace(func.env); nextInstructions.push({ id: makeInstructionId(0), - lvalue: operandLValue, + lvalue: temp, value: { kind: "Memoize", value: { ...operand }, @@ -164,12 +161,13 @@ export function dropManualMemoization(func: HIRFunction): void { nextInstructions.push(instr); + const temp = createTemporaryPlace(func.env); nextInstructions.push({ id: makeInstructionId(0), - lvalue, + lvalue: temp, value: { kind: "Memoize", - value: temp, + value: { ...instr.lvalue }, loc: instr.loc, }, loc: instr.loc, diff --git a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts index a6afdf6d51..200f3762f7 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts @@ -1163,14 +1163,14 @@ function inferBlock( continue; } case "Memoize": { - state.initialize(instrValue, { - kind: ValueKind.Frozen, - reason: new Set([ValueReason.Other]), - }); state.reference(instrValue.value, Effect.Freeze, ValueReason.Other); const lvalue = instr.lvalue; lvalue.effect = Effect.ConditionallyMutate; - state.alias(lvalue, instrValue.value); + state.initialize(instrValue, { + kind: ValueKind.Immutable, + reason: new Set([ValueReason.Other]), + }); + state.define(lvalue, instrValue); continue; } case "LoadLocal": { diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts index 15380ed6bc..c62e2336a7 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -835,6 +835,8 @@ function codegenInstructionNullable( assertExhaustive(kind, `Unexpected instruction kind '${kind}'`); } } + } else if (instr.value.kind === "Memoize") { + return null; } else if (instr.value.kind === "Debugger") { return t.debuggerStatement(); } else if (instr.value.kind === "ObjectMethod") { @@ -1628,10 +1630,7 @@ function codegenInstructionValue( ); break; } - case "Memoize": { - value = codegenPlaceToExpression(cx, instrValue.value); - break; - } + case "Memoize": case "Debugger": case "DeclareLocal": case "DeclareContext": diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts index e97ae5e9e0..4dce513858 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts @@ -453,6 +453,7 @@ function computeMemoizationInputs( }; } case "NextPropertyOf": + case "Memoize": case "Debugger": case "ComputedDelete": case "PropertyDelete": @@ -471,7 +472,6 @@ function computeMemoizationInputs( rvalues: [], }; } - case "Memoize": case "Await": case "TypeCastExpression": case "NextIterableOf": { diff --git a/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts b/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts index c39abb1cde..7708948427 100644 --- a/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts +++ b/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts @@ -280,11 +280,6 @@ function* generateInstructionTypes( break; } - case "Memoize": { - yield equation(left, value.value.identifier.type); - break; - } - case "PropertyDelete": case "ComputedDelete": { yield equation(left, { kind: "Primitive" }); @@ -319,7 +314,9 @@ function* generateInstructionTypes( case "NextIterableOf": case "UnsupportedNode": case "Debugger": + case "Memoize": { break; + } default: assertExhaustive(value, `Unhandled instruction value kind: ${value}`); } diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-mabye-modified-free-variable-preserve-memoization-guarantees.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-mabye-modified-free-variable-preserve-memoization-guarantees.expect.md index 74056b4b88..05fa49c84a 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-mabye-modified-free-variable-preserve-memoization-guarantees.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-mabye-modified-free-variable-preserve-memoization-guarantees.expect.md @@ -64,11 +64,7 @@ function Component(props) { const free2 = t1; const part = free2.part; useHook(); - - props.value; - free; - part; - let t44; + let t39; let x; if ($[2] !== props.value) { x = makeObject_Primitives(); @@ -79,8 +75,8 @@ function Component(props) { } else { x = $[3]; } - t44 = x; - const object = t44; + t39 = x; + const object = t39; return object; } diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-maybe-modified-later-preserve-memoization-guarantees.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-maybe-modified-later-preserve-memoization-guarantees.expect.md index d652d52c94..e2ab6a86bf 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-maybe-modified-later-preserve-memoization-guarantees.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-maybe-modified-later-preserve-memoization-guarantees.expect.md @@ -28,7 +28,7 @@ import { identity, makeObject_Primitives, mutate } from "shared-runtime"; function Component(props) { const $ = useMemoCache(1); - let t15; + let t7; let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = makeObject_Primitives(); @@ -36,8 +36,8 @@ function Component(props) { } else { t0 = $[0]; } - t15 = t0; - const object = t15; + t7 = t0; + const object = t7; identity(object); return object; }