Update on "[compiler] Instruction reordering"

Adds a pass just after DCE to reorder safely reorderable instructions (jsx, primitives, globals) closer to where they are used, to allow other optimization passes to be more effective. Notably, the reordering allows scope merging to be more effective, since that pass relies on two scopes not having intervening instructions — in many cases we can now reorder such instructions out of the way and unlock merging, as demonstrated in the changed fixtures.

The algorithm itself is described in the docblock.

note: This is a cleaned up version of #29579 that is ready for review.

[ghstack-poisoned]

Author: josephsavona

compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

@@ -269,7 +269,7 @@ const EnvironmentConfigSchema = z.object({
    * Enable instruction reordering. See InstructionReordering.ts for the details
    * of the approach.
    */
-  enableInstructionReordering: z.boolean().default(true),
+  enableInstructionReordering: z.boolean().default(false),
 
   /*
    * Enables instrumentation codegen. This emits a dev-mode only call to an

compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts

@@ -350,7 +350,7 @@ export function isStatementBlockKind(kind: BlockKind): boolean {
  * Returns true for "value", "loop", and "sequence" block kinds which correspond to
  * expressions in the source, such as ConditionalExpression, LogicalExpression, loop
  * initializer/test/updaters, etc
-
+ *
  * Inverse of isStatementBlockKind()
  */
 export function isExpressionBlockKind(kind: BlockKind): boolean {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/align-scopes-within-nested-valueblock-in-array.expect.md

@@ -52,12 +52,11 @@ import { Stringify, identity, makeArray, mutate } from "shared-runtime";
  * handles this correctly.
  */
 function Foo(t0) {
-  const $ = _c(3);
+  const $ = _c(4);
   const { cond1, cond2 } = t0;
+  const arr = makeArray({ a: 2 }, 2, []);
   let t1;
-  if ($[0] !== cond1 || $[1] !== cond2) {
-    const arr = makeArray({ a: 2 }, 2, []);
-
+  if ($[0] !== cond1 || $[1] !== cond2 || $[2] !== arr) {
     t1 = cond1 ? (
       <>
         <div>{identity("foo")}</div>
@@ -66,9 +65,10 @@ function Foo(t0) {
     ) : null;
     $[0] = cond1;
     $[1] = cond2;
-    $[2] = t1;
+    $[2] = arr;
+    $[3] = t1;
   } else {
-    t1 = $[2];
+    t1 = $[3];
   }
   return t1;
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-global-mutation-in-effect-indirect.expect.md

@@ -39,51 +39,57 @@ import { useEffect, useState } from "react";
 let someGlobal = {};
 
 function Component() {
-  const $ = _c(6);
+  const $ = _c(7);
   const [state, setState] = useState(someGlobal);
   let t0;
-  let t1;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    const setGlobal = () => {
+    t0 = () => {
       someGlobal.value = true;
     };
-
-    t0 = () => {
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  const setGlobal = t0;
+  let t1;
+  let t2;
+  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+    t1 = () => {
       setGlobal();
     };
-    t1 = [];
-    $[0] = t0;
+    t2 = [];
     $[1] = t1;
+    $[2] = t2;
   } else {
-    t0 = $[0];
     t1 = $[1];
+    t2 = $[2];
   }
-  useEffect(t0, t1);
-  let t2;
+  useEffect(t1, t2);
   let t3;
-  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
-    t2 = () => {
+  let t4;
+  if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
+    t3 = () => {
       setState(someGlobal.value);
     };
-    t3 = [someGlobal];
-    $[2] = t2;
+    t4 = [someGlobal];
     $[3] = t3;
+    $[4] = t4;
   } else {
-    t2 = $[2];
     t3 = $[3];
+    t4 = $[4];
   }
-  useEffect(t2, t3);
+  useEffect(t3, t4);
 
-  const t4 = String(state);
-  let t5;
-  if ($[4] !== t4) {
-    t5 = <div>{t4}</div>;
-    $[4] = t4;
+  const t5 = String(state);
+  let t6;
+  if ($[5] !== t5) {
+    t6 = <div>{t5}</div>;
     $[5] = t5;
+    $[6] = t6;
   } else {
-    t5 = $[5];
+    t6 = $[6];
   }
-  return t5;
+  return t6;
 }
 
 export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-global-reassignment-in-effect-indirect.expect.md

@@ -39,51 +39,57 @@ import { useEffect, useState } from "react";
 let someGlobal = false;
 
 function Component() {
-  const $ = _c(6);
+  const $ = _c(7);
   const [state, setState] = useState(someGlobal);
   let t0;
-  let t1;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    const setGlobal = () => {
+    t0 = () => {
       someGlobal = true;
     };
-
-    t0 = () => {
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  const setGlobal = t0;
+  let t1;
+  let t2;
+  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+    t1 = () => {
       setGlobal();
     };
-    t1 = [];
-    $[0] = t0;
+    t2 = [];
     $[1] = t1;
+    $[2] = t2;
   } else {
-    t0 = $[0];
     t1 = $[1];
+    t2 = $[2];
   }
-  useEffect(t0, t1);
-  let t2;
+  useEffect(t1, t2);
   let t3;
-  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
-    t2 = () => {
+  let t4;
+  if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
+    t3 = () => {
       setState(someGlobal);
     };
-    t3 = [someGlobal];
-    $[2] = t2;
+    t4 = [someGlobal];
     $[3] = t3;
+    $[4] = t4;
   } else {
-    t2 = $[2];
     t3 = $[3];
+    t4 = $[4];
   }
-  useEffect(t2, t3);
+  useEffect(t3, t4);
 
-  const t4 = String(state);
-  let t5;
-  if ($[4] !== t4) {
-    t5 = <div>{t4}</div>;
-    $[4] = t4;
+  const t5 = String(state);
+  let t6;
+  if ($[5] !== t5) {
+    t6 = <div>{t5}</div>;
     $[5] = t5;
+    $[6] = t6;
   } else {
-    t5 = $[5];
+    t6 = $[6];
   }
-  return t5;
+  return t6;
 }
 
 export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-merge-if-dep-is-inner-declaration-of-previous-scope.expect.md

@@ -99,41 +99,41 @@ function Component(t0) {
     t2 = $[12];
   }
   let t3;
-  if ($[13] !== a || $[14] !== b) {
-    t3 = [a, b];
-    $[13] = a;
-    $[14] = b;
+  if ($[13] !== t2 || $[14] !== x) {
+    t3 = <ValidateMemoization inputs={t2} output={x} />;
+    $[13] = t2;
+    $[14] = x;
     $[15] = t3;
   } else {
     t3 = $[15];
   }
   let t4;
-  if ($[16] !== t2 || $[17] !== x) {
-    t4 = <ValidateMemoization inputs={t2} output={x} />;
-    $[16] = t2;
-    $[17] = x;
+  if ($[16] !== a || $[17] !== b) {
+    t4 = [a, b];
+    $[16] = a;
+    $[17] = b;
     $[18] = t4;
   } else {
     t4 = $[18];
   }
   let t5;
-  if ($[19] !== t3 || $[20] !== z) {
-    t5 = <ValidateMemoization inputs={t3} output={z} />;
-    $[19] = t3;
+  if ($[19] !== t4 || $[20] !== z) {
+    t5 = <ValidateMemoization inputs={t4} output={z} />;
+    $[19] = t4;
     $[20] = z;
     $[21] = t5;
   } else {
     t5 = $[21];
   }
   let t6;
-  if ($[22] !== t4 || $[23] !== t5) {
+  if ($[22] !== t3 || $[23] !== t5) {
     t6 = (
       <>
-        {t4}
+        {t3}
         {t5}
       </>
     );
-    $[22] = t4;
+    $[22] = t3;
     $[23] = t5;
     $[24] = t6;
   } else {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-repro-trycatch-nested-overlapping-range.expect.md

@@ -20,7 +20,7 @@ function Foo() {
 ## Error
 
 ```
-Invariant: Invalid nesting in program blocks or scopes. Items overlap but are not nested: 2:23(17:25)
+Invariant: Invalid nesting in program blocks or scopes. Items overlap but are not nested: 2:24(18:26)
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.repro-bug-ref-mutable-range.expect.md

@@ -21,7 +21,7 @@ function Foo(props, ref) {
 ## Error
 
 ```
-Invariant: Invalid nesting in program blocks or scopes. Items overlap but are not nested: 1:20(16:23)
+Invariant: Invalid nesting in program blocks or scopes. Items overlap but are not nested: 1:21(16:23)
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-reactive-scope-overlaps-try.expect.md

@@ -30,7 +30,7 @@ export const FIXTURE_ENTRYPOINT = {
 ## Error
 
 ```
-Invariant: Invalid nesting in program blocks or scopes. Items overlap but are not nested: 3:17(4:20)
+Invariant: Invalid nesting in program blocks or scopes. Items overlap but are not nested: 4:19(5:22)
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbtparam-with-jsx-element-content.expect.md

@@ -32,7 +32,7 @@ function Component(t0) {
   const $ = _c(4);
   const { name, data, icon } = t0;
   let t1;
-  if ($[0] !== name || $[1] !== data || $[2] !== icon) {
+  if ($[0] !== name || $[1] !== icon || $[2] !== data) {
     t1 = (
       <Text type="body4">
         {fbt._(
@@ -62,8 +62,8 @@ function Component(t0) {
       </Text>
     );
     $[0] = name;
-    $[1] = data;
-    $[2] = icon;
+    $[1] = icon;
+    $[2] = data;
     $[3] = t1;
   } else {
     t1 = $[3];

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/lambda-with-fbt.expect.md

@@ -42,10 +42,10 @@ import { c as _c } from "react/compiler-runtime";
 import { fbt } from "fbt";
 
 function Component() {
-  const $ = _c(1);
+  const $ = _c(2);
   let t0;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    const buttonLabel = () => {
+    t0 = () => {
       if (!someCondition) {
         return fbt._("Purchase as a gift", null, { hk: "1gHj4g" });
       } else {
@@ -66,17 +66,23 @@ function Component() {
         }
       }
     };
-
-    t0 = (
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  const buttonLabel = t0;
+  let t1;
+  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+    t1 = (
       <View>
         <Button text={buttonLabel()} />
       </View>
     );
-    $[0] = t0;
+    $[1] = t1;
   } else {
-    t0 = $[0];
+    t1 = $[1];
   }
-  return t0;
+  return t1;
 }
 
 ```