From 35ba4149ec2fb3831e7f82c00f68725c7897124f Mon Sep 17 00:00:00 2001 From: Joseph Savona Date: Mon, 12 Dec 2022 15:40:46 -0800 Subject: [PATCH] Support assignment in for statement's update clause Per the title, this PR adds support for assignment expressions in update clauses. This was mostly fixed by the previous diff to improve value block handling, and there's only a bit more to do here to allow a "value block" that doesn't produce a value (we need a better name). --- compiler/forget/src/HIR/BuildHIR.ts | 20 ++-- compiler/forget/src/HIR/Codegen.ts | 10 +- compiler/forget/src/HIR/HIRTreeVisitor.ts | 23 ++-- .../forget/src/HIR/InferReactiveScopes.ts | 37 ++++-- compiler/forget/src/HIR/LeaveSSA.ts | 9 ++ .../hir/ssa-for-trivial-update.expect.md | 113 ++++++++++++++++++ .../fixtures/hir/ssa-for-trivial-update.js | 7 ++ .../__tests__/fixtures/hir/ssa-for.expect.md | 50 ++++---- .../src/__tests__/fixtures/hir/ssa-for.js | 2 +- .../hir/ssa-while-no-reassign.expect.md | 5 +- .../fixtures/hir/ssa-while.expect.md | 8 +- 11 files changed, 221 insertions(+), 63 deletions(-) create mode 100644 compiler/forget/src/__tests__/fixtures/hir/ssa-for-trivial-update.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/hir/ssa-for-trivial-update.js diff --git a/compiler/forget/src/HIR/BuildHIR.ts b/compiler/forget/src/HIR/BuildHIR.ts index c994aae9e5..bb7a563990 100644 --- a/compiler/forget/src/HIR/BuildHIR.ts +++ b/compiler/forget/src/HIR/BuildHIR.ts @@ -340,6 +340,7 @@ function lowerStatement( const updateBlock = builder.enter((blockId) => { const update = stmt.get("update"); + todoInvariant(update.hasNode(), "Handle empty for updater"); if (update.hasNode()) { lowerExpressionToVoid(builder, update); } @@ -1275,20 +1276,21 @@ function lowerExpressionToPlace( return place; } +/** + * Lowers an expression to an instruction with no lvalue + */ function lowerExpressionToVoid( builder: HIRBuilder, exprPath: NodePath ): void { const instr = lowerExpression(builder, exprPath); - if (instr.kind !== "Identifier") { - const exprLoc = exprPath.node.loc ?? GeneratedSource; - builder.push({ - id: makeInstructionId(0), - value: instr, - loc: exprLoc, - lvalue: null, - }); - } + const exprLoc = exprPath.node.loc ?? GeneratedSource; + builder.push({ + id: makeInstructionId(0), + value: instr, + loc: exprLoc, + lvalue: null, + }); } function lowerLVal(builder: HIRBuilder, exprPath: NodePath): Place { diff --git a/compiler/forget/src/HIR/Codegen.ts b/compiler/forget/src/HIR/Codegen.ts index e8aa9dbf97..33f444c90d 100644 --- a/compiler/forget/src/HIR/Codegen.ts +++ b/compiler/forget/src/HIR/Codegen.ts @@ -137,9 +137,13 @@ class CodegenVisitor appendValueBlock(block: t.Statement[], item: t.Statement): void { this.appendBlock(block, item); } - leaveValueBlock(block: t.Statement[], place: t.Expression): t.Expression { + leaveValueBlock( + block: t.Statement[], + place: t.Expression | null + ): t.Expression { this.depth--; if (block.length === 0) { + invariant(place !== null, "Unexpected empty value block"); return place; } const expressions = block.map((stmt) => { @@ -153,7 +157,9 @@ class CodegenVisitor ); } }); - expressions.push(place); + if (place !== null) { + expressions.push(place); + } return t.sequenceExpression(expressions); } diff --git a/compiler/forget/src/HIR/HIRTreeVisitor.ts b/compiler/forget/src/HIR/HIRTreeVisitor.ts index 7979e8646a..30cae5e64f 100644 --- a/compiler/forget/src/HIR/HIRTreeVisitor.ts +++ b/compiler/forget/src/HIR/HIRTreeVisitor.ts @@ -451,24 +451,27 @@ class Driver { ): TValue { const valueBlock = this.visitor.enterValueBlock(parent); const instructions = [...block.instructions]; - let lastValue: { value: InstructionValue; id: InstructionId }; + let lastValue: { value: InstructionValue; id: InstructionId } | null = null; if (terminalValue != null) { lastValue = terminalValue; } else { - invariant(instructions.length > 0, "Value block may not be empty"); - const last = instructions.pop()!; - invariant( - last.lvalue === null, - "Expected value block to end in a value, not an assignment" - ); - lastValue = { value: last.value, id: last.id }; + if ( + instructions.length && + instructions[instructions.length - 1].lvalue === null + ) { + const last = instructions.pop()!; + lastValue = { value: last.value, id: last.id }; + } } for (const instr of instructions) { const value = this.visitor.visitValue(instr.value, instr.id); const item = this.visitor.visitInstruction(instr, value); this.visitor.appendValueBlock(valueBlock, item); } - const value = this.visitor.visitValue(lastValue.value, lastValue.id); + const value = + lastValue !== null + ? this.visitor.visitValue(lastValue.value, lastValue.id) + : null; return this.visitor.leaveValueBlock(valueBlock, value); } @@ -817,7 +820,7 @@ export interface Visitor< * Converts the visitor's value block (and final value) to the visitor's * value representation. */ - leaveValueBlock(block: TValueBlock, value: TValue): TValue; + leaveValueBlock(block: TValueBlock, value: TValue | null): TValue; enterInitBlock(block: TBlock): TValueBlock; diff --git a/compiler/forget/src/HIR/InferReactiveScopes.ts b/compiler/forget/src/HIR/InferReactiveScopes.ts index 0f767f9553..3f61f6ddeb 100644 --- a/compiler/forget/src/HIR/InferReactiveScopes.ts +++ b/compiler/forget/src/HIR/InferReactiveScopes.ts @@ -336,7 +336,10 @@ class AlignReactiveScopesToBlockScopeRangeVisitor { // For each block scope (outer array) stores a list of ReactiveScopes that start // in that block scope. - blockScopes: Array> = []; + blockScopes: Array<{ + kind: "block" | "value"; + scopes: Array; + }> = []; // ReactiveScopes whose declaring block scope has ended but may still need to // be "closed" (ie have their range.end be updated). A given scope can be in @@ -349,7 +352,10 @@ class AlignReactiveScopesToBlockScopeRangeVisitor visitId(id: InstructionId) { const currentScopes = this.blockScopes[this.blockScopes.length - 1]!; - const scopes = [...currentScopes, ...this.unclosedScopes]; + if (currentScopes.kind === "value") { + return; + } + const scopes = [...currentScopes.scopes, ...this.unclosedScopes]; for (const pending of scopes) { if (!pending.active) { continue; @@ -362,7 +368,7 @@ class AlignReactiveScopesToBlockScopeRangeVisitor } enterBlock(): void { - this.blockScopes.push([]); + this.blockScopes.push({ kind: "block", scopes: [] }); } appendBlock(block: void, item: void, label?: BlockId | undefined): void {} @@ -370,10 +376,10 @@ class AlignReactiveScopesToBlockScopeRangeVisitor leaveBlock(block: void): void { const lastScope = this.blockScopes.pop(); invariant( - lastScope !== undefined, + lastScope !== undefined && lastScope.kind === "block", "Expected enterBlock/leaveBlock to be called 1:1" ); - for (const scope of lastScope) { + for (const scope of lastScope.scopes) { if (scope.active) { this.unclosedScopes.push(scope); } @@ -381,19 +387,30 @@ class AlignReactiveScopesToBlockScopeRangeVisitor } enterValueBlock(): void { - this.enterBlock(); + this.blockScopes.push({ kind: "value", scopes: [] }); } appendValueBlock(block: void, item: void): void {} leaveValueBlock(block: void, value: void): void { - this.leaveBlock(block); + const lastScope = this.blockScopes.pop(); + invariant( + lastScope !== undefined && lastScope.kind === "value", + "Expected enterValueBlock/leaveValueBlock to be called 1:1" + ); + for (const scope of lastScope.scopes) { + invariant( + scope.active, + "Value scopes cannot be closed separately from the parent block" + ); + this.unclosedScopes.push(scope); + } } enterInitBlock(block: void): void { - this.enterBlock(); + this.enterValueBlock(); } appendInitBlock(block: void, item: void): void {} leaveInitBlock(block: void): void { - this.leaveBlock(block); + this.leaveValueBlock(block); } visitInstruction(instruction: Instruction, value: void): void { @@ -403,7 +420,7 @@ class AlignReactiveScopesToBlockScopeRangeVisitor if (!this.seenScopes.has(scope.id)) { const currentScopes = this.blockScopes[this.blockScopes.length - 1]!; this.seenScopes.add(scope.id); - currentScopes.push({ + currentScopes.scopes.push({ active: true, scope, }); diff --git a/compiler/forget/src/HIR/LeaveSSA.ts b/compiler/forget/src/HIR/LeaveSSA.ts index 0f8fb49870..09787b9781 100644 --- a/compiler/forget/src/HIR/LeaveSSA.ts +++ b/compiler/forget/src/HIR/LeaveSSA.ts @@ -62,8 +62,17 @@ export function leaveSSA(fn: HIRFunction) { phis.push(...loop.phis); } if (terminal.kind === "for") { + const init = fn.body.blocks.get(terminal.init)!; + phis.push(...init.phis); const update = fn.body.blocks.get(terminal.update)!; phis.push(...update.phis); + + // find declarations in the for init + for (const instr of init.instructions) { + if (instr.lvalue !== null && instr.lvalue.place.memberPath === null) { + hasDeclaration.add(instr.lvalue.place.identifier); + } + } } // For each phi, determine a canonical identifier to use for versions of the variable diff --git a/compiler/forget/src/__tests__/fixtures/hir/ssa-for-trivial-update.expect.md b/compiler/forget/src/__tests__/fixtures/hir/ssa-for-trivial-update.expect.md new file mode 100644 index 0000000000..446ebf66ad --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/ssa-for-trivial-update.expect.md @@ -0,0 +1,113 @@ + +## Input + +```javascript +function foo() { + let x = 1; + for (let i = 0; i < 10; /* update is intentally a single identifier */ i) { + x += 1; + } + return x; +} + +``` + +## HIR + +``` +bb0: + [1] Let mutate x$6_@1[1:13] = 1 + [2] For init=bb3 test=bb1 loop=bb5 update=bb4 fallthrough=bb2 +bb3: + predecessor blocks: bb0 + [3] Const mutate i$7_@1[1:13] = 0 + [4] Goto bb1 +bb1: + predecessor blocks: bb3 bb4 + [5] Const mutate $8_@1[1:13] = 10 + [6] Const mutate $10_@3[6:8] = Binary read i$7_@1 < read $8_@1 + [7] If (read $10_@3) then:bb5 else:bb2 fallthrough=bb2 +bb5: + predecessor blocks: bb1 + [8] Const mutate $11_@4 = 1 + [9] Reassign mutate x$6_@1[1:13] = Binary read x$6_@1 + read $11_@4 + [10] Goto(Continue) bb4 +bb4: + predecessor blocks: bb5 + [11] read i$7_@1 + [12] Goto bb1 +bb2: + predecessor blocks: bb1 + [13] Return read x$6_@1 + +``` + +### CFG + +```mermaid +flowchart TB + %% Basic Blocks + subgraph bb0 + bb0_instrs[" + [1] Let mutate x$6_@1[1:13] = 1 + "] + bb0_instrs --> bb0_terminal(["For"]) + end + subgraph bb3 + bb3_instrs[" + [3] Const mutate i$7_@1[1:13] = 0 + "] + bb3_instrs --> bb3_terminal(["Goto"]) + end + subgraph bb1 + bb1_instrs[" + [5] Const mutate $8_@1[1:13] = 10 + [6] Const mutate $10_@3[6:8] = Binary read i$7_@1 < read $8_@1 + "] + bb1_instrs --> bb1_terminal(["If (read $10_@3)"]) + end + subgraph bb5 + bb5_instrs[" + [8] Const mutate $11_@4 = 1 + [9] Reassign mutate x$6_@1[1:13] = Binary read x$6_@1 + read $11_@4 + "] + bb5_instrs --> bb5_terminal(["Goto"]) + end + subgraph bb4 + bb4_instrs[" + [11] read i$7_@1 + "] + bb4_instrs --> bb4_terminal(["Goto"]) + end + subgraph bb2 + bb2_terminal(["Return read x$6_@1"]) + end + + %% Jumps + bb0_terminal -- "init" --> bb3 + bb0_terminal -- "test" --> bb1 + bb0_terminal -- "update" --> bb4 + bb0_terminal -- "loop" --> bb5 + bb0_terminal -- "fallthrough" --> bb2 + bb3_terminal --> bb1 + bb1_terminal -- "then" --> bb5 + bb1_terminal -- "else" --> bb2 + bb5_terminal --> bb4 + bb4_terminal --> bb1 + +``` + +## Code + +```javascript +function foo$0() { + let x$6 = 1; + bb2: for (const i$7 = 0; i$7 < 10; i$7) { + x$6 = x$6 + 1; + } + + return x$6; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/hir/ssa-for-trivial-update.js b/compiler/forget/src/__tests__/fixtures/hir/ssa-for-trivial-update.js new file mode 100644 index 0000000000..ec6ed5cc75 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/hir/ssa-for-trivial-update.js @@ -0,0 +1,7 @@ +function foo() { + let x = 1; + for (let i = 0; i < 10; /* update is intentally a single identifier */ i) { + x += 1; + } + return x; +} diff --git a/compiler/forget/src/__tests__/fixtures/hir/ssa-for.expect.md b/compiler/forget/src/__tests__/fixtures/hir/ssa-for.expect.md index 8543ea1bf1..2cbce2ee3b 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/ssa-for.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/ssa-for.expect.md @@ -4,7 +4,7 @@ ```javascript function foo() { let x = 1; - for (let i = 0; i < 10; update()) { + for (let i = 0; i < 10; i += 1) { x += 1; } return x; @@ -16,32 +16,32 @@ function foo() { ``` bb0: - [1] Let mutate x$7_@0[1:13] = 1 + [1] Let mutate x$7_@1[1:15] = 1 [2] For init=bb3 test=bb1 loop=bb5 update=bb4 fallthrough=bb2 bb3: predecessor blocks: bb0 - [3] Const mutate i$8_@1[3:5] = 0 + [3] Let mutate i$8_@1[1:15] = 0 [4] Goto bb1 bb1: predecessor blocks: bb3 bb4 - [5] Const mutate $9_@2 = 10 - [6] Const mutate $11_@3[6:8] = Binary read i$8_@1 < read $9_@2 - [7] If (read $11_@3) then:bb5 else:bb2 fallthrough=bb2 + [5] Const mutate $9_@1[1:15] = 10 + [6] Const mutate $11_@1[1:15] = Binary read i$8_@1 < read $9_@1 + [7] If (read $11_@1) then:bb5 else:bb2 fallthrough=bb2 bb5: predecessor blocks: bb1 - [8] Const mutate $12_@4 = 1 - [9] Reassign mutate x$7_@0[1:13] = Binary read x$7_@0 + read $12_@4 + [8] Const mutate $12_@3 = 1 + [9] Reassign mutate x$7_@1[1:15] = Binary read x$7_@1 + read $12_@3 [10] Goto(Continue) bb4 bb4: predecessor blocks: bb5 - [11] Call mutate update$3_@5() - [12] Goto bb1 + [11] Const mutate $15_@1[1:15] = 1 + [12] Reassign mutate i$8_@1[1:15] = Binary read i$8_@1 + read $15_@1 + [13] read i$8_@1 + [14] Goto bb1 bb2: predecessor blocks: bb1 - [13] Return read x$7_@0 -scope3 [6:8]: - - dependency: read i$8_@1 - - dependency: read $9_@2 + [15] Return read x$7_@1 + ``` ### CFG @@ -51,38 +51,40 @@ flowchart TB %% Basic Blocks subgraph bb0 bb0_instrs[" - [1] Let mutate x$7_@0[1:13] = 1 + [1] Let mutate x$7_@1[1:15] = 1 "] bb0_instrs --> bb0_terminal(["For"]) end subgraph bb3 bb3_instrs[" - [3] Const mutate i$8_@1[3:5] = 0 + [3] Let mutate i$8_@1[1:15] = 0 "] bb3_instrs --> bb3_terminal(["Goto"]) end subgraph bb1 bb1_instrs[" - [5] Const mutate $9_@2 = 10 - [6] Const mutate $11_@3[6:8] = Binary read i$8_@1 < read $9_@2 + [5] Const mutate $9_@1[1:15] = 10 + [6] Const mutate $11_@1[1:15] = Binary read i$8_@1 < read $9_@1 "] - bb1_instrs --> bb1_terminal(["If (read $11_@3)"]) + bb1_instrs --> bb1_terminal(["If (read $11_@1)"]) end subgraph bb5 bb5_instrs[" - [8] Const mutate $12_@4 = 1 - [9] Reassign mutate x$7_@0[1:13] = Binary read x$7_@0 + read $12_@4 + [8] Const mutate $12_@3 = 1 + [9] Reassign mutate x$7_@1[1:15] = Binary read x$7_@1 + read $12_@3 "] bb5_instrs --> bb5_terminal(["Goto"]) end subgraph bb4 bb4_instrs[" - [11] Call mutate update$3_@5() + [11] Const mutate $15_@1[1:15] = 1 + [12] Reassign mutate i$8_@1[1:15] = Binary read i$8_@1 + read $15_@1 + [13] read i$8_@1 "] bb4_instrs --> bb4_terminal(["Goto"]) end subgraph bb2 - bb2_terminal(["Return read x$7_@0"]) + bb2_terminal(["Return read x$7_@1"]) end %% Jumps @@ -104,7 +106,7 @@ flowchart TB ```javascript function foo$0() { let x$7 = 1; - bb2: for (const i$8 = 0; i$8 < 10; update$3()) { + bb2: for (let i$8 = 0; i$8 < 10; i$8 = i$8 + 1, i$8) { x$7 = x$7 + 1; } diff --git a/compiler/forget/src/__tests__/fixtures/hir/ssa-for.js b/compiler/forget/src/__tests__/fixtures/hir/ssa-for.js index 950e2450e7..455f279915 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/ssa-for.js +++ b/compiler/forget/src/__tests__/fixtures/hir/ssa-for.js @@ -1,6 +1,6 @@ function foo() { let x = 1; - for (let i = 0; i < 10; update()) { + for (let i = 0; i < 10; i += 1) { x += 1; } return x; diff --git a/compiler/forget/src/__tests__/fixtures/hir/ssa-while-no-reassign.expect.md b/compiler/forget/src/__tests__/fixtures/hir/ssa-while-no-reassign.expect.md index 088e4e6766..2307b9a5b3 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/ssa-while-no-reassign.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/ssa-while-no-reassign.expect.md @@ -21,7 +21,7 @@ bb0: [2] While test=bb1 loop=bb3 fallthrough=bb2 bb1: predecessor blocks: bb0 bb3 - [3] Const mutate $6_@1 = 10 + [3] Const mutate $6_@1[3:6] = 10 [4] Const mutate $8_@2[4:6] = Binary read x$5_@0 < read $6_@1 [5] If (read $8_@2) then:bb3 else:bb2 fallthrough=bb2 bb3: @@ -34,7 +34,6 @@ bb2: [9] Return read x$5_@0 scope2 [4:6]: - dependency: read x$5_@0 - - dependency: read $6_@1 scope3 [6:7]: - dependency: read x$5_@0 ``` @@ -52,7 +51,7 @@ flowchart TB end subgraph bb1 bb1_instrs[" - [3] Const mutate $6_@1 = 10 + [3] Const mutate $6_@1[3:6] = 10 [4] Const mutate $8_@2[4:6] = Binary read x$5_@0 < read $6_@1 "] bb1_instrs --> bb1_terminal(["If (read $8_@2)"]) diff --git a/compiler/forget/src/__tests__/fixtures/hir/ssa-while.expect.md b/compiler/forget/src/__tests__/fixtures/hir/ssa-while.expect.md index 8fda351553..32e88277ad 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/ssa-while.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/ssa-while.expect.md @@ -21,8 +21,8 @@ bb0: [2] While test=bb1 loop=bb3 fallthrough=bb2 bb1: predecessor blocks: bb0 bb3 - [3] Const mutate $6_@1 = 10 - [4] Const mutate $8_@0[1:9] = Binary read x$5_@0 < read $6_@1 + [3] Const mutate $6_@0[1:9] = 10 + [4] Const mutate $8_@0[1:9] = Binary read x$5_@0 < read $6_@0 [5] If (read $8_@0) then:bb3 else:bb2 fallthrough=bb2 bb3: predecessor blocks: bb1 @@ -48,8 +48,8 @@ flowchart TB end subgraph bb1 bb1_instrs[" - [3] Const mutate $6_@1 = 10 - [4] Const mutate $8_@0[1:9] = Binary read x$5_@0 < read $6_@1 + [3] Const mutate $6_@0[1:9] = 10 + [4] Const mutate $8_@0[1:9] = Binary read x$5_@0 < read $6_@0 "] bb1_instrs --> bb1_terminal(["If (read $8_@0)"]) end