Add invariant for problematic LeaveSSA case
The new LeaveSSA looks ahead to the phis of fallback blocks. However, HIR can sometimes have multiple blocks with the same fallthrough (totally fine), so this diff clears the phis of fallbacks as they are reached to avoid reprocessing them. This caused a previously incorrect case to now fail, yay.
Joe Savona committed
Dec 14, 2022 at 08:42 UTC
73ad571c6b52cd7e5321ab52c81c5a133fd1d201
7 files changed
+72
-97
compiler/forget/src/HIR/HIRBuilder.ts
+11
-7
@@ -23,6 +23,7 @@ import {
23
makeType,
24
Terminal,
25
} from "./HIR";
26
+import { logHIR } from "./logger";
27
import { printInstruction } from "./PrintHIR";
28
import { eachTerminalSuccessor, mapTerminalSuccessors } from "./visitors";
29
@@ -156,16 +157,19 @@ export default class HIRBuilder {
157
preds: new Set(),
158
phis: new Set(),
159
});
159
- // First reduce indirections and prune unreachable blocks
160
- let reduced = shrink({
160
+ let ir: HIR = {
161
blocks: this.#completed,
162
entry: this.#entry,
163
- });
163
+ };
164
+ logHIR("Build (pre-shrink)", ir);
165
+ // First reduce indirections and prune unreachable blocks
166
+ let shrunk = shrink(ir);
167
+ logHIR("Build (shrunk)", shrunk);
168
// then convert to reverse postorder
165
- const blocks = reversePostorderBlocks(reduced);
166
- markInstructionIds(blocks);
167
- markPredecessors(blocks);
168
- return blocks;
169
+ const rpo = reversePostorderBlocks(shrunk);
170
+ markInstructionIds(rpo);
171
+ markPredecessors(rpo);
172
+ return rpo;
173
}
174
175
/**
compiler/forget/src/HIR/LeaveSSA.ts
+13
-2
@@ -5,6 +5,7 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
+import invariant from "invariant";
9
import {
10
Effect,
11
GeneratedSource,
@@ -34,6 +35,11 @@ export function leaveSSA(fn: HIRFunction) {
35
const hasDeclaration: Set<Identifier> = new Set();
36
37
for (const [, block] of fn.body.blocks) {
38
+ invariant(
39
+ block.phis.size === 0,
40
+ "Expected all phis to be cleared by predecessors"
41
+ );
42
+
43
// Identifiers (from phis) that *may* need a new `let` declaration created. If the original
44
// variable declaration flows into the phi, then we can reuse its declaration - this is
45
// discovered during iteration of instructions.
@@ -53,18 +59,25 @@ export function leaveSSA(fn: HIRFunction) {
59
) {
60
const fallthrough = fn.body.blocks.get(terminal.fallthrough)!;
61
phis.push(...fallthrough.phis);
62
+ fallthrough.phis.clear();
63
}
64
if (terminal.kind === "while" || terminal.kind === "for") {
65
const test = fn.body.blocks.get(terminal.test)!;
66
phis.push(...test.phis);
67
+ test.phis.clear();
68
+
69
const loop = fn.body.blocks.get(terminal.loop)!;
70
phis.push(...loop.phis);
71
+ loop.phis.clear();
72
}
73
if (terminal.kind === "for") {
74
const init = fn.body.blocks.get(terminal.init)!;
75
phis.push(...init.phis);
76
+ init.phis.clear();
77
+
78
const update = fn.body.blocks.get(terminal.update)!;
79
phis.push(...update.phis);
80
+ update.phis.clear();
81
82
// find declarations in the for init
83
for (const instr of init.instructions) {
@@ -174,8 +187,6 @@ export function leaveSSA(fn: HIRFunction) {
187
};
188
block.instructions.push(instr);
189
}
177
-
178
- block.phis.clear();
190
}
191
}
192
compiler/forget/src/HIR/Pipeline.ts
+11
-16
@@ -18,9 +18,8 @@ import { inferMutableRanges } from "./InferMutableRanges";
18
import { inferReactiveScopeDependencies } from "./InferReactiveScopeDependencies";
19
import { inferReactiveScopes } from "./InferReactiveScopes";
20
import { inferReactiveScopeVariables } from "./InferReactiveScopeVariables";
21
-import { log } from "./logger";
22
-import { printFunction } from "./PrintHIR";
21
import { inferTypes } from "./InferTypes";
22
+import { logHIRFunction } from "./logger";
23
24
export type CompilerFlags = {
25
eliminateRedundantPhi: boolean;
@@ -46,47 +45,47 @@ export default function (
45
const env = new Environment();
46
47
const ir = lower(func, env);
49
- logStep("HIR", ir);
48
+ logHIRFunction("HIR", ir);
49
50
enterSSA(ir, env);
52
- logStep("SSA", ir);
51
+ logHIRFunction("SSA", ir);
52
53
if (flags.eliminateRedundantPhi) {
54
eliminateRedundantPhi(ir);
56
- logStep("eliminateRedundantPhi", ir);
55
+ logHIRFunction("eliminateRedundantPhi", ir);
56
}
57
if (flags.inferTypes) {
58
inferTypes(ir);
60
- logStep("inferTypes", ir);
59
+ logHIRFunction("inferTypes", ir);
60
}
61
if (flags.inferReferenceEffects) {
62
inferReferenceEffects(ir);
64
- logStep("inferReferenceEffects", ir);
63
+ logHIRFunction("inferReferenceEffects", ir);
64
}
65
66
if (flags.inferMutableRanges) {
67
inferMutableRanges(ir);
69
- logStep("inferMutableRanges", ir);
68
+ logHIRFunction("inferMutableRanges", ir);
69
}
70
71
if (flags.leaveSSA) {
72
leaveSSA(ir);
74
- logStep("leaveSSA", ir);
73
+ logHIRFunction("leaveSSA", ir);
74
}
75
76
if (flags.inferReactiveScopeVariables) {
77
inferReactiveScopeVariables(ir);
79
- logStep("inferReactiveScopeVariables", ir);
78
+ logHIRFunction("inferReactiveScopeVariables", ir);
79
}
80
81
if (flags.inferReactiveScopes) {
82
inferReactiveScopes(ir);
84
- logStep("inferReactiveScopes", ir);
83
+ logHIRFunction("inferReactiveScopes", ir);
84
}
85
86
if (flags.inferReactiveScopeDependencies) {
87
inferReactiveScopeDependencies(ir);
89
- logStep("inferReactiveScopeDependencies", ir);
88
+ logHIRFunction("inferReactiveScopeDependencies", ir);
89
}
90
91
if (flags.codegen) {
@@ -98,7 +97,3 @@ export default function (
97
98
return { ast: null, ir: ir };
99
}
101
-
102
-function logStep(step: string, ir: HIRFunction) {
103
- log(() => `${step}:\n${printFunction(ir)}`);
104
-}
compiler/forget/src/HIR/logger.ts
+11
@@ -5,12 +5,23 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
+import { HIR, HIRFunction } from "./HIR";
9
+import printHIR, { printFunction } from "./PrintHIR";
10
+
11
let ENABLED: boolean = false;
12
13
export function toggleLogging(enabled: boolean) {
14
ENABLED = enabled;
15
}
16
17
+export function logHIR(step: string, ir: HIR): void {
18
+ log(() => `${step}:\n${printHIR(ir)}`);
19
+}
20
+
21
+export function logHIRFunction(step: string, fn: HIRFunction): void {
22
+ log(() => `${step}:\n${printFunction(fn)}`);
23
+}
24
+
25
export function log(fn: () => string) {
26
if (ENABLED) {
27
const message = fn();
compiler/forget/src/__tests__/fixtures/hir/_bug_inverted-if-else.expect.md
deleted
-72
@@ -1,72 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-function foo(a, b, c) {
6
- let x = null;
7
- label: {
8
- if (a) {
9
- x = b;
10
- break label;
11
- }
12
- x = c;
13
- }
14
- return x;
15
-}
16
-
17
-```
18
-
19
-## HIR
20
-
21
-```
22
-bb0:
23
- [1] Const mutate x$8_@0:TPrimitive = null
24
- [2] If (read a$5) then:bb3 else:bb2 fallthrough=bb2
25
-bb3:
26
- predecessor blocks: bb0
27
- [3] Const mutate x$9_@1 = read b$6
28
- [4] Goto bb1
29
-bb2:
30
- predecessor blocks: bb0
31
- [5] Const mutate x$10_@2 = read c$7
32
- [6] Goto bb1
33
-bb1:
34
- predecessor blocks: bb3 bb2
35
- [7] Return read x$11
36
-scope1 [3:4]:
37
- - dependency: read b$6
38
-scope2 [5:6]:
39
- - dependency: read c$7
40
-```
41
-
42
-## Reactive Scopes
43
-
44
-```
45
-function foo(
46
- a,
47
- b,
48
- c,
49
-) {
50
- [1] Const mutate x$8_@0:TPrimitive = null
51
- if (read a$5) {
52
- [3] Const mutate x$9_@1 = read b$6
53
- }
54
- [5] Const mutate x$10_@2 = read c$7
55
-}
56
-
57
-```
58
-
59
-## Code
60
-
61
-```javascript
62
-function foo$0(a$5, b$6, c$7) {
63
- const x$8 = null;
64
- bb2: if (a$5) {
65
- const x$9 = b$6;
66
- }
67
-
68
- const x$10 = c$7;
69
-}
70
-
71
-```
72
-
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/error.inverted-if-else.expect.md
new
+26
@@ -0,0 +1,26 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function foo(a, b, c) {
6
+ let x = null;
7
+ label: {
8
+ if (a) {
9
+ x = b;
10
+ break label;
11
+ }
12
+ x = c;
13
+ }
14
+ return x;
15
+}
16
+
17
+```
18
+
19
+
20
+## Error
21
+
22
+```
23
+Expected all phis to be cleared by predecessors
24
+```
25
+
26
+
\ No newline at end of file