From 241b35463c662a9da230b511dc161fd9aa943634 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 21 Mar 2024 14:11:04 -0700 Subject: [PATCH] Fix PruneMaybeThrows to update phi operand predecessor ids When PruneMaybeThrows removes maybe-throw terminals, it's possible that the block in question reassigned a value s.t. it appears as a later phi operand. That phi has to be rewritten to reflect the updated predecessor block. Here we track these rewrites (transitively) and rewrite phi operands accordingly. --- .../src/Optimization/PruneMaybeThrows.ts | 45 +++++++------ .../compiler/error.repro-preds-undefined.js | 13 ---- ...fined-try-catch-return-primitive.expect.md | 64 +++++++++++++++++++ ...s-undefined-try-catch-return-primitive.js} | 22 +++---- 4 files changed, 96 insertions(+), 48 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.repro-preds-undefined.js create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-preds-undefined-try-catch-return-primitive.expect.md rename compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/{error.repro-preds-undefined.expect.md => repro-preds-undefined-try-catch-return-primitive.js} (68%) diff --git a/compiler/packages/babel-plugin-react-forget/src/Optimization/PruneMaybeThrows.ts b/compiler/packages/babel-plugin-react-forget/src/Optimization/PruneMaybeThrows.ts index 0f8a2321fc..c39d2a04c3 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Optimization/PruneMaybeThrows.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Optimization/PruneMaybeThrows.ts @@ -5,7 +5,10 @@ * LICENSE file in the root directory of this source tree. */ +import { CompilerError } from ".."; import { + BlockId, + GeneratedSource, GotoVariant, HIRFunction, Instruction, @@ -17,12 +20,11 @@ import { } from "../HIR"; import { markInstructionIds, - markPredecessors, removeDeadDoWhileStatements, removeUnnecessaryTryCatch, removeUnreachableForUpdates, } from "../HIR/HIRBuilder"; -import { eliminateRedundantPhi } from "../SSA"; +import { printIdentifier } from "../HIR/PrintHIR"; /* * This pass prunes `maybe-throw` terminals for blocks that can provably *never* throw. @@ -30,8 +32,8 @@ import { eliminateRedundantPhi } from "../SSA"; * array/object literals. Even a variable reference could throw bc of the TDZ. */ export function pruneMaybeThrows(fn: HIRFunction): void { - const didPrune = pruneMaybeThrowsImpl(fn); - if (didPrune) { + const terminalMapping = pruneMaybeThrowsImpl(fn); + if (terminalMapping) { /* * If terminals have changed then blocks may have become newly unreachable. * Re-run minification of the graph (incl reordering instruction ids) @@ -42,36 +44,36 @@ export function pruneMaybeThrows(fn: HIRFunction): void { removeDeadDoWhileStatements(fn.body); removeUnnecessaryTryCatch(fn.body); markInstructionIds(fn.body); - markPredecessors(fn.body); + mergeConsecutiveBlocks(fn); - // Now that predecessors are updated, prune phi operands that can never be reached + // Rewrite phi operands to reference the updated predecessor blocks for (const [, block] of fn.body.blocks) { for (const phi of block.phis) { - for (const [predecessor] of phi.operands) { + for (const [predecessor, operand] of phi.operands) { if (!block.preds.has(predecessor)) { + const mappedTerminal = terminalMapping.get(predecessor); + CompilerError.invariant(mappedTerminal != null, { + reason: `Expected non-existing phi operand's predecessor to have been mapped to a new terminal`, + loc: GeneratedSource, + description: `Could not find mapping for predecessor bb${predecessor} in block bb${ + block.id + } for phi ${printIdentifier(phi.id)}`, + suggestions: null, + }); phi.operands.delete(predecessor); + phi.operands.set(mappedTerminal, operand); } } } } - /* - * By removing some phi operands, there may be phis that were not previously - * redundant but now are - */ - eliminateRedundantPhi(fn); - /* - * Finally, merge together any blocks that are now guaranteed to execute - * consecutively - */ - mergeConsecutiveBlocks(fn); assertConsistentIdentifiers(fn); assertTerminalSuccessorsExist(fn); } } -function pruneMaybeThrowsImpl(fn: HIRFunction): boolean { - let hasChanges = false; +function pruneMaybeThrowsImpl(fn: HIRFunction): Map | null { + const terminalMapping = new Map(); for (const [_, block] of fn.body.blocks) { const terminal = block.terminal; if (terminal.kind !== "maybe-throw") { @@ -81,7 +83,8 @@ function pruneMaybeThrowsImpl(fn: HIRFunction): boolean { instructionMayThrow(instr) ); if (!canThrow) { - hasChanges = true; + const source = terminalMapping.get(block.id) ?? block.id; + terminalMapping.set(terminal.continuation, source); block.terminal = { kind: "goto", block: terminal.continuation, @@ -91,7 +94,7 @@ function pruneMaybeThrowsImpl(fn: HIRFunction): boolean { }; } } - return hasChanges; + return terminalMapping.size > 0 ? terminalMapping : null; } function instructionMayThrow(instr: Instruction): boolean { diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.repro-preds-undefined.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.repro-preds-undefined.js deleted file mode 100644 index a9500cb531..0000000000 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.repro-preds-undefined.js +++ /dev/null @@ -1,13 +0,0 @@ -// @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions -function useSupportsTouchEvent() { - return useMemo(() => { - if (checkforTouchEvents) { - try { - document.createEvent("TouchEvent"); - return true; - } catch { - return false; - } - } - }, []); -} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-preds-undefined-try-catch-return-primitive.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-preds-undefined-try-catch-return-primitive.expect.md new file mode 100644 index 0000000000..9210aac5d4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-preds-undefined-try-catch-return-primitive.expect.md @@ -0,0 +1,64 @@ + +## Input + +```javascript +// @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions + +import { useMemo } from "react"; + +const checkforTouchEvents = true; +function useSupportsTouchEvent() { + return useMemo(() => { + if (checkforTouchEvents) { + try { + document.createEvent("TouchEvent"); + return true; + } catch { + return false; + } + } + }, []); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useSupportsTouchEvent, + params: [], +}; + +``` + +## Code + +```javascript +// @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions + +import { useMemo } from "react"; + +const checkforTouchEvents = true; +function useSupportsTouchEvent() { + let t0; + bb15: { + if (checkforTouchEvents) { + try { + document.createEvent("TouchEvent"); + t0 = true; + break bb15; + } catch { + t0 = false; + break bb15; + } + } + t0 = undefined; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useSupportsTouchEvent, + params: [], +}; + +``` + +### Eval output +(kind: ok) true \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.repro-preds-undefined.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-preds-undefined-try-catch-return-primitive.js similarity index 68% rename from compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.repro-preds-undefined.expect.md rename to compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-preds-undefined-try-catch-return-primitive.js index 587da7fd0c..a092ff44e0 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.repro-preds-undefined.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-preds-undefined-try-catch-return-primitive.js @@ -1,8 +1,8 @@ - -## Input - -```javascript // @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions + +import { useMemo } from "react"; + +const checkforTouchEvents = true; function useSupportsTouchEvent() { return useMemo(() => { if (checkforTouchEvents) { @@ -16,13 +16,7 @@ function useSupportsTouchEvent() { }, []); } -``` - - -## Error - -``` -Cannot read properties of undefined (reading 'preds') -``` - - \ No newline at end of file +export const FIXTURE_ENTRYPOINT = { + fn: useSupportsTouchEvent, + params: [], +};