From 87d70b9129e91f85bbcc6945a4416d9d7ffe22a6 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 11 Jan 2023 14:04:45 -0800 Subject: [PATCH] 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. --- compiler/forget/src/CompilerFlags.ts | 14 ++ compiler/forget/src/CompilerPipeline.ts | 54 +++- compiler/forget/src/HIR/HIR.ts | 18 +- compiler/forget/src/HIR/HIRTreeVisitor.ts | 51 ++-- .../AlignReactiveScopesToBlockScopes.ts | 50 ++++ .../src/ReactiveScopes/BuildReactiveBlocks.ts | 123 ++++++++++ .../ReactiveScopes/BuildReactiveFunction.ts | 16 +- .../BuildReactiveFunctionWithoutScopes.ts | 230 ++++++++++++++++++ .../MergeOverlappingReactiveScopes.ts | 85 +++++++ compiler/forget/src/ReactiveScopes/index.ts | 4 + compiler/forget/src/__tests__/hir-test.ts | 4 + compiler/forget/src/index.ts | 2 +- 12 files changed, 617 insertions(+), 34 deletions(-) create mode 100644 compiler/forget/src/CompilerFlags.ts create mode 100644 compiler/forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopes.ts create mode 100644 compiler/forget/src/ReactiveScopes/BuildReactiveBlocks.ts create mode 100644 compiler/forget/src/ReactiveScopes/BuildReactiveFunctionWithoutScopes.ts create mode 100644 compiler/forget/src/ReactiveScopes/MergeOverlappingReactiveScopes.ts diff --git a/compiler/forget/src/CompilerFlags.ts b/compiler/forget/src/CompilerFlags.ts new file mode 100644 index 0000000000000..30d6be875633e --- /dev/null +++ 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 index 0d312923fb09e..a77c71e9ce711 100644 --- 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 index a81931088b3ca..f36caa8bf2639 100644 --- a/compiler/forget/src/HIR/HIR.ts +++ b/compiler/forget/src/HIR/HIR.ts @@ -77,10 +77,10 @@ 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; @@ -88,20 +88,28 @@ export type ReactiveTerminal = 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 index 280b805440542..3ee7a8dd8e600 100644 --- 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,20 +924,27 @@ export interface Visitor< } export type BlockTerminal = - | { 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 } + | { kind: "switch"; test: TValue; cases: Array; id: InstructionId } | { kind: "while"; loc: SourceLocation; test: TValue; loop: TBlock; + id: InstructionId; } | { kind: "for"; @@ -932,6 +952,7 @@ export type BlockTerminal = 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 }; diff --git a/compiler/forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopes.ts b/compiler/forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopes.ts new file mode 100644 index 0000000000000..bd3d16f10513a --- /dev/null +++ b/compiler/forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopes.ts @@ -0,0 +1,50 @@ +/** + * 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. + */ + +import { ReactiveFunction } from "../HIR/HIR"; + +/** + * Note: this is the 2nd of 3 passes that determine how to break a function into discrete + * reactive scopes (independently memoizeable units of code): + * 1. InferReactiveScopeVariables (on HIR) determines operands that mutate together and assigns + * them a unique reactive scope. + * 2. AlignReactiveScopesToBlockScopes (this pass, on ReactiveFunction) aligns reactive scopes + * to block scopes. + * 3. MergeOverlappingReactiveScopes (on ReactiveFunction) ensures that reactive scopes do not + * overlap, merging any such scopes. + * + * Prior inference passes assign a reactive scope to each operand, but the ranges of these + * scopes are based on specific instructions at arbitrary points in the control-flow graph. + * However, to codegen blocks around the instructions in each scope, the scopes must be + * aligned to block-scope boundaries - we can't memoize half of a loop! + * + * This pass updates reactive scope boundaries to align to control flow boundaries, for + * example: + * + * ```javascript + * function foo(cond, a) { + * ⌵ original scope + * ⌵ expanded scope + * const x = []; ⌝ ⌝ + * if (cond) { ⎮ ⎮ + * ... ⎮ ⎮ + * x.push(a); ⌟ ⎮ + * ... ⎮ + * } ⌟ + * } + * ``` + * + * Here the original scope for `x` ended partway through the if consequent, but we can't + * memoize part of that block. This pass would align the scope to the end of the consequent. + * + * The more general rule is that a reactive scope may only end at the same block scope as it + * began: this pass therefore finds, for each scope, the block where that scope started and + * finds the first instruction after the scope's mutable range in that same block scope (which + * will be the updated end for that scope). + */ + +export function alignReactiveScopesToBlockScopes(fn: ReactiveFunction): void {} diff --git a/compiler/forget/src/ReactiveScopes/BuildReactiveBlocks.ts b/compiler/forget/src/ReactiveScopes/BuildReactiveBlocks.ts new file mode 100644 index 0000000000000..9450db688353c --- /dev/null +++ b/compiler/forget/src/ReactiveScopes/BuildReactiveBlocks.ts @@ -0,0 +1,123 @@ +/** + * 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. + */ + +import invariant from "invariant"; +import { + InstructionId, + makeInstructionId, + ReactiveBlock, + ReactiveFunction, + ReactiveInstruction, + ReactiveScope, + ReactiveScopeBlock, +} from "../HIR"; +import { eachInstructionValueOperand } from "../HIR/visitors"; +import { mapTerminalBlocks } from "./visitors"; + +/** + * Given a function where the reactive scopes have been correctly aligned and merged, + * this pass groups the instructions for each reactive scope into ReactiveBlocks. + */ +export function buildReactiveBlocks(fn: ReactiveFunction): void { + fn.body = visitBlock(fn.body); +} + +type Entry = + | ReactiveScopeBlock + | { kind: "block"; instructions: ReactiveBlock }; + +function visitBlock(block: ReactiveBlock): ReactiveBlock { + let current: Entry = { kind: "block", instructions: [] }; + const stack: Array = [current]; + let lastId: InstructionId = makeInstructionId(0); + for (const stmt of block) { + switch (stmt.kind) { + case "instruction": { + lastId = stmt.instruction.id; + while (current.kind === "scope" && lastId >= current.scope.range.end) { + current = stack.pop()!; + } + const scope = getInstructionScope(stmt.instruction); + + if ( + scope !== null && + (current.kind !== "scope" || current.scope.id !== scope.id) + ) { + const reactiveScope: ReactiveScopeBlock = { + kind: "scope", + scope, + instructions: [], + }; + current.instructions.push(reactiveScope); + stack.push(current); + current = reactiveScope; + } + + current.instructions.push(stmt); + break; + } + case "terminal": { + const id = stmt.terminal.id; + if (id !== null) { + lastId = id; + while ( + current.kind === "scope" && + lastId >= current.scope.range.end + ) { + current = stack.pop()!; + } + } + mapTerminalBlocks(stmt.terminal, visitBlock); + current.instructions.push(stmt); + break; + } + case "scope": { + invariant( + false, + "Expected the function to not have scopes already assigned" + ); + } + } + } + while (current.kind === "scope") { + invariant(current.scope.range.end === lastId + 1, "Scope ended too soon"); + current = stack.pop()!; + } + return current.instructions; +} + +function getInstructionScope({ + id, + lvalue, + value, +}: ReactiveInstruction): ReactiveScope | null { + invariant( + lvalue !== null, + "Expected lvalues to not be null when assigning scopes. " + + "Pruning lvalues too early can result in missing scope information." + ); + if ( + lvalue.place.identifier.scope !== null && + isScopeActive(lvalue.place.identifier.scope, id) + ) { + return lvalue.place.identifier.scope; + } else { + for (const operand of eachInstructionValueOperand(value)) { + if ( + operand.identifier.scope !== null && + isScopeActive(operand.identifier.scope, id) + ) { + return operand.identifier.scope; + } + } + } + return null; +} + +function isScopeActive(scope: ReactiveScope, id: InstructionId): boolean { + return id >= scope.range.start && id < scope.range.end; +} diff --git a/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts index 3b5d24d8e9d36..8a900292633c3 100644 --- a/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts @@ -218,11 +218,11 @@ class ReactiveFunctionBuilder let result: ReactiveTerminal; switch (terminal.kind) { case "break": { - result = { kind: "break", label: terminal.label }; + result = { kind: "break", label: terminal.label, id: terminal.id }; break; } case "continue": { - result = { kind: "continue", label: terminal.label }; + result = { kind: "continue", label: terminal.label, id: terminal.id }; break; } case "for": { @@ -233,6 +233,7 @@ class ReactiveFunctionBuilder test: terminal.test as ReactiveValueBlock, update: terminal.update as ReactiveValueBlock, loop: terminal.loop, + id: terminal.id, }; break; } @@ -242,6 +243,7 @@ class ReactiveFunctionBuilder test: terminal.test as Place, consequent: terminal.consequent, alternate: terminal.alternate, + id: terminal.id, }; break; } @@ -250,7 +252,7 @@ class ReactiveFunctionBuilder if (value !== null && value.kind !== "Identifier") { invariant(false, "Expected return to be a Place"); } - result = { kind: "return", value }; + result = { kind: "return", value, id: terminal.id }; break; } case "switch": { @@ -261,11 +263,16 @@ class ReactiveFunctionBuilder test: Place | null; block: ReactiveBlock | void; }>, + id: terminal.id, }; break; } case "throw": { - result = { kind: "throw", value: terminal.value as Place }; + result = { + kind: "throw", + value: terminal.value as Place, + id: terminal.id, + }; break; } case "while": { @@ -273,6 +280,7 @@ class ReactiveFunctionBuilder kind: "while", test: terminal.test as ReactiveValueBlock, loop: terminal.loop, + id: terminal.id, }; break; } diff --git a/compiler/forget/src/ReactiveScopes/BuildReactiveFunctionWithoutScopes.ts b/compiler/forget/src/ReactiveScopes/BuildReactiveFunctionWithoutScopes.ts new file mode 100644 index 0000000000000..fb489370fb9a0 --- /dev/null +++ b/compiler/forget/src/ReactiveScopes/BuildReactiveFunctionWithoutScopes.ts @@ -0,0 +1,230 @@ +/** + * 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. + */ + +import invariant from "invariant"; +import { + BlockId, + HIRFunction, + Instruction, + InstructionId, + InstructionValue, + Place, + ReactiveBlock, + ReactiveFunction, + ReactiveStatement, + ReactiveTerminal, + ReactiveValueBlock, +} from "../HIR/HIR"; +import { BlockTerminal, Visitor, visitTree } from "../HIR/HIRTreeVisitor"; +import { assertExhaustive } from "../Utils/utils"; + +export function buildReactiveFunction(fn: HIRFunction): ReactiveFunction { + const builder = new ReactiveFunctionBuilder(); + const body = visitTree(fn, builder); + invariant(body != null, "Expected a root block"); + return { + loc: fn.loc, + id: fn.id, + params: fn.params, + generator: fn.generator, + async: fn.async, + body, + }; +} + +class Builder { + #instructions: ReactiveBlock = []; + + append(item: ReactiveStatement, label: BlockId | undefined): void { + if (label !== undefined) { + invariant(item.kind === "terminal", "Only terminals may have a label"); + item.label = label; + } + this.#instructions.push(item); + } + + complete(): ReactiveBlock { + return this.#instructions; + } +} + +class ReactiveFunctionBuilder + implements + Visitor< + Builder, + ReactiveBlock, + ReactiveValueBlock, + ReactiveValueBlock, + InstructionValue | ReactiveValueBlock, + ReactiveStatement, + { test: InstructionValue | null; block: ReactiveBlock } + > +{ + enterBlock(): Builder { + return new Builder(); + } + appendBlock( + block: Builder, + item: ReactiveStatement, + label?: BlockId | undefined + ): void { + block.append(item, label); + } + leaveBlock(block: Builder): ReactiveBlock { + return block.complete(); + } + + enterValueBlock(block: Builder): ReactiveValueBlock { + return { + kind: "value-block", + instructions: [], + value: null, + }; + } + appendValueBlock(block: ReactiveValueBlock, item: ReactiveStatement): void { + block.instructions.push(item); + } + leaveValueBlock( + block: ReactiveValueBlock, + value: InstructionValue | ReactiveValueBlock | null + ): InstructionValue | ReactiveValueBlock { + if (value !== null) { + invariant( + value.kind !== "value-block", + "Expected value block to end in a value" + ); + block.value = value; + } + return block; + } + + enterInitBlock(block: Builder): ReactiveValueBlock { + return this.enterValueBlock(block); + } + appendInitBlock(block: ReactiveValueBlock, item: ReactiveStatement): void { + this.appendValueBlock(block, item); + } + leaveInitBlock(block: ReactiveValueBlock): ReactiveValueBlock { + return block; + } + + visitValue( + value: InstructionValue, + id: InstructionId + ): InstructionValue | ReactiveValueBlock { + return value; + } + visitInstruction( + instruction: Instruction, + value: InstructionValue | ReactiveValueBlock + ): ReactiveStatement { + return { kind: "instruction", instruction }; + } + visitTerminalId(id: InstructionId): void {} + visitImplicitTerminal(): ReactiveStatement | null { + return null; + } + visitTerminal( + terminal: BlockTerminal< + ReactiveValueBlock, + InstructionValue | ReactiveValueBlock, + ReactiveBlock, + { test: InstructionValue | null; block: ReactiveBlock } + > + ): ReactiveStatement { + let result: ReactiveTerminal; + switch (terminal.kind) { + case "break": { + result = { kind: "break", label: terminal.label, id: terminal.id }; + break; + } + case "continue": { + result = { kind: "continue", label: terminal.label, id: terminal.id }; + break; + } + case "for": { + const { test, update } = terminal; + result = { + kind: "for", + init: terminal.init, + test: terminal.test as ReactiveValueBlock, + update: terminal.update as ReactiveValueBlock, + loop: terminal.loop, + id: terminal.id, + }; + break; + } + case "if": { + result = { + kind: "if", + test: terminal.test as Place, + consequent: terminal.consequent, + alternate: terminal.alternate, + id: terminal.id, + }; + break; + } + case "return": { + const value = terminal.value; + if (value !== null && value.kind !== "Identifier") { + invariant(false, "Expected return to be a Place"); + } + result = { kind: "return", value, id: terminal.id }; + break; + } + case "switch": { + result = { + kind: "switch", + test: terminal.test as Place, + cases: terminal.cases as Array<{ + test: Place | null; + block: ReactiveBlock | void; + }>, + id: terminal.id, + }; + break; + } + case "throw": { + result = { + kind: "throw", + value: terminal.value as Place, + id: terminal.id, + }; + break; + } + case "while": { + result = { + kind: "while", + test: terminal.test as ReactiveValueBlock, + loop: terminal.loop, + id: terminal.id, + }; + break; + } + default: { + assertExhaustive( + terminal, + `Unexpected terminal kind '${(terminal as any).kind}'` + ); + } + } + return { + kind: "terminal", + terminal: result, + label: null, + }; + } + visitCase( + test: InstructionValue | ReactiveValueBlock | null, + block: ReactiveBlock + ): { test: InstructionValue | null; block: ReactiveBlock } { + if (test !== null && test.kind !== "Identifier") { + invariant(false, "Expected a Place"); + } + return { test, block }; + } +} diff --git a/compiler/forget/src/ReactiveScopes/MergeOverlappingReactiveScopes.ts b/compiler/forget/src/ReactiveScopes/MergeOverlappingReactiveScopes.ts new file mode 100644 index 0000000000000..cf92ca6e2c163 --- /dev/null +++ b/compiler/forget/src/ReactiveScopes/MergeOverlappingReactiveScopes.ts @@ -0,0 +1,85 @@ +/** + * 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. + */ + +import { ReactiveFunction } from "../HIR"; + +/** + * Note: this is the 3rd of 3 passes that determine how to break a function into discrete + * reactive scopes (independently memoizeable units of code): + * 1. InferReactiveScopeVariables (on HIR) determines operands that mutate together and assigns + * them a unique reactive scope. + * 2. AlignReactiveScopesToBlockScopes (on ReactiveFunction) aligns reactive scopes + * to block scopes. + * 3. MergeOverlappingReactiveScopes (this pass, on ReactiveFunction) ensures that reactive + * scopes do not overlap, merging any such scopes. + * + * Previous passes may leave "overlapping" scopes, ie where one or more instructions are within + * the mutable range of multiple reactive scopes. We prefer to avoid executing instructions twice + * for performance reasons (side effects are less of a concern bc components are required to be + * idempotent), so we cannot simply repeat the instruction once for each scope. Instead, the only + * option is to combine the two scopes into one. This is an area where an eventual Forget IDE + * could provide real-time feedback to the developer that two computations are accidentally merged. + * + * ## Detailed Walkthrough + * + * Two scopes overlap if there is one or more instruction that is inside the range + * of both scopes. In general, overlapping scopes are merged togther. The only + * exception to this is when one scope *shadows* another scope. For example: + * + * ```javascript + * function foo(cond, a) { + * ⌵ scope for x + * let x = []; ⌝ + * if (cond) { ⎮ + * ⌵ scope for y ⎮ + * let y = []; ⌝ ⎮ + * if (b) { ⎮ ⎮ + * y.push(b); ⌟ ⎮ + * } ⎮ + * x.push(
{y}
); ⎮ + * } ⌟ + * } + * ``` + * + * In this example the two scopes overlap, but mutation of the two scopes is not + * interleaved. Specifically within the y scope there are no instructions that + * modify any other scope: the inner scope "shadows" the outer one. This category + * of overlap does *NOT* merge the scopes together. + * + * The implementation is inspired by the Rust notion of "stacked borrows". We traverse + * the control-flow graph in tree form, at each point keeping track of which scopes are + * active. So initially we see + * + * `let x = []` + * active scopes: [x] + * + * and mark the x scope as active. + * + * Then we later encounter + * + * `let y = [];` + * active scopes: [x, y] + * + * Here we first check to see if 'y' is already in the list of active scopes. It isn't, + * so we push it to the stop of the stack. + * + * Then + * + * `y.push(b)` + * active scopes: [x, y] + * + * Mutates y, so we check if y is the top of the stack. It is, so no merging must occur. + * + * If instead we saw eg + * + * `x.push(b)` + * active scopes: [x, y] + * + * Then we would see that 'x' is active, but that it is shadowed. The two scopes would have + * to be merged. + */ +export function mergeOverlappingReactiveScopes(fn: ReactiveFunction): void {} diff --git a/compiler/forget/src/ReactiveScopes/index.ts b/compiler/forget/src/ReactiveScopes/index.ts index 2905926ece705..5c032071e8aa5 100644 --- a/compiler/forget/src/ReactiveScopes/index.ts +++ b/compiler/forget/src/ReactiveScopes/index.ts @@ -5,11 +5,15 @@ * LICENSE file in the root directory of this source tree. */ +export { alignReactiveScopesToBlockScopes } from "./AlignReactiveScopesToBlockScopes"; +export { buildReactiveBlocks } from "./BuildReactiveBlocks"; export { buildReactiveFunction } from "./BuildReactiveFunction"; +export { buildReactiveFunction as buildReactiveFunctionWithoutScopes } from "./BuildReactiveFunctionWithoutScopes"; export { codegenReactiveFunction } from "./CodegenReactiveFunction"; export { flattenReactiveLoops } from "./FlattenReactiveLoops"; export { inferReactiveScopes } from "./InferReactiveScopes"; export { inferReactiveScopeVariables } from "./InferReactiveScopeVariables"; +export { mergeOverlappingReactiveScopes } from "./MergeOverlappingReactiveScopes"; export { printReactiveFunction } from "./PrintReactiveFunction"; export { propagateScopeDependencies } from "./PropagateScopeDependencies"; export { pruneTemporaryLValues as pruneUnusedLValues } from "./PruneTemporaryLValues"; diff --git a/compiler/forget/src/__tests__/hir-test.ts b/compiler/forget/src/__tests__/hir-test.ts index 13e4c364c0c06..c0d6c13f0e964 100644 --- a/compiler/forget/src/__tests__/hir-test.ts +++ b/compiler/forget/src/__tests__/hir-test.ts @@ -13,6 +13,7 @@ import traverse from "@babel/traverse"; import { wasmFolder } from "@hpcc-js/wasm"; import path from "path"; import prettier from "prettier"; +import { flags } from "../CompilerFlags"; import { compile } from "../CompilerPipeline"; import { toggleLogging } from "../Utils/logger"; import generateTestsFromFixtures from "./test-utils/generateTestsFromFixtures"; @@ -30,6 +31,9 @@ wasmFolder( const Pragma_RE = /\/\/\s*@enable\((\w+)\)$/gm; describe("React Forget (HIR version)", () => { + flags.enableNewReactiveFunctionBuilder = + String(process.env["ENABLE_NEW_BUILDER"]) === "1"; + generateTestsFromFixtures( path.join(__dirname, "fixtures", "hir"), (input, file, options) => { diff --git a/compiler/forget/src/index.ts b/compiler/forget/src/index.ts index c4edc005b7066..fe64c5b10434d 100644 --- a/compiler/forget/src/index.ts +++ b/compiler/forget/src/index.ts @@ -20,7 +20,7 @@ import codegen from "./HIR/Codegen"; import { Environment } from "./HIR/HIRBuilder"; import printHIR, { printFunction } from "./HIR/PrintHIR"; import { inferMutableRanges, inferReferenceEffects } from "./Inference"; -import { buildReactiveFunction } from "./ReactiveScopes/BuildReactiveFunction"; +import { buildReactiveFunction } from "./ReactiveScopes/BuildReactiveFunctionWithoutScopes"; import { codegenReactiveFunction } from "./ReactiveScopes/CodegenReactiveFunction"; import { flattenReactiveLoops } from "./ReactiveScopes/FlattenReactiveLoops"; import { inferReactiveScopes } from "./ReactiveScopes/InferReactiveScopes";