From 4f80cbbfdc7c03faeed05ab351f8e8616de88171 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 22 Aug 2024 17:18:30 -0700 Subject: [PATCH] Update on "[compiler] Infer phi types, extend mutable ranges to account for Store effects" Redo of an earlier (pre-OSS) PR to infer types of phi nodes. There are a few pieces to this: 1. Update InferTypes to infer the type of `phi.id.type`, not the unused `phi.type`. 2. Update the algorithm to verify that all the phi types are actually equal, not just have the same kind. 3. Handle circular types by removing the cycle. However, that reveals another issue: InferMutableRanges currently infers the results of `Store` effects _after_ its fixpoint loop. That was fine when a Store could never occur on a phi (since they wouldn't have a type to get a function signature from). Now though, we can have Store effects occur on phis, and we need to ensure that this correctly updates the mutable range of the phi operands - recursively. See new test that fails without the fixpoint loop. [ghstack-poisoned] --- .../src/Inference/InferMutableRanges.ts | 3 +- .../compiler/array-push-effect.expect.md | 11 +++++-- ...e-if-nonexhaustive-poisoned-deps.expect.md | 15 ++++++--- ...-if-nonexhaustive-poisoned-deps1.expect.md | 31 ++++++++++++++----- ...e-if-exhaustive-nonpoisoned-deps.expect.md | 20 ++++++++---- ...-if-exhaustive-nonpoisoned-deps1.expect.md | 30 +++++++++++++----- ...duce-if-exhaustive-poisoned-deps.expect.md | 31 ++++++++++++++----- .../compiler/ssa-property-alias-if.expect.md | 15 ++++++--- .../try-catch-mutate-outer-value.expect.md | 11 +++++-- .../try-catch-within-mutable-range.expect.md | 20 ++++++++++-- 10 files changed, 141 insertions(+), 46 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts index 5a1b79770d..19f8ab5dc2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts @@ -50,11 +50,10 @@ export function inferMutableRanges(ir: HIRFunction): void { // Re-infer mutable ranges for all values inferMutableLifetimes(ir, true); - // Re-infer mutable ranges for aliases + // Re-infer mutable ranges for aliases, but *not* for stores prevAliases = aliases.canonicalize(); while (true) { inferMutableRangesForAlias(ir, aliases); - inferAliasForStores(ir, aliases); inferAliasForPhis(ir, aliases); const nextAliases = aliases.canonicalize(); if (areEqualMaps(prevAliases, nextAliases)) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-push-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-push-effect.expect.md index ac75c6b5ac..22cda38b1d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-push-effect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-push-effect.expect.md @@ -23,7 +23,7 @@ import { c as _c } from "react/compiler-runtime"; // arrayInstance.push should h // - read on all args (rest parameter) // - mutate on receiver function Component(props) { - const $ = _c(7); + const $ = _c(8); let t0; if ($[0] !== props.x) { t0 = foo(props.x); @@ -45,7 +45,14 @@ function Component(props) { let arr; if ($[4] !== x || $[5] !== y) { arr = []; - arr.push({}); + let t2; + if ($[7] === Symbol.for("react.memo_cache_sentinel")) { + t2 = {}; + $[7] = t2; + } else { + t2 = $[7]; + } + arr.push(t2); arr.push(x, y); $[4] = x; $[5] = y; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-poisoned/reduce-if-nonexhaustive-poisoned-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-poisoned/reduce-if-nonexhaustive-poisoned-deps.expect.md index 80c69f37f4..bb47587687 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-poisoned/reduce-if-nonexhaustive-poisoned-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-poisoned/reduce-if-nonexhaustive-poisoned-deps.expect.md @@ -40,7 +40,7 @@ import { c as _c } from "react/compiler-runtime"; import { identity } from "shared-runtime"; function useFoo(t0) { - const $ = _c(7); + const $ = _c(9); const { input, cond, hasAB } = t0; let x; let t1; @@ -53,9 +53,6 @@ function useFoo(t0) { t1 = null; break bb0; } - - x.push(identity(input.a.b)); - } else { let t2; if ($[5] !== input.a.b) { t2 = identity(input.a.b); @@ -65,6 +62,16 @@ function useFoo(t0) { t2 = $[6]; } x.push(t2); + } else { + let t2; + if ($[7] !== input.a.b) { + t2 = identity(input.a.b); + $[7] = input.a.b; + $[8] = t2; + } else { + t2 = $[8]; + } + x.push(t2); } } $[0] = cond; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-poisoned/reduce-if-nonexhaustive-poisoned-deps1.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-poisoned/reduce-if-nonexhaustive-poisoned-deps1.expect.md index 10bf34ecc2..59225b5155 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-poisoned/reduce-if-nonexhaustive-poisoned-deps1.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-poisoned/reduce-if-nonexhaustive-poisoned-deps1.expect.md @@ -41,7 +41,7 @@ import { c as _c } from "react/compiler-runtime"; import { identity } from "shared-runtime"; function useFoo(t0) { - const $ = _c(7); + const $ = _c(11); const { input, cond, hasAB } = t0; let x; let t1; @@ -54,18 +54,33 @@ function useFoo(t0) { t1 = null; break bb0; } else { - x.push(identity(input.a.b)); + let t2; + if ($[5] !== input.a.b) { + t2 = identity(input.a.b); + $[5] = input.a.b; + $[6] = t2; + } else { + t2 = $[6]; + } + x.push(t2); } - - x.push(identity(input.a.b)); + let t2; + if ($[7] !== input.a.b) { + t2 = identity(input.a.b); + $[7] = input.a.b; + $[8] = t2; + } else { + t2 = $[8]; + } + x.push(t2); } else { let t2; - if ($[5] !== input.a.b) { + if ($[9] !== input.a.b) { t2 = identity(input.a.b); - $[5] = input.a.b; - $[6] = t2; + $[9] = input.a.b; + $[10] = t2; } else { - t2 = $[6]; + t2 = $[10]; } x.push(t2); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/reduce-if-exhaustive-nonpoisoned-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/reduce-if-exhaustive-nonpoisoned-deps.expect.md index 63df548f02..84b8fa1d43 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/reduce-if-exhaustive-nonpoisoned-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/reduce-if-exhaustive-nonpoisoned-deps.expect.md @@ -31,7 +31,7 @@ import { c as _c } from "react/compiler-runtime"; import { identity } from "shared-runtime"; function useFoo(t0) { - const $ = _c(7); + const $ = _c(9); const { input, hasAB, returnNull } = t0; let x; let t1; @@ -40,19 +40,27 @@ function useFoo(t0) { bb0: { x = []; if (!hasAB) { - x.push(identity(input.a)); + let t2; + if ($[5] !== input.a) { + t2 = identity(input.a); + $[5] = input.a; + $[6] = t2; + } else { + t2 = $[6]; + } + x.push(t2); if (!returnNull) { t1 = null; break bb0; } } else { let t2; - if ($[5] !== input.a.b) { + if ($[7] !== input.a.b) { t2 = identity(input.a.b); - $[5] = input.a.b; - $[6] = t2; + $[7] = input.a.b; + $[8] = t2; } else { - t2 = $[6]; + t2 = $[8]; } x.push(t2); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/reduce-if-exhaustive-nonpoisoned-deps1.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/reduce-if-exhaustive-nonpoisoned-deps1.expect.md index 9ac2c76b65..a55922f610 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/reduce-if-exhaustive-nonpoisoned-deps1.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/reduce-if-exhaustive-nonpoisoned-deps1.expect.md @@ -41,7 +41,7 @@ import { c as _c } from "react/compiler-runtime"; import { identity } from "shared-runtime"; function useFoo(t0) { - const $ = _c(7); + const $ = _c(11); const { input, cond2, cond1 } = t0; let x; let t1; @@ -51,20 +51,36 @@ function useFoo(t0) { x = []; if (cond1) { if (!cond2) { - x.push(identity(input.a.b)); + let t2; + if ($[5] !== input.a.b) { + t2 = identity(input.a.b); + $[5] = input.a.b; + $[6] = t2; + } else { + t2 = $[6]; + } + x.push(t2); t1 = null; break bb0; } else { - x.push(identity(input.a.b)); + let t2; + if ($[7] !== input.a.b) { + t2 = identity(input.a.b); + $[7] = input.a.b; + $[8] = t2; + } else { + t2 = $[8]; + } + x.push(t2); } } else { let t2; - if ($[5] !== input.a.b) { + if ($[9] !== input.a.b) { t2 = identity(input.a.b); - $[5] = input.a.b; - $[6] = t2; + $[9] = input.a.b; + $[10] = t2; } else { - t2 = $[6]; + t2 = $[10]; } x.push(t2); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/reduce-if-exhaustive-poisoned-deps.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/reduce-if-exhaustive-poisoned-deps.expect.md index 6fff19422f..01ab4f6b0a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/reduce-if-exhaustive-poisoned-deps.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/reduce-if-exhaustive-poisoned-deps.expect.md @@ -32,7 +32,7 @@ import { c as _c } from "react/compiler-runtime"; import { identity } from "shared-runtime"; function useFoo(t0) { - const $ = _c(7); + const $ = _c(11); const { input, inputHasAB, inputHasABC } = t0; let x; let t1; @@ -41,21 +41,36 @@ function useFoo(t0) { bb0: { x = []; if (!inputHasABC) { - x.push(identity(input.a)); + let t2; + if ($[5] !== input.a) { + t2 = identity(input.a); + $[5] = input.a; + $[6] = t2; + } else { + t2 = $[6]; + } + x.push(t2); if (!inputHasAB) { t1 = null; break bb0; } - - x.push(identity(input.a.b)); + let t3; + if ($[7] !== input.a.b) { + t3 = identity(input.a.b); + $[7] = input.a.b; + $[8] = t3; + } else { + t3 = $[8]; + } + x.push(t3); } else { let t2; - if ($[5] !== input.a.b.c) { + if ($[9] !== input.a.b.c) { t2 = identity(input.a.b.c); - $[5] = input.a.b.c; - $[6] = t2; + $[9] = input.a.b.c; + $[10] = t2; } else { - t2 = $[6]; + t2 = $[10]; } x.push(t2); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-property-alias-if.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-property-alias-if.expect.md index 0bc9659483..a2e2948c43 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-property-alias-if.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/ssa-property-alias-if.expect.md @@ -27,14 +27,11 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function foo(a) { - const $ = _c(3); + const $ = _c(4); let x; if ($[0] !== a) { x = {}; if (a) { - const y = {}; - x.y = y; - } else { let t0; if ($[2] === Symbol.for("react.memo_cache_sentinel")) { t0 = {}; @@ -42,6 +39,16 @@ function foo(a) { } else { t0 = $[2]; } + const y = t0; + x.y = y; + } else { + let t0; + if ($[3] === Symbol.for("react.memo_cache_sentinel")) { + t0 = {}; + $[3] = t0; + } else { + t0 = $[3]; + } const z = t0; x.z = z; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-mutate-outer-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-mutate-outer-value.expect.md index 0bf6309fba..856d132640 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-mutate-outer-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-mutate-outer-value.expect.md @@ -28,12 +28,19 @@ import { c as _c } from "react/compiler-runtime"; const { shallowCopy, throwErrorWithMessage } = require("shared-runtime"); function Component(props) { - const $ = _c(2); + const $ = _c(3); let x; if ($[0] !== props.a) { x = []; try { - x.push(throwErrorWithMessage("oops")); + let t0; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t0 = throwErrorWithMessage("oops"); + $[2] = t0; + } else { + t0 = $[2]; + } + x.push(t0); } catch { x.push(shallowCopy({ a: props.a })); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-within-mutable-range.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-within-mutable-range.expect.md index a01819094e..2b4fab6fef 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-within-mutable-range.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/try-catch-within-mutable-range.expect.md @@ -29,14 +29,28 @@ import { c as _c } from "react/compiler-runtime"; const { throwErrorWithMessage, shallowCopy } = require("shared-runtime"); function Component(props) { - const $ = _c(2); + const $ = _c(4); let x; if ($[0] !== props.value) { x = []; try { - x.push(throwErrorWithMessage("oops")); + let t0; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t0 = throwErrorWithMessage("oops"); + $[2] = t0; + } else { + t0 = $[2]; + } + x.push(t0); } catch { - x.push(shallowCopy({})); + let t0; + if ($[3] === Symbol.for("react.memo_cache_sentinel")) { + t0 = shallowCopy({}); + $[3] = t0; + } else { + t0 = $[3]; + } + x.push(t0); } x.push(props.value);