LeaveSSA: handle phi as operand to subsequent phi

Fixes https://github.com/facebook/react-forget/pull/858#discussion_r1044626137. 
The case is 

```javascript 

function foo() { 

let x$1 = 1; 

let y = 2; 

if (y === 2) { 

x$2 = 3; 

} 

x$3 = phi(x$1, x$2) 

if (y === 3) { 

x4 = 5; 

} 

x$5 = phi(x$3, x$4); 

y = x$5; 

} 

``` 

What happens here is that there are two _sequential_ phis for `x`. Previously 
when we encountered the second phi we would find that there is no `let` 
declaration for the phi or any of its operands, and create a new one before the 
second `if`. That's incorrect, these should all merge into a single `x` 
declaration. We now look up the phi operands to see if they are part of a 
previous phi, and merge them correctly.
This commit is contained in:
Joseph Savona
2022-12-09 09:47:47 -08:00
parent d03c5bc0a0
commit 748992d508
6 changed files with 66 additions and 19 deletions
+3 -2
View File
@@ -78,8 +78,9 @@ export function leaveSSA(fn: HIRFunction) {
// options we'll choose it and can reuse the declaration.
canonicalId = phi.id;
for (const [, operand] of phi.operands) {
if (operand.id < canonicalId.id) {
canonicalId = operand;
let canonicalOperand = variableMapping.get(operand) ?? operand;
if (canonicalOperand.id < canonicalId.id) {
canonicalId = canonicalOperand;
}
}
canonicalId.mutableRange.start = Math.min(
+25
View File
@@ -18,6 +18,8 @@ import { inferMutableRanges } from "./InferMutableRanges";
import { inferReactiveScopeDependencies } from "./InferReactiveScopeDependencies";
import { inferReactiveScopes } from "./InferReactiveScopes";
import { inferReactiveScopeVariables } from "./InferReactiveScopeVariables";
import { log } from "./logger";
import { printFunction } from "./PrintHIR";
export type CompilerFlags = {
eliminateRedundantPhi: boolean;
@@ -40,29 +42,48 @@ export default function (
flags: CompilerFlags
): CompilerResult {
const env = new Environment();
const ir = lower(func, env);
logStep("HIR", ir);
enterSSA(ir, env);
logStep("SSA", ir);
if (flags.eliminateRedundantPhi) {
eliminateRedundantPhi(ir);
logStep("eliminateRedundantPhi", ir);
}
if (flags.inferReferenceEffects) {
inferReferenceEffects(ir);
logStep("inferReferenceEffects", ir);
}
if (flags.inferMutableRanges) {
inferMutableRanges(ir);
logStep("inferMutableRanges", ir);
}
if (flags.leaveSSA) {
leaveSSA(ir);
logStep("leaveSSA", ir);
}
if (flags.inferReactiveScopeVariables) {
inferReactiveScopeVariables(ir);
logStep("inferReactiveScopeVariables", ir);
}
if (flags.inferReactiveScopes) {
inferReactiveScopes(ir);
logStep("inferReactiveScopes", ir);
}
if (flags.inferReactiveScopeDependencies) {
inferReactiveScopeDependencies(ir);
logStep("inferReactiveScopeDependencies", ir);
}
if (flags.codegen) {
return {
ast: codegen(ir),
@@ -72,3 +93,7 @@ export default function (
return { ast: null, ir: ir };
}
function logStep(step: string, ir: HIRFunction) {
log(() => `${step}:\n${printFunction(ir)}`);
}
+19
View File
@@ -0,0 +1,19 @@
/**
* 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.
*/
let ENABLED: boolean = false;
export function toggleLogging(enabled: boolean) {
ENABLED = enabled;
}
export function log(fn: () => string) {
if (ENABLED) {
const message = fn();
process.stdout.write(message.trim() + "\n\n");
}
}
@@ -21,28 +21,27 @@ function foo() {
```
bb0:
[1] Let mutate x$7_@0[1:8] = 1
[1] Let mutate x$7_@0[1:14] = 1
[2] Const mutate y$8_@1 = 2
[3] Const mutate $9_@2 = 2
[4] Const mutate $10_@3 = Binary read y$8_@1 === read $9_@2
[5] If (read $10_@3) then:bb2 else:bb1 fallthrough=bb1
bb2:
predecessor blocks: bb0
[6] Reassign mutate x$7_@0[1:8] = 3
[6] Reassign mutate x$7_@0[1:14] = 3
[7] Goto bb1
bb1:
predecessor blocks: bb2 bb0
[8] Const mutate $12_@4 = 3
[9] Const mutate $14_@5 = Binary read y$8_@1 === read $12_@4
[10] Let mutate x$15_@6[1:14] = undefined
[10] If (read $14_@5) then:bb4 else:bb3 fallthrough=bb3
bb4:
predecessor blocks: bb1
[11] Reassign mutate x$15_@6[1:14] = 5
[11] Reassign mutate x$7_@0[1:14] = 5
[12] Goto bb3
bb3:
predecessor blocks: bb4 bb1
[13] Const mutate y$18_@6[1:14] = read x$15_@6
[13] Const mutate y$18_@0[1:14] = read x$7_@0
[14] Return
scope3 [4:5]:
- read y$8_@1
@@ -59,7 +58,7 @@ flowchart TB
%% Basic Blocks
subgraph bb0
bb0_instrs["
[1] Let mutate x$7_@0[1:8] = 1
[1] Let mutate x$7_@0[1:14] = 1
[2] Const mutate y$8_@1 = 2
[3] Const mutate $9_@2 = 2
[4] Const mutate $10_@3 = Binary read y$8_@1 === read $9_@2
@@ -68,7 +67,7 @@ flowchart TB
end
subgraph bb2
bb2_instrs["
[6] Reassign mutate x$7_@0[1:8] = 3
[6] Reassign mutate x$7_@0[1:14] = 3
"]
bb2_instrs --> bb2_terminal(["Goto"])
end
@@ -76,19 +75,18 @@ flowchart TB
bb1_instrs["
[8] Const mutate $12_@4 = 3
[9] Const mutate $14_@5 = Binary read y$8_@1 === read $12_@4
[10] Let mutate x$15_@6[1:14] = undefined
"]
bb1_instrs --> bb1_terminal(["If (read $14_@5)"])
end
subgraph bb4
bb4_instrs["
[11] Reassign mutate x$15_@6[1:14] = 5
[11] Reassign mutate x$7_@0[1:14] = 5
"]
bb4_instrs --> bb4_terminal(["Goto"])
end
subgraph bb3
bb3_instrs["
[13] Const mutate y$18_@6[1:14] = read x$15_@6
[13] Const mutate y$18_@0[1:14] = read x$7_@0
"]
bb3_instrs --> bb3_terminal(["Return"])
end
@@ -113,13 +111,11 @@ function foo$0() {
x$7 = 3;
}
let x$15 = undefined;
bb3: if (y$8 === 3) {
x$15 = 5;
x$7 = 5;
}
const y$18 = x$15;
const y$18 = x$7;
}
```
+5 -1
View File
@@ -14,6 +14,7 @@ import { wasmFolder } from "@hpcc-js/wasm";
import invariant from "invariant";
import path from "path";
import prettier from "prettier";
import { toggleLogging } from "../HIR/logger";
import run from "../HIR/Pipeline";
import { printFunction } from "../HIR/PrintHIR";
import visualizeHIRMermaid from "../HIR/VisualizeHIRMermaid";
@@ -34,7 +35,7 @@ const Pragma_RE = /\/\/\s*@enable\((\w+)\)$/gm;
describe("React Forget (HIR version)", () => {
generateTestsFromFixtures(
path.join(__dirname, "fixtures", "hir"),
(input, file) => {
(input, file, options) => {
const matches = input.matchAll(Pragma_RE);
for (const match of matches) {
@@ -50,6 +51,9 @@ describe("React Forget (HIR version)", () => {
let items: Array<[string, string, string]> | null = null;
let error: Error | null = null;
if (options.debug) {
toggleLogging(options.debug);
}
try {
items = transform(input, file);
} catch (e) {
@@ -42,7 +42,7 @@ expect.extend({
export default function generateTestsFromFixtures(
fixturesPath: string,
transform: (input: string, file: any) => string
transform: (input: string, file: any, options: { debug: boolean }) => string
) {
const files = fs.readdirSync(fixturesPath);
const fixtures = matchInputOutputFixtures(files, fixturesPath);
@@ -87,18 +87,20 @@ export default function generateTestsFromFixtures(
}
let input: string | null = null;
let debug = false;
if (inputFile != null) {
input = fs.readFileSync(inputFile, "utf8");
const lines = input.split("\n");
if (lines[0]!.indexOf("@only") !== -1) {
testCommand = test.only;
debug = true;
}
}
testCommand(basename, () => {
let receivedOutput;
if (input !== null) {
receivedOutput = transform(input, basename);
receivedOutput = transform(input, basename, { debug });
} else {
receivedOutput = "<<input deleted>>";
}