Fix for invalid mutable range in phi with backedge

Fixes the repro added in 947832009997bf9149e88e583c46cc39f6a6136c - previously 
when computing mutable ranges of phis, we didn't check that all operands had 
been visited. This meant that a backedge could allow a phi's mutable range to 
start at 0. Then in PropagateScopeDeps, we might see reject dependencies of a 
scope since they appeared to start after a scope — only because the scope's 
start was incorrectly too early. 

The fix here is to initially set phi.id mutable ranges based on only on operands 
that are already visited. Then, during/after the fixpoint iteration of 
InferMutableRanges, we start account for all operands since we know they've been 
visited at least once and have a real range.
This commit is contained in:
Joe Savona
2024-03-25 21:36:33 -07:00
parent 48ecaf92d5
commit fbd68eef7f
5 changed files with 94 additions and 42 deletions
@@ -5,7 +5,6 @@
* LICENSE file in the root directory of this source tree.
*/
import { CompilerError } from "../CompilerError";
import {
Effect,
HIRFunction,
@@ -102,22 +101,30 @@ export function inferMutableLifetimes(
): void {
for (const [_, block] of func.body.blocks) {
for (const phi of block.phis) {
let start = Number.MAX_SAFE_INTEGER;
let end = phi.id.mutableRange.end as number;
for (const [_, operand] of phi.operands) {
start = Math.min(start, operand.mutableRange.start);
end = Math.max(end, operand.mutableRange.end);
if (
operand.mutableRange.start === 0 &&
operand.mutableRange.end === 0
) {
// operand's range is uninitialized, skip
continue;
} else if (
phi.id.mutableRange.start === 0 &&
phi.id.mutableRange.end === 0
) {
// phi's range is uninitialized, take the range from the operand
phi.id.mutableRange.start = operand.mutableRange.start;
phi.id.mutableRange.end = operand.mutableRange.end;
} else {
// else join the phi and operand's range
phi.id.mutableRange.start = makeInstructionId(
Math.min(phi.id.mutableRange.start, operand.mutableRange.start)
);
phi.id.mutableRange.end = makeInstructionId(
Math.max(phi.id.mutableRange.end, operand.mutableRange.end)
);
}
}
CompilerError.invariant(start !== Number.MAX_SAFE_INTEGER, {
reason: "Expected phi to have a start range value",
description: null,
loc: null,
suggestions: null,
});
phi.id.mutableRange = {
start: makeInstructionId(start),
end: makeInstructionId(end),
};
}
for (const instr of block.instructions) {
@@ -39,11 +39,11 @@ export const FIXTURE_ENTRYPOINT = {
import { unstable_useMemoCache as useMemoCache } from "react";
const someGlobal = true;
export default function Component(props) {
const $ = useMemoCache(1);
const $ = useMemoCache(4);
const { b } = props;
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
const items = [];
let items;
if ($[0] !== b) {
items = [];
let i = 0;
while (i < 10) {
if (someGlobal) {
@@ -51,11 +51,18 @@ export default function Component(props) {
i++;
}
}
t0 = <>{items}</>;
$[0] = t0;
$[0] = b;
$[1] = items;
} else {
t0 = $[0];
items = $[1];
}
let t0;
if ($[2] !== items) {
t0 = <>{items}</>;
$[2] = items;
$[3] = t0;
} else {
t0 = $[3];
}
return t0;
}
@@ -76,4 +83,13 @@ export const FIXTURE_ENTRYPOINT = {
};
```
### Eval output
(kind: ok) <div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div>
<div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div>
<div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div>
<div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div>
<div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div>
<div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div>
<div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div><div>0</div>
<div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div><div>42</div>
@@ -16,7 +16,17 @@ function Component(props) {
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ value: { a: "a", continue: "skip", b: "b!" } }],
params: [{ value: { a: "a", continue: "skip", b: "hello!" } }],
sequentialRenders: [
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "skip!", continue: true } },
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "skip!", continue: true } },
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "skip!", continue: true } },
{ value: { a: "skip!", continue: true } },
],
};
```
@@ -26,18 +36,10 @@ export const FIXTURE_ENTRYPOINT = {
```javascript
import { unstable_useMemoCache as useMemoCache } from "react";
function Component(props) {
const $ = useMemoCache(3);
const $ = useMemoCache(2);
let x;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
let t0;
if ($[1] !== props.value) {
t0 = { ...props.value };
$[1] = props.value;
$[2] = t0;
} else {
t0 = $[2];
}
const object = t0;
if ($[0] !== props.value) {
const object = { ...props.value };
for (const y in object) {
if (y === "continue") {
continue;
@@ -45,19 +47,37 @@ function Component(props) {
x = object[y];
}
$[0] = x;
$[0] = props.value;
$[1] = x;
} else {
x = $[0];
x = $[1];
}
return x;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ value: { a: "a", continue: "skip", b: "b!" } }],
params: [{ value: { a: "a", continue: "skip", b: "hello!" } }],
sequentialRenders: [
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "skip!", continue: true } },
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "skip!", continue: true } },
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "skip!", continue: true } },
{ value: { a: "skip!", continue: true } },
],
};
```
### Eval output
(kind: ok) "b!"
(kind: ok) "hello!"
"hello!"
"skip!"
"hello!"
"skip!"
"hello!"
"skip!"
"skip!"
@@ -12,5 +12,15 @@ function Component(props) {
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ value: { a: "a", continue: "skip", b: "b!" } }],
params: [{ value: { a: "a", continue: "skip", b: "hello!" } }],
sequentialRenders: [
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "skip!", continue: true } },
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "skip!", continue: true } },
{ value: { a: "a", continue: "skip", b: "hello!" } },
{ value: { a: "skip!", continue: true } },
{ value: { a: "skip!", continue: true } },
],
};
@@ -534,7 +534,6 @@ const skipFilter = new Set([
// bugs
"bug-reduce-reactive-deps-return-in-scope",
"bug-reduce-reactive-deps-break-in-scope",
"bug-repro-missing-dependency-if-within-while",
// 'react-forget-runtime' not yet supported
"flag-enable-emit-hook-guards",