BuildReactiveBlocks to construct scopes in ReactiveFunction

The primary goal of this stack is to change HIRTreeVisitor to make it easier to handle value blocks. That's complicated by the fact that the visitor is a general-purpose visitor, used in several analysis passes including BuildReactiveFunction (which translates HIR->ReactiveFunction while also grouping instructions into scopes) and InferReactiveScopes (which is actually two passes, one to align scopes to block boundaries, one to merge overlapping scopes). The long-term goal then is as follows: 1. Make BuildReactiveFunction transform HIR->ReactiveFunction but _without_ reactive scopes. 2. Align scopes to block boundaries, but rewritten to operate on ReactiveFunction 3. Merge overlapping scopes, again rewritten to operate on ReactiveFunction 4. Group statements within ReactiveFunction into ReactiveScopeBlocks (today this occurs when constructing the ReactiveFunction). This PR implements 1 and 4. Because the implementation is incomplete this would break the whole compiler, so for now both versions are still around. By default compilation uses the old pipeline, but if a feature flag is enabled we use the new version. The plan is to incrementally fix up the new version of the passes in this stack, and then cutover: removing the flag and the old version of the passes.
facebook · Jan 11, 2023 · 87d70b9 · 87d70b9
1 parent a4fc975
commit 87d70b9
Show file tree

Hide file tree

Showing 12 changed files with 617 additions and 34 deletions.
diff --git a/compiler/forget/src/CompilerFlags.ts b/compiler/forget/src/CompilerFlags.ts
@@ -0,0 +1,14 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+type Flags = {
+  enableNewReactiveFunctionBuilder: boolean;
+};
+
+export const flags: Flags = {
+  enableNewReactiveFunctionBuilder: false,
+};
diff --git a/compiler/forget/src/CompilerPipeline.ts b/compiler/forget/src/CompilerPipeline.ts
@@ -6,6 +6,7 @@
  */
 import { NodePath } from "@babel/traverse";
 import * as t from "@babel/types";
+import { flags } from "./CompilerFlags";
 import {
   Environment,
   HIRFunction,
@@ -16,11 +17,15 @@ import {
 import { inferMutableRanges, inferReferenceEffects } from "./Inference";
 import { constantPropagation } from "./Optimization";
 import {
+  alignReactiveScopesToBlockScopes,
+  buildReactiveBlocks,
   buildReactiveFunction,
+  buildReactiveFunctionWithoutScopes,
   codegenReactiveFunction,
   flattenReactiveLoops,
   inferReactiveScopes,
   inferReactiveScopeVariables,
+  mergeOverlappingReactiveScopes,
   propagateScopeDependencies,
   pruneUnusedLabels,
   pruneUnusedLValues,
@@ -72,15 +77,46 @@ export function* run(
   inferReactiveScopeVariables(hir);
   yield log({ kind: "hir", name: "InferReactiveScopeVariables", value: hir });
 
-  inferReactiveScopes(hir);
-  yield log({ kind: "hir", name: "InferReactiveScopes", value: hir });
-
-  const reactiveFunction = buildReactiveFunction(hir);
-  yield log({
-    kind: "reactive",
-    name: "BuildReactiveFunction",
-    value: reactiveFunction,
-  });
+  let reactiveFunction: ReactiveFunction;
+  if (!flags.enableNewReactiveFunctionBuilder) {
+    inferReactiveScopes(hir);
+    yield log({ kind: "hir", name: "InferReactiveScopes", value: hir });
+
+    reactiveFunction = buildReactiveFunction(hir);
+    yield log({
+      kind: "reactive",
+      name: "BuildReactiveFunction",
+      value: reactiveFunction,
+    });
+  } else {
+    reactiveFunction = buildReactiveFunctionWithoutScopes(hir);
+    yield log({
+      kind: "reactive",
+      name: "BuildReactiveFunction",
+      value: reactiveFunction,
+    });
+
+    alignReactiveScopesToBlockScopes(reactiveFunction);
+    yield log({
+      kind: "reactive",
+      name: "AlignReactiveScopesToBlockScopes",
+      value: reactiveFunction,
+    });
+
+    mergeOverlappingReactiveScopes(reactiveFunction);
+    yield log({
+      kind: "reactive",
+      name: "MergeOverlappingReactiveScopes",
+      value: reactiveFunction,
+    });
+
+    buildReactiveBlocks(reactiveFunction);
+    yield log({
+      kind: "reactive",
+      name: "BuildReactiveBlocks",
+      value: reactiveFunction,
+    });
+  }
 
   pruneUnusedLabels(reactiveFunction);
   yield log({

diff --git a/compiler/forget/src/HIR/HIR.ts b/compiler/forget/src/HIR/HIR.ts
@@ -77,31 +77,39 @@ export type ReactiveInstruction = {
 };
 
 export type ReactiveTerminal =
-  | { kind: "break"; label: BlockId | null }
-  | { kind: "continue"; label: BlockId | null }
-  | { kind: "return"; value: Place | null }
-  | { kind: "throw"; value: Place }
+  | { kind: "break"; label: BlockId | null; id: InstructionId | null }
+  | { kind: "continue"; label: BlockId | null; id: InstructionId }
+  | { kind: "return"; value: Place | null; id: InstructionId }
+  | { kind: "throw"; value: Place; id: InstructionId }
   | {
       kind: "switch";
       test: Place;
       cases: Array<{
         test: Place | null;
         block: ReactiveBlock | void;
       }>;
+      id: InstructionId;
+    }
+  | {
+      kind: "while";
+      test: ReactiveValueBlock;
+      loop: ReactiveBlock;
+      id: InstructionId;
     }
-  | { kind: "while"; test: ReactiveValueBlock; loop: ReactiveBlock }
   | {
       kind: "for";
       init: ReactiveValueBlock;
       test: ReactiveValueBlock;
       update: ReactiveValueBlock;
       loop: ReactiveBlock;
+      id: InstructionId;
     }
   | {
       kind: "if";
       test: Place;
       consequent: ReactiveBlock;
       alternate: ReactiveBlock | null;
+      id: InstructionId;
     };
 
 /**

diff --git a/compiler/forget/src/HIR/HIRTreeVisitor.ts b/compiler/forget/src/HIR/HIRTreeVisitor.ts
@@ -120,6 +120,7 @@ class Driver<
             kind: "return",
             loc: terminal.loc,
             value,
+            id: terminal.id,
           })
         );
         break;
@@ -132,6 +133,7 @@ class Driver<
           this.visitor.visitTerminal({
             kind: "throw",
             value,
+            id: terminal.id,
           })
         );
         break;
@@ -156,7 +158,7 @@ class Driver<
         this.visitor.visitTerminalId(terminal.id);
         let consequent: TBlock | null = null;
         if (this.cx.isScheduled(terminal.consequent)) {
-          const break_ = this.visitBreak(terminal.consequent);
+          const break_ = this.visitBreak(terminal.consequent, null);
           if (break_ !== null) {
             const builder = this.visitor.enterBlock();
             this.visitor.appendBlock(builder, break_);
@@ -171,7 +173,7 @@ class Driver<
         let alternate: TBlock | null = null;
         if (alternateId !== null) {
           if (this.cx.isScheduled(alternateId)) {
-            const break_ = this.visitBreak(alternateId);
+            const break_ = this.visitBreak(alternateId, null);
             if (break_ !== null) {
               const builder = this.visitor.enterBlock();
               this.visitor.appendBlock(builder, break_);
@@ -191,6 +193,7 @@ class Driver<
               test,
               consequent: consequent ?? this.emptyBlock(),
               alternate: alternate,
+              id: terminal.id,
             }),
             fallthroughId
           );
@@ -203,6 +206,7 @@ class Driver<
               test,
               consequent: consequent ?? this.emptyBlock(),
               alternate: alternate,
+              id: terminal.id,
             })
           );
         }
@@ -234,7 +238,7 @@ class Driver<
             // that are already scheduled. emit as follows:
             // - if the block is for another case branch, don't emit a break and fall-through
             // - else, emit an explicit break.
-            const break_ = this.visitBreak(case_.block);
+            const break_ = this.visitBreak(case_.block, null);
             if (
               index === 0 &&
               break_ === null &&
@@ -270,6 +274,7 @@ class Driver<
               kind: "switch",
               test,
               cases,
+              id: terminal.id,
             }),
             fallthroughId
           );
@@ -281,6 +286,7 @@ class Driver<
               kind: "switch",
               test,
               cases,
+              id: terminal.id,
             })
           );
         }
@@ -319,7 +325,7 @@ class Driver<
         if (loopId) {
           loopBody = this.traverseBlock(this.cx.ir.blocks.get(loopId)!);
         } else {
-          const break_ = this.visitBreak(terminal.loop);
+          const break_ = this.visitBreak(terminal.loop, null);
           invariant(
             break_ !== null,
             "If loop body is already scheduled it must be a break"
@@ -338,6 +344,7 @@ class Driver<
               loc: terminal.loc,
               test: testValue,
               loop: loopBody,
+              id: terminal.id,
             }),
             fallthroughId
           );
@@ -350,6 +357,7 @@ class Driver<
               loc: terminal.loc,
               test: testValue,
               loop: loopBody,
+              id: terminal.id,
             })
           );
         }
@@ -408,7 +416,7 @@ class Driver<
         if (loopId) {
           loopBody = this.traverseBlock(this.cx.ir.blocks.get(loopId)!);
         } else {
-          const break_ = this.visitBreak(terminal.loop);
+          const break_ = this.visitBreak(terminal.loop, null);
           invariant(
             break_ !== null,
             "If loop body is already scheduled it must be a break"
@@ -428,6 +436,7 @@ class Driver<
               test: testValue,
               update: updateValue,
               loop: loopBody,
+              id: terminal.id,
             }),
             fallthroughId
           );
@@ -441,6 +450,7 @@ class Driver<
               test: testValue,
               update: updateValue,
               loop: loopBody,
+              id: terminal.id,
             })
           );
         }
@@ -453,14 +463,14 @@ class Driver<
         this.visitor.visitTerminalId(terminal.id);
         switch (terminal.variant) {
           case GotoVariant.Break: {
-            const break_ = this.visitBreak(terminal.block);
+            const break_ = this.visitBreak(terminal.block, terminal.id);
             if (break_ !== null) {
               this.visitor.appendBlock(blockValue, break_);
             }
             break;
           }
           case GotoVariant.Continue: {
-            const continue_ = this.visitContinue(terminal.block);
+            const continue_ = this.visitContinue(terminal.block, terminal.id);
             if (continue_ !== null) {
               this.visitor.appendBlock(blockValue, continue_);
             }
@@ -519,7 +529,7 @@ class Driver<
     return this.visitor.leaveBlock(block);
   }
 
-  visitBreak(block: BlockId): TStatement | null {
+  visitBreak(block: BlockId, id: InstructionId | null): TStatement | null {
     const target = this.cx.getBreakTarget(block);
     if (target === null) {
       // TODO: we should always have a target
@@ -530,18 +540,19 @@ class Driver<
         return this.visitor.visitImplicitTerminal();
       }
       case "unlabeled": {
-        return this.visitor.visitTerminal({ kind: "break", label: null });
+        return this.visitor.visitTerminal({ kind: "break", label: null, id });
       }
       case "labeled": {
         return this.visitor.visitTerminal({
           kind: "break",
           label: target.block,
+          id,
         });
       }
     }
   }
 
-  visitContinue(block: BlockId): TStatement | null {
+  visitContinue(block: BlockId, id: InstructionId): TStatement | null {
     const target = this.cx.getContinueTarget(block);
     invariant(
       target !== null,
@@ -552,12 +563,14 @@ class Driver<
         return this.visitor.visitTerminal({
           kind: "continue",
           label: target.block,
+          id,
         });
       }
       case "unlabeled": {
         return this.visitor.visitTerminal({
           kind: "continue",
           label: null,
+          id,
         });
       }
       case "implicit": {
@@ -911,27 +924,35 @@ export interface Visitor<
 }
 
 export type BlockTerminal<TInit, TValue, TBlock, TCase> =
-  | { kind: "return"; loc: SourceLocation; value: TValue | null }
-  | { kind: "throw"; value: TValue }
+  | {
+      kind: "return";
+      loc: SourceLocation;
+      value: TValue | null;
+      id: InstructionId;
+    }
+  | { kind: "throw"; value: TValue; id: InstructionId }
   | {
       kind: "if";
       test: TValue;
       consequent: TBlock;
       alternate: TBlock | null;
+      id: InstructionId;
     }
-  | { kind: "switch"; test: TValue; cases: Array<TCase> }
+  | { kind: "switch"; test: TValue; cases: Array<TCase>; id: InstructionId }
   | {
       kind: "while";
       loc: SourceLocation;
       test: TValue;
       loop: TBlock;
+      id: InstructionId;
     }
   | {
       kind: "for";
       init: TInit;
       test: TValue;
       update: TValue;
       loop: TBlock;
+      id: InstructionId;
     }
-  | { kind: "break"; label: BlockId | null }
-  | { kind: "continue"; label: BlockId | null };
+  | { kind: "break"; label: BlockId | null; id: InstructionId | null }
+  | { kind: "continue"; label: BlockId | null; id: InstructionId };