From 5149ce0fbb5114a8ee46f313365d5e4984fcab94 Mon Sep 17 00:00:00 2001 From: Joseph Savona Date: Wed, 12 Oct 2022 16:11:15 -0700 Subject: [PATCH] [new-arch] Store HIR blocks in reverse postorder * Changes HIR to store blocks in reverse postorder, which allows forward data flow analysis to iterate the blocks in order and (in the absence of loops) see all predecessors before visiting a successor. * Updates reference kind inference to exploit this ordering Note that the approach of modifying the ordering in `mapTerminalSuccessors()` feels gross, i'd like to split this up a bit. --- compiler/forget/src/HIR/HIR.ts | 8 ++ compiler/forget/src/HIR/HIRBuilder.ts | 85 +++++++------ .../src/HIR/InferReferenceCapability.ts | 83 +++++++------ .../fixtures/hir/component.expect.md | 22 ++-- .../fixtures/hir/conditional-break.expect.md | 6 +- .../fixtures/hir/reverse-postorder.expect.md | 114 ++++++++++++++++++ .../fixtures/hir/reverse-postorder.js | 27 +++++ .../hir/switch-non-final-default.expect.md | 10 +- 8 files changed, 262 insertions(+), 93 deletions(-) create mode 100644 compiler/forget/src/__tests__/fixtures/hir/reverse-postorder.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/reverse-postorder.js diff --git a/compiler/forget/src/HIR/HIR.ts b/compiler/forget/src/HIR/HIR.ts index 5bdfb38ac6..4aa9e44754 100644 --- a/compiler/forget/src/HIR/HIR.ts +++ b/compiler/forget/src/HIR/HIR.ts @@ -72,6 +72,13 @@ export type HIRFunction = { */ export type HIR = { entry: BlockId; + + /** + * Basic blocks are stored as a map to aid certain operations that need to + * lookup blocks by their id. However, the order of the items in the map is + * reverse postorder, that is, barring cycles, predecessors appear before + * successors. This is designed to facilitate forward data flow analysis. + */ blocks: Map; }; @@ -84,6 +91,7 @@ export type HIR = { * statements and not implicit exceptions which may occur. */ export type BasicBlock = { + id: BlockId; instructions: Array; terminal: Terminal; }; diff --git a/compiler/forget/src/HIR/HIRBuilder.ts b/compiler/forget/src/HIR/HIRBuilder.ts index 3c081e9d19..0fc3b860f3 100644 --- a/compiler/forget/src/HIR/HIRBuilder.ts +++ b/compiler/forget/src/HIR/HIRBuilder.ts @@ -111,6 +111,7 @@ export default class HIRBuilder { build(): HIR { const { id: blockId, instructions } = this.#current; this.#completed.set(blockId, { + id: blockId, instructions, terminal: { kind: "return", value: null }, }); @@ -126,6 +127,7 @@ export default class HIRBuilder { terminate(terminal: Terminal) { const { id: blockId, instructions } = this.#current; this.#completed.set(blockId, { + id: blockId, instructions, terminal, }); @@ -140,6 +142,7 @@ export default class HIRBuilder { terminateWithContinuation(terminal: Terminal, continuation: WipBlock) { const { id: blockId, instructions } = this.#current; this.#completed.set(blockId, { + id: blockId, instructions, terminal, }); @@ -160,7 +163,7 @@ export default class HIRBuilder { */ complete(block: WipBlock, terminal: Terminal) { const { id: blockId, instructions } = block; - this.#completed.set(blockId, { instructions, terminal }); + this.#completed.set(blockId, { id: blockId, instructions, terminal }); } /** @@ -175,7 +178,7 @@ export default class HIRBuilder { this.#current = newBlock(nextId); const terminal = fn(nextId); const { id: blockId, instructions } = this.#current; - this.#completed.set(blockId, { instructions, terminal }); + this.#completed.set(blockId, { id: blockId, instructions, terminal }); this.#current = current; return nextId; } @@ -288,7 +291,10 @@ export default class HIRBuilder { /** * Helper to shrink a CFG to eliminate unreachable node and eliminate jump-only blocks. */ -function shrink(func: HIR): HIR { +function shrink(func: { + blocks: Map; + entry: BlockId; +}): HIR { const gotos = new Map(); /** * Given a target block for some terminator, resolves the ideal block that should be @@ -313,45 +319,49 @@ function shrink(func: HIR): HIR { } } - /** - * Fixpoint iteration to explore all blocks reachable from the entry block, - * and to resolve their terminator targets to avoid indirections. - * This implicitly prunes unreachable blocks since they are never visited and - * therefore not added to the output. - */ - const queue = [func.entry]; - // new set of output blocks + const visited: Set = new Set(); const blocks: Map = new Map(); - while (queue.length !== 0) { - const blockId = queue.shift()!; - if (blocks.has(blockId)) { - continue; - } - const { instructions, terminal: prevTerminal } = func.blocks.get(blockId)!; - const terminal = mapTerminalSuccessors(prevTerminal, (prevTarget) => { - const target = resolveBlockTarget(prevTarget); - queue.push(target); - return target; - }); - blocks.set(blockId, { - instructions, - terminal, - }); - } - for (const block of blocks.values()) { + // Visit all the blocks and map their successors to remove indirections, + // and eliminate unreachable blocks. Stores blocks in postorder, + // successors before predecessors. + function visit(blockId: BlockId) { + visited.add(blockId); + const block = func.blocks.get(blockId)!; + const { instructions, terminal: prevTerminal } = block; + const terminal = mapTerminalSuccessors( + prevTerminal, + (prevTarget, isFallthrough) => { + const target = resolveBlockTarget(prevTarget); + if (!visited.has(target) && !isFallthrough) { + visit(target); + } + return target; + } + ); + block.terminal = terminal; + blocks.set(blockId, block); + } + visit(func.entry); + + // Cleanup any fallthrough blocks that weren't visited + // also store into reverse postorder. + const reversedBlocks: Map = new Map(); + for (const blockId of Array.from(blocks.keys()).reverse()) { + const block = blocks.get(blockId)!; if (block.terminal.kind === "if" || block.terminal.kind === "switch") { if ( block.terminal.fallthrough !== null && - !blocks.has(block.terminal.fallthrough) + !visited.has(block.terminal.fallthrough) ) { block.terminal.fallthrough = null; } } + reversedBlocks.set(blockId, block); } return { - blocks, + blocks: reversedBlocks, entry: func.entry, }; } @@ -368,6 +378,9 @@ function getTargetIfIndirection(block: BasicBlock): number | null { /** * Maps a terminal node's block assignments using the provided function. + * + * TODO: this visits successors in reverse ordering to facilitate shrink()'s + * goal of producing a reverse postorder graph where siblings are in-order. */ export function mapTerminalSuccessors( terminal: Terminal, @@ -382,10 +395,10 @@ export function mapTerminalSuccessors( }; } case "if": { - const consequent = fn(terminal.consequent, false); - const alternate = fn(terminal.alternate, false); const fallthrough = terminal.fallthrough !== null ? fn(terminal.fallthrough, true) : null; + const alternate = fn(terminal.alternate, false); + const consequent = fn(terminal.consequent, false); return { kind: "if", test: terminal.test, @@ -395,19 +408,19 @@ export function mapTerminalSuccessors( }; } case "switch": { - const cases = terminal.cases.map((case_) => { + const fallthrough = + terminal.fallthrough !== null ? fn(terminal.fallthrough, true) : null; + const cases = [...terminal.cases].reverse().map((case_) => { const target = fn(case_.block, false); return { test: case_.test, block: target, }; }); - const fallthrough = - terminal.fallthrough !== null ? fn(terminal.fallthrough, true) : null; return { kind: "switch", test: terminal.test, - cases, + cases: cases.reverse(), fallthrough, }; } diff --git a/compiler/forget/src/HIR/InferReferenceCapability.ts b/compiler/forget/src/HIR/InferReferenceCapability.ts index c086761b16..19043577ac 100644 --- a/compiler/forget/src/HIR/InferReferenceCapability.ts +++ b/compiler/forget/src/HIR/InferReferenceCapability.ts @@ -99,51 +99,58 @@ export default function inferReferenceCapability(fn: HIRFunction) { initialEnvironment.define(place, value); } - // Queue of blocks to visit, with block and the incoming Environment value - const queue: Array = [ - { blockId: fn.body.entry, environment: initialEnvironment }, - ]; - // Map of blocks to the last incoming environment that was processed + // Map of blocks to the last (merged) incoming environment that was processed const environmentsByBlock: Map = new Map(); - while (queue.length !== 0) { - const { blockId, environment: incomingEnvironment } = queue.shift()!; - - let previousEnvironment = environmentsByBlock.get(blockId); - let nextEnvironment: Environment | null = null; - if (previousEnvironment === undefined) { - // If no previous environment, save the incoming environment and - // infer the block with this environment - environmentsByBlock.set(blockId, incomingEnvironment); - nextEnvironment = incomingEnvironment.clone(); + // Multiple predecessors may be visited prior to reaching a given successor, + // so track the list of incoming environments for each successor block. + // These are merged when reaching that block again. + const queuedEnvironments: Map = new Map(); + function queue(blockId: BlockId, environment: Environment) { + let queuedEnvironment = queuedEnvironments.get(blockId); + if (queuedEnvironment != null) { + // merge the queued environments for this block + environment = queuedEnvironment.merge(environment) ?? environment; + queuedEnvironments.set(blockId, environment); } else { - // If there's a previous environment, merge the previous/new incoming - // environments. If there are no changes, then the block can be skipped. - // otherwise save the merged environment and revisit the block with it. - const mergedEnvironment = previousEnvironment.merge(incomingEnvironment); - if (mergedEnvironment !== null) { - environmentsByBlock.set(blockId, mergedEnvironment); - nextEnvironment = mergedEnvironment.clone(); - } else { - continue; + // this is the first queued environment for this block, see whether + // there are changed relative to the last time it was processed. + const prevEnvironment = environmentsByBlock.get(blockId); + const nextEnvironment = + prevEnvironment != null + ? prevEnvironment.merge(environment) + : environment; + if (nextEnvironment != null) { + queuedEnvironments.set(blockId, nextEnvironment); } } + } + queue(fn.body.entry, initialEnvironment); - const environment = nextEnvironment; // rebind to preserve the non-null refinement - const block = fn.body.blocks.get(blockId)!; - inferBlock(environment, block); - - // TODO: add a `forEachTerminalSuccessor` helper, we don't actually want the result - // here - const _ = mapTerminalSuccessors( - block.terminal, - (nextBlockId, isFallthrough) => { - if (!isFallthrough) { - queue.push({ blockId: nextBlockId, environment }); - } - return nextBlockId; + while (queuedEnvironments.size !== 0) { + for (const [blockId, block] of fn.body.blocks) { + const incomingEnvironment = queuedEnvironments.get(blockId); + queuedEnvironments.delete(blockId); + if (incomingEnvironment == null) { + continue; } - ); + + environmentsByBlock.set(blockId, incomingEnvironment); + const environment = incomingEnvironment.clone(); + inferBlock(environment, block); + + // TODO: add a `forEachTerminalSuccessor` helper, we don't actually want the result + // here + const _ = mapTerminalSuccessors( + block.terminal, + (nextBlockId, isFallthrough) => { + if (!isFallthrough) { + queue(nextBlockId, environment); + } + return nextBlockId; + } + ); + } } } diff --git a/compiler/forget/src/__tests__/fixtures/hir/component.expect.md b/compiler/forget/src/__tests__/fixtures/hir/component.expect.md index 95a1623d6b..c520e5268a 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/component.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/component.expect.md @@ -48,17 +48,6 @@ bb3: Const mutable $11 = null Const mutable $12 = Binary readonly item$10 == readonly $11 If (readonly $12) then:bb8 else:bb9 -bb2: - Const mutable count$17 = readonly renderedItems$4.length - Const mutable $18 = "div" - Const mutable $19 = "\n " - Const mutable $20 = "h1" - Const mutable $21 = " Items" - Const mutable $22 = JSX {freeze count$17}{readonly $21} - Const mutable $23 = "\n " - Const mutable $24 = "\n " - Const mutable $25 = JSX {readonly $19}{readonly $22}{readonly $23}{freeze renderedItems$4}{readonly $24} - Return readonly $25 bb8: Const mutable $13 = readonly $12 Goto bb7 @@ -74,6 +63,17 @@ bb4: Call mutable renderedItems$4.push(readonly $15) Const mutable $16 = Binary readonly renderedItems$4.length >= readonly max$7 If (readonly $16) then:bb2 else:bb1 +bb2: + Const mutable count$17 = readonly renderedItems$4.length + Const mutable $18 = "div" + Const mutable $19 = "\n " + Const mutable $20 = "h1" + Const mutable $21 = " Items" + Const mutable $22 = JSX {freeze count$17}{readonly $21} + Const mutable $23 = "\n " + Const mutable $24 = "\n " + Const mutable $25 = JSX {readonly $19}{readonly $22}{readonly $23}{freeze renderedItems$4}{readonly $24} + Return readonly $25 ``` ## Code diff --git a/compiler/forget/src/__tests__/fixtures/hir/conditional-break.expect.md b/compiler/forget/src/__tests__/fixtures/hir/conditional-break.expect.md index f656d1c9af..8d63bafad3 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/conditional-break.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/conditional-break.expect.md @@ -201,12 +201,12 @@ bb0: Const mutable a$2 = Array [] Call mutable a$2.push(readonly props$1.a) If (readonly props$1.b) then:bb1 else:bb2 -bb1: - Call mutable a$2.push(readonly props$1.d) - Return freeze a$2 bb2: Call mutable a$2.push(readonly props$1.c) Goto bb1 +bb1: + Call mutable a$2.push(readonly props$1.d) + Return freeze a$2 ``` ## Code diff --git a/compiler/forget/src/__tests__/fixtures/hir/reverse-postorder.expect.md b/compiler/forget/src/__tests__/fixtures/hir/reverse-postorder.expect.md new file mode 100644 index 0000000000..e2b77e0b05 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/reverse-postorder.expect.md @@ -0,0 +1,114 @@ + +## Input + +```javascript +function Component(props) { + let x; + if (props.cond) { + switch (props.test) { + case 0: { + x = props.v0; + break; + } + case 1: { + x = props.v1; + break; + } + case 2: { + } + default: { + x = props.v2; + } + } + } else { + if (props.cond2) { + x = props.b; + } else { + x = props.c; + } + } + x; +} + +``` + +## HIR + +``` +bb0: + Let mutable x$2 = undefined + If (readonly props$1.cond) then:bb2 else:bb10 +bb2: + Const mutable $3 = 2 + Const mutable $4 = 1 + Const mutable $5 = 0 + Switch ( props$1.test) + Case readonly $5: bb8 + Case readonly $4: bb6 + Case readonly $3: bb4 + Default: bb4 +bb8: + Reassign mutable x$2 = readonly props$1.v0 + Goto bb1 +bb6: + Reassign mutable x$2 = readonly props$1.v1 + Goto bb1 +bb4: + Reassign mutable x$2 = readonly props$1.v2 + Goto bb1 +bb10: + If (readonly props$1.cond2) then:bb12 else:bb13 +bb12: + Reassign mutable x$2 = readonly props$1.b + Goto bb1 +bb13: + Reassign mutable x$2 = readonly props$1.c + Goto bb1 +bb1: + readonly x$2 + Return +``` + +## Code + +```javascript +function Component$0(props$1) { + let x$2 = undefined; + if (props$1.cond) { + switch (props$1.test) { + case 0: { + x$2 = props$1.v0; + ("<>"); + } + case 1: { + x$2 = props$1.v1; + ("<>"); + } + case 2: { + x$2 = props$1.v2; + ("<>"); + } + default: { + x$2 = props$1.v2; + ("<>"); + } + } + x$2; + return; + } else { + if (props$1.cond2) { + x$2 = props$1.b; + ("<>"); + } else { + x$2 = props$1.c; + ("<>"); + } + x$2; + return; + } + x$2; + return; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/reverse-postorder.js b/compiler/forget/src/__tests__/fixtures/hir/reverse-postorder.js new file mode 100644 index 0000000000..ce5bb33bd1 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/reverse-postorder.js @@ -0,0 +1,27 @@ +function Component(props) { + let x; + if (props.cond) { + switch (props.test) { + case 0: { + x = props.v0; + break; + } + case 1: { + x = props.v1; + break; + } + case 2: { + } + default: { + x = props.v2; + } + } + } else { + if (props.cond2) { + x = props.b; + } else { + x = props.c; + } + } + x; +} diff --git a/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md b/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md index 9f783f2078..3a0bb1fb0d 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md @@ -42,11 +42,6 @@ bb0: Case readonly $5: bb6 Default: bb1 Case readonly $4: bb2 -bb1: - Const mutable child$7 = JSX - Call readonly y$3.push(readonly props$1.p4) - Const mutable $8 = JSX {readonly child$7} - Return readonly $8 bb6: Call mutable x$2.push(readonly props$1.p2) Reassign mutable y$3 = Array [] @@ -54,6 +49,11 @@ bb6: bb2: Reassign mutable y$3 = readonly x$2 Goto bb1 +bb1: + Const mutable child$7 = JSX + Call readonly y$3.push(readonly props$1.p4) + Const mutable $8 = JSX {readonly child$7} + Return readonly $8 ``` ## Code