[compiler] Allow more instruction kinds btw merged scopes

Updates our scope merging pass to allow more types of instructions to intervene btw scopes. This includes all the non-allocating kinds of nodes that are considered reorderable in #29863. It's already safe to merge scopes with these instructions — we only merge if the lvalue is not used past the next scope. Additionally, without changing this pass reordering isn't very effective, since we would reorder to add these types of intervening instructions and then not be able to merge scopes. Sequencing this first helps to see the win just from reordering alone. ghstack-source-id: 79263576d8eaeb45ef4d1ec4951478459853a287 Pull Request resolved: #29881
facebook · Jun 14, 2024 · ed8dd20 · ed8dd20
1 parent 88959fd
commit ed8dd20
Show file tree

Hide file tree

Showing 27 changed files with 217 additions and 411 deletions.
diff --git a/...bel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts b/...bel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts
@@ -186,11 +186,15 @@ class Transform extends ReactiveFunctionTransform<ReactiveScopeDependencies | nu
         }
         case "instruction": {
           switch (instr.instruction.value.kind) {
+            case "BinaryExpression":
             case "ComputedLoad":
             case "JSXText":
+            case "LoadGlobal":
             case "LoadLocal":
             case "Primitive":
-            case "PropertyLoad": {
+            case "PropertyLoad":
+            case "TemplateLiteral":
+            case "UnaryExpression": {
               /*
                * We can merge two scopes if there are intervening instructions, but:
                * - Only if the instructions are simple and it's okay to make them

diff --git a/...ixtures/compiler/allow-global-mutation-in-effect-indirect-usecallback.expect.md b/...ixtures/compiler/allow-global-mutation-in-effect-indirect-usecallback.expect.md
@@ -67,19 +67,16 @@ function Component() {
   }
   useEffect(t1, t2);
   let t3;
+  let t4;
   if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
     t3 = () => {
       setState(someGlobal.value);
     };
-    $[3] = t3;
-  } else {
-    t3 = $[3];
-  }
-  let t4;
-  if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
     t4 = [someGlobal];
+    $[3] = t3;
     $[4] = t4;
   } else {
+    t3 = $[3];
     t4 = $[4];
   }
   useEffect(t3, t4);

diff --git a/.../__tests__/fixtures/compiler/allow-global-mutation-in-effect-indirect.expect.md b/.../__tests__/fixtures/compiler/allow-global-mutation-in-effect-indirect.expect.md
@@ -66,19 +66,16 @@ function Component() {
   }
   useEffect(t1, t2);
   let t3;
+  let t4;
   if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
     t3 = () => {
       setState(someGlobal.value);
     };
-    $[3] = t3;
-  } else {
-    t3 = $[3];
-  }
-  let t4;
-  if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
     t4 = [someGlobal];
+    $[3] = t3;
     $[4] = t4;
   } else {
+    t3 = $[3];
     t4 = $[4];
   }
   useEffect(t3, t4);

diff --git a/...ests__/fixtures/compiler/allow-global-reassignment-in-effect-indirect.expect.md b/...ests__/fixtures/compiler/allow-global-reassignment-in-effect-indirect.expect.md
@@ -66,19 +66,16 @@ function Component() {
   }
   useEffect(t1, t2);
   let t3;
+  let t4;
   if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
     t3 = () => {
       setState(someGlobal);
     };
-    $[3] = t3;
-  } else {
-    t3 = $[3];
-  }
-  let t4;
-  if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
     t4 = [someGlobal];
+    $[3] = t3;
     $[4] = t4;
   } else {
+    t3 = $[3];
     t4 = $[4];
   }
   useEffect(t3, t4);

diff --git a/...r/src/__tests__/fixtures/compiler/allow-global-reassignment-in-effect.expect.md b/...r/src/__tests__/fixtures/compiler/allow-global-reassignment-in-effect.expect.md
@@ -54,19 +54,16 @@ function Component() {
   }
   useEffect(t0, t1);
   let t2;
+  let t3;
   if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
     t2 = () => {
       setState(someGlobal);
     };
-    $[2] = t2;
-  } else {
-    t2 = $[2];
-  }
-  let t3;
-  if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
     t3 = [someGlobal];
+    $[2] = t2;
     $[3] = t3;
   } else {
+    t2 = $[2];
     t3 = $[3];
   }
   useEffect(t2, t3);

diff --git a/...rc/__tests__/fixtures/compiler/allow-mutate-global-in-effect-fixpoint.expect.md b/...rc/__tests__/fixtures/compiler/allow-mutate-global-in-effect-fixpoint.expect.md
@@ -70,19 +70,16 @@ function Component() {
   }
   useEffect(t0, t1);
   let t2;
+  let t3;
   if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
     t2 = () => {
       setState(someGlobal.value);
     };
-    $[2] = t2;
-  } else {
-    t2 = $[2];
-  }
-  let t3;
-  if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
     t3 = [someGlobal];
+    $[2] = t2;
     $[3] = t3;
   } else {
+    t2 = $[2];
     t3 = $[3];
   }
   useEffect(t2, t3);

diff --git a/...gin-react-compiler/src/__tests__/fixtures/compiler/concise-arrow-expr.expect.md b/...gin-react-compiler/src/__tests__/fixtures/compiler/concise-arrow-expr.expect.md
@@ -15,24 +15,17 @@ function component() {
 ```javascript
 import { c as _c } from "react/compiler-runtime";
 function component() {
-  const $ = _c(2);
+  const $ = _c(1);
   const [x, setX] = useState(0);
   let t0;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    t0 = (v) => setX(v);
+    const handler = (v) => setX(v);
+    t0 = <Foo handler={handler} />;
     $[0] = t0;
   } else {
     t0 = $[0];
   }
-  const handler = t0;
-  let t1;
-  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
-    t1 = <Foo handler={handler} />;
-    $[1] = t1;
-  } else {
-    t1 = $[1];
-  }
-  return t1;
+  return t0;
 }
 
 ```

diff --git a/...react-compiler/src/__tests__/fixtures/compiler/conditional-on-mutable.expect.md b/...react-compiler/src/__tests__/fixtures/compiler/conditional-on-mutable.expect.md
@@ -36,65 +36,45 @@ function mayMutate() {}
 ```javascript
 import { c as _c } from "react/compiler-runtime";
 function ComponentA(props) {
-  const $ = _c(6);
-  let a;
-  let b;
+  const $ = _c(2);
+  let t0;
   if ($[0] !== props) {
-    a = [];
-    b = [];
+    const a = [];
+    const b = [];
     if (b) {
       a.push(props.p0);
     }
     if (props.p1) {
       b.push(props.p2);
     }
-    $[0] = props;
-    $[1] = a;
-    $[2] = b;
-  } else {
-    a = $[1];
-    b = $[2];
-  }
-  let t0;
-  if ($[3] !== a || $[4] !== b) {
+
     t0 = <Foo a={a} b={b} />;
-    $[3] = a;
-    $[4] = b;
-    $[5] = t0;
+    $[0] = props;
+    $[1] = t0;
   } else {
-    t0 = $[5];
+    t0 = $[1];
   }
   return t0;
 }
 
 function ComponentB(props) {
-  const $ = _c(6);
-  let a;
-  let b;
+  const $ = _c(2);
+  let t0;
   if ($[0] !== props) {
-    a = [];
-    b = [];
+    const a = [];
+    const b = [];
     if (mayMutate(b)) {
       a.push(props.p0);
     }
     if (props.p1) {
       b.push(props.p2);
     }
-    $[0] = props;
-    $[1] = a;
-    $[2] = b;
-  } else {
-    a = $[1];
-    b = $[2];
-  }
-  let t0;
-  if ($[3] !== a || $[4] !== b) {
+
     t0 = <Foo a={a} b={b} />;
-    $[3] = a;
-    $[4] = b;
-    $[5] = t0;
+    $[0] = props;
+    $[1] = t0;
   } else {
-    t0 = $[5];
+    t0 = $[1];
   }
   return t0;
 }

diff --git a/...ts__/fixtures/compiler/constant-propagation-into-function-expressions.expect.md b/...ts__/fixtures/compiler/constant-propagation-into-function-expressions.expect.md
@@ -17,25 +17,19 @@ function Component(props) {
 ```javascript
 import { c as _c } from "react/compiler-runtime";
 function Component(props) {
-  const $ = _c(2);
+  const $ = _c(1);
   let t0;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    t0 = () => {
+    const onEvent = () => {
       console.log(42);
     };
+
+    t0 = <Foo onEvent={onEvent} />;
     $[0] = t0;
   } else {
     t0 = $[0];
   }
-  const onEvent = t0;
-  let t1;
-  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
-    t1 = <Foo onEvent={onEvent} />;
-    $[1] = t1;
-  } else {
-    t1 = $[1];
-  }
-  return t1;
+  return t0;
 }
 
 ```

diff --git a/...sts__/fixtures/compiler/context-variable-reassigned-outside-of-lambda.expect.md b/...sts__/fixtures/compiler/context-variable-reassigned-outside-of-lambda.expect.md
@@ -27,26 +27,20 @@ import { c as _c } from "react/compiler-runtime";
 import { Stringify } from "shared-runtime";
 
 function Component(props) {
-  const $ = _c(2);
-  let callback;
+  const $ = _c(1);
+  let t0;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
     let x;
     x = null;
-    callback = () => {
+    const callback = () => {
       console.log(x);
     };
 
     x = {};
-    $[0] = callback;
-  } else {
-    callback = $[0];
-  }
-  let t0;
-  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
     t0 = <Stringify callback={callback} shouldInvokeFns={true} />;
-    $[1] = t0;
+    $[0] = t0;
   } else {
-    t0 = $[1];
+    t0 = $[0];
   }
   return t0;
 }

diff --git a/..._/fixtures/compiler/declare-reassign-variable-in-function-declaration.expect.md b/..._/fixtures/compiler/declare-reassign-variable-in-function-declaration.expect.md
@@ -18,7 +18,7 @@ function Component() {
 ```javascript
 import { c as _c } from "react/compiler-runtime";
 function Component() {
-  const $ = _c(2);
+  const $ = _c(1);
   let t0;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
     let x;
@@ -27,20 +27,13 @@ function Component() {
       x = 9;
     };
 
-    t0 = bar(foo);
+    const y = bar(foo);
+    t0 = <Child y={y} />;
     $[0] = t0;
   } else {
     t0 = $[0];
   }
-  const y = t0;
-  let t1;
-  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
-    t1 = <Child y={y} />;
-    $[1] = t1;
-  } else {
-    t1 = $[1];
-  }
-  return t1;
+  return t0;
 }
 
 ```