From d35fef9e21100463b0bfecb975e9d9eca45c963f Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Tue, 8 Jul 2025 19:36:47 -0400 Subject: [PATCH] [compiler] Fix for consecutive DCE'd branches with phis (#33725) This is an optimized version of @asmjmp0's fix in https://github.com/facebook/react/pull/31940. When we merge consecutive blocks we need to take care to rewrite later phis whose operands will now be different blocks due to merging. Rather than iterate all the blocks on each merge as in #31940, we can do a single iteration over all the phis at the end to fix them up. Note: this is a redo of #31959 --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33725). * #33726 * __->__ #33725 --- .../src/HIR/MergeConsecutiveBlocks.ts | 11 ++++ ...ssing-phi-after-dce-merge-scopes.expect.md | 52 +++++++++++++++++++ ...epro-missing-phi-after-dce-merge-scopes.js | 20 +++++++ 3 files changed, 83 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-missing-phi-after-dce-merge-scopes.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-missing-phi-after-dce-merge-scopes.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/MergeConsecutiveBlocks.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/MergeConsecutiveBlocks.ts index 3d6ae4e6b2..881e4e93ff 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/MergeConsecutiveBlocks.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/MergeConsecutiveBlocks.ts @@ -107,6 +107,17 @@ export function mergeConsecutiveBlocks(fn: HIRFunction): void { merged.merge(block.id, predecessorId); fn.body.blocks.delete(block.id); } + for (const [, block] of fn.body.blocks) { + for (const phi of block.phis) { + for (const [predecessorId, operand] of phi.operands) { + const mapped = merged.get(predecessorId); + if (mapped !== predecessorId) { + phi.operands.delete(predecessorId); + phi.operands.set(mapped, operand); + } + } + } + } markPredecessors(fn.body); for (const [, {terminal}] of fn.body.blocks) { if (terminalHasFallthrough(terminal)) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-missing-phi-after-dce-merge-scopes.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-missing-phi-after-dce-merge-scopes.expect.md new file mode 100644 index 0000000000..da7220c73c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-missing-phi-after-dce-merge-scopes.expect.md @@ -0,0 +1,52 @@ + +## Input + +```javascript +function Component() { + let v3, v4, acc; + v3 = false; + v4 = v3; + acc = v3; + if (acc) { + acc = true; + v3 = acc; + } + if (acc) { + v3 = v4; + } + v4 = v3; + return [acc, v3, v4]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = [false, false, false]; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], +}; + +``` + +### Eval output +(kind: ok) [false,false,false] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-missing-phi-after-dce-merge-scopes.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-missing-phi-after-dce-merge-scopes.js new file mode 100644 index 0000000000..298ccbebaf --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-missing-phi-after-dce-merge-scopes.js @@ -0,0 +1,20 @@ +function Component() { + let v3, v4, acc; + v3 = false; + v4 = v3; + acc = v3; + if (acc) { + acc = true; + v3 = acc; + } + if (acc) { + v3 = v4; + } + v4 = v3; + return [acc, v3, v4]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], +};