[ssa] Patch: propagate every rewrite to function expressions
--- #1899 only propagated eliminated phi nodes to function expressions on the first iteration through blocks. However, this was buggy as later iterations could introduce rewrites that need to be propagated. [playground repro](https://0xeac7-forget.vercel.app/#eyJzb3VyY2UiOiJmdW5jdGlvbiBDb21wb25lbnQoKSB7XG4gIGNvbnN0IHggPSA0O1xuXG4gIGNvbnN0IGdldDQgPSAoKSA9PiB7XG4gICAgd2hpbGUgKGJhcigpKSB7XG4gICAgICBpZiAoYmF6KSB7XG4gICAgICAgIGJhcigpO1xuICAgICAgfVxuICAgIH1cbiAgICByZXR1cm4gKCkgPT4geDtcbiAgfTtcblxuICByZXR1cm4gZ2V0NDtcbn0ifQ==). I manually synced #1907 to check that this fix works for the VR Store codebase.
Mofei Zhang committed
Aug 7, 2023 at 17:41 UTC
e33c9c43ccd36113f2d85d700f231679c7068612
4 files changed
+45
-31
compiler/forget/packages/babel-plugin-react-forget/src/SSA/EliminateRedundantPhi.ts
+4
-5
@@ -46,7 +46,6 @@ export function eliminateRedundantPhi(
46
// compare to see if any new rewrites were added in that iteration.
47
let size = rewrites.size;
48
do {
49
- const isFirstIteration = !hasBackEdge;
49
size = rewrites.size;
50
for (const [blockId, block] of ir.blocks) {
51
// On the first iteration of the loop check for any back-edges.
@@ -116,10 +115,10 @@ export function eliminateRedundantPhi(
115
rewritePlace(place, rewrites);
116
}
117
119
- // visit function expressions on first iteration of each block
120
- if (isFirstIteration) {
121
- eliminateRedundantPhi(instr.value.loweredFunc, rewrites);
122
- }
118
+ // recursive call to:
119
+ // - eliminate phi nodes in child node
120
+ // - propagate rewrites, which may have changed between iterations
121
+ eliminateRedundantPhi(instr.value.loweredFunc, rewrites);
122
}
123
}
124
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/e2e/constant-prop.e2e.js
+5
@@ -96,6 +96,11 @@ test("lambda-constant-propagation-of-phi-node", () => {
96
if (constantValue) {
97
noopCallback();
98
}
99
+ for (let i = 0; i < 5; i++) {
100
+ if (!constantValue) {
101
+ noopCallback();
102
+ }
103
+ }
104
const getDiv = () => <div>{x}</div>;
105
return getDiv();
106
}
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rewrite-phis-in-lambda-capture-context.expect.md
+25
-21
@@ -2,13 +2,19 @@
2
## Input
3
4
```javascript
5
-function ConstantPropagationBug() {
6
- const x = CONSTANT1;
7
- const createPhiNode = CONSTANT2 || 5;
8
-
9
- const getFoo = () => <Foo x={x} y={createPhiNode} />;
10
-
11
- return getFoo();
5
+function Component() {
6
+ const x = 4;
7
+
8
+ const get4 = () => {
9
+ while (bar()) {
10
+ if (baz) {
11
+ bar();
12
+ }
13
+ }
14
+ return () => x;
15
+ };
16
+
17
+ return get4;
18
}
19
20
```
@@ -17,26 +23,24 @@ function ConstantPropagationBug() {
23
24
```javascript
25
import { unstable_useMemoCache as useMemoCache } from "react";
20
-function ConstantPropagationBug() {
21
- const $ = useMemoCache(2);
22
-
23
- const createPhiNode = CONSTANT2 || 5;
26
+function Component() {
27
+ const $ = useMemoCache(1);
28
let t0;
29
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
26
- t0 = () => <Foo x={CONSTANT1} y={createPhiNode} />;
30
+ t0 = () => {
31
+ while (bar()) {
32
+ if (baz) {
33
+ bar();
34
+ }
35
+ }
36
+ return () => 4;
37
+ };
38
$[0] = t0;
39
} else {
40
t0 = $[0];
41
}
31
- const getFoo = t0;
32
- let t1;
33
- if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
34
- t1 = getFoo();
35
- $[1] = t1;
36
- } else {
37
- t1 = $[1];
38
- }
39
- return t1;
42
+ const get4 = t0;
43
+ return get4;
44
}
45
46
```
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/rewrite-phis-in-lambda-capture-context.js
+11
-5
@@ -1,8 +1,14 @@
1
-function ConstantPropagationBug() {
2
- const x = CONSTANT1;
3
- const createPhiNode = CONSTANT2 || 5;
1
+function Component() {
2
+ const x = 4;
3
5
- const getFoo = () => <Foo x={x} y={createPhiNode} />;
4
+ const get4 = () => {
5
+ while (bar()) {
6
+ if (baz) {
7
+ bar();
8
+ }
9
+ }
10
+ return () => x;
11
+ };
12
7
- return getFoo();
13
+ return get4;
14
}