[compiler] Maintain RPO and uniquely instruction ids when constructing scope terminals

Later passes may rely on HIR invariants such as blocks being in RPO or instructions having unique, ascending InstructionIds. However, BuildReactiveScopeTerminalsHIR doesn't currently gurantee this.

This PR updates that pass to first restore RPO, fixup predecessors (the previous logic tried to do this but failed on unreachable blocks, where `markPredecessors()` handles that case), and renumber instructions. Then it walks instructions and scopes to update identifier and scope ranges given the new instruction ids.

ghstack-source-id: 5a3e3dc
Pull Request resolved: #30399

Author: josephsavona

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

@@ -39,6 +39,7 @@ export function assertTerminalPredsExist(fn: HIRFunction): void {
         [...eachTerminalSuccessor(predBlock.terminal)].includes(block.id),
         {
           reason: 'Terminal successor does not reference correct predecessor',
+          description: `Block bb${block.id} has bb${predBlock.id} as a predecessor, but bb${predBlock.id}'s successors do not include bb${block.id}`,
           loc: GeneratedSource,
         },
       );

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

@@ -9,10 +9,22 @@ import {
   GotoVariant,
   HIRFunction,
   InstructionId,
+  makeInstructionId,
+  Place,
   ReactiveScope,
   ReactiveScopeTerminal,
   ScopeId,
 } from './HIR';
+import {
+  markInstructionIds,
+  markPredecessors,
+  reversePostorderBlocks,
+} from './HIRBuilder';
+import {
+  eachInstructionLValue,
+  eachInstructionValueOperand,
+  eachTerminalOperand,
+} from './visitors';
 
 /**
  * This pass assumes that all program blocks are properly nested with respect to fallthroughs
@@ -142,16 +154,9 @@ export function buildReactiveScopeTerminalsHIR(fn: HIRFunction): void {
 
   /**
    * Step 3:
-   * Repoint preds and phis when they refer to a rewritten block.
+   * Repoint phis when they refer to a rewritten block.
    */
   for (const [, block] of originalBlocks) {
-    for (const pred of block.preds) {
-      const newId = rewrittenFinalBlocks.get(pred);
-      if (newId != null) {
-        block.preds.delete(pred);
-        block.preds.add(newId);
-      }
-    }
     for (const phi of block.phis) {
       for (const [originalId, value] of phi.operands) {
         const newId = rewrittenFinalBlocks.get(originalId);
@@ -162,6 +167,50 @@ export function buildReactiveScopeTerminalsHIR(fn: HIRFunction): void {
       }
     }
   }
+
+  /**
+   * Step 4:
+   * Fixup the HIR to restore RPO, ensure correct predecessors, and
+   * renumber instructions. Note that the renumbering instructions
+   * invalidates scope and identifier ranges, so we fix them in the
+   * next step.
+   */
+  reversePostorderBlocks(fn.body);
+  markPredecessors(fn.body);
+  markInstructionIds(fn.body);
+
+  /**
+   * Step 5:
+   * Fix scope and identifier ranges to account for renumbered instructions
+   */
+  for (const [, block] of fn.body.blocks) {
+    for (const instruction of block.instructions) {
+      for (const lvalue of eachInstructionLValue(instruction)) {
+        // Any lvalues whose mutable range was a single instruction must have
+        // started at the current instruction, so update the range to match
+        // the instruction's new id
+        if (
+          lvalue.identifier.mutableRange.end ===
+          lvalue.identifier.mutableRange.start + 1
+        ) {
+          lvalue.identifier.mutableRange.start = instruction.id;
+          lvalue.identifier.mutableRange.end = makeInstructionId(
+            instruction.id + 1,
+          );
+        }
+      }
+    }
+    const terminal = block.terminal;
+    if (terminal.kind === 'scope' || terminal.kind === 'pruned-scope') {
+      // Scope ranges should always align to start at the 'scope' terminal
+      // and end at the first instruction of the fallthrough block
+      const fallthroughBlock = fn.body.blocks.get(terminal.fallthrough)!;
+      const firstId =
+        fallthroughBlock.instructions[0]?.id ?? fallthroughBlock.terminal.id;
+      terminal.scope.range.start = terminal.id;
+      terminal.scope.range.end = firstId;
+    }
+  }
 }
 
 type TerminalRewriteInfo =

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts

@@ -22,7 +22,7 @@ import {
   ScopeId,
   SourceLocation,
 } from '../HIR';
-import {printManualMemoDependency} from '../HIR/PrintHIR';
+import {printManualMemoDependency, printPlace} from '../HIR/PrintHIR';
 import {eachInstructionValueOperand} from '../HIR/visitors';
 import {collectMaybeMemoDependencies} from '../Inference/DropManualMemoization';
 import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
@@ -470,7 +470,6 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
           state.errors.push({
             reason:
               'React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value may be mutated later, which could cause the value to change unexpectedly',
-            description: null,
             severity: ErrorSeverity.CannotPreserveMemoization,
             loc: typeof instruction.loc !== 'symbol' ? instruction.loc : null,
             suggestions: null,