Fix PruneMaybeThrows to update phi operand predecessor ids

When PruneMaybeThrows removes maybe-throw terminals, it's possible that the 
block in question reassigned a value s.t. it appears as a later phi operand. 
That phi has to be rewritten to reflect the updated predecessor block. 

Here we track these rewrites (transitively) and rewrite phi operands 
accordingly.
This commit is contained in:
Joe Savona
2024-03-21 14:11:04 -07:00
parent 1cf8d9bc8b
commit 241b35463c
4 changed files with 96 additions and 48 deletions
@@ -5,7 +5,10 @@
* LICENSE file in the root directory of this source tree.
*/
import { CompilerError } from "..";
import {
BlockId,
GeneratedSource,
GotoVariant,
HIRFunction,
Instruction,
@@ -17,12 +20,11 @@ import {
} from "../HIR";
import {
markInstructionIds,
markPredecessors,
removeDeadDoWhileStatements,
removeUnnecessaryTryCatch,
removeUnreachableForUpdates,
} from "../HIR/HIRBuilder";
import { eliminateRedundantPhi } from "../SSA";
import { printIdentifier } from "../HIR/PrintHIR";
/*
* This pass prunes `maybe-throw` terminals for blocks that can provably *never* throw.
@@ -30,8 +32,8 @@ import { eliminateRedundantPhi } from "../SSA";
* array/object literals. Even a variable reference could throw bc of the TDZ.
*/
export function pruneMaybeThrows(fn: HIRFunction): void {
const didPrune = pruneMaybeThrowsImpl(fn);
if (didPrune) {
const terminalMapping = pruneMaybeThrowsImpl(fn);
if (terminalMapping) {
/*
* If terminals have changed then blocks may have become newly unreachable.
* Re-run minification of the graph (incl reordering instruction ids)
@@ -42,36 +44,36 @@ export function pruneMaybeThrows(fn: HIRFunction): void {
removeDeadDoWhileStatements(fn.body);
removeUnnecessaryTryCatch(fn.body);
markInstructionIds(fn.body);
markPredecessors(fn.body);
mergeConsecutiveBlocks(fn);
// Now that predecessors are updated, prune phi operands that can never be reached
// Rewrite phi operands to reference the updated predecessor blocks
for (const [, block] of fn.body.blocks) {
for (const phi of block.phis) {
for (const [predecessor] of phi.operands) {
for (const [predecessor, operand] of phi.operands) {
if (!block.preds.has(predecessor)) {
const mappedTerminal = terminalMapping.get(predecessor);
CompilerError.invariant(mappedTerminal != null, {
reason: `Expected non-existing phi operand's predecessor to have been mapped to a new terminal`,
loc: GeneratedSource,
description: `Could not find mapping for predecessor bb${predecessor} in block bb${
block.id
} for phi ${printIdentifier(phi.id)}`,
suggestions: null,
});
phi.operands.delete(predecessor);
phi.operands.set(mappedTerminal, operand);
}
}
}
}
/*
* By removing some phi operands, there may be phis that were not previously
* redundant but now are
*/
eliminateRedundantPhi(fn);
/*
* Finally, merge together any blocks that are now guaranteed to execute
* consecutively
*/
mergeConsecutiveBlocks(fn);
assertConsistentIdentifiers(fn);
assertTerminalSuccessorsExist(fn);
}
}
function pruneMaybeThrowsImpl(fn: HIRFunction): boolean {
let hasChanges = false;
function pruneMaybeThrowsImpl(fn: HIRFunction): Map<BlockId, BlockId> | null {
const terminalMapping = new Map<BlockId, BlockId>();
for (const [_, block] of fn.body.blocks) {
const terminal = block.terminal;
if (terminal.kind !== "maybe-throw") {
@@ -81,7 +83,8 @@ function pruneMaybeThrowsImpl(fn: HIRFunction): boolean {
instructionMayThrow(instr)
);
if (!canThrow) {
hasChanges = true;
const source = terminalMapping.get(block.id) ?? block.id;
terminalMapping.set(terminal.continuation, source);
block.terminal = {
kind: "goto",
block: terminal.continuation,
@@ -91,7 +94,7 @@ function pruneMaybeThrowsImpl(fn: HIRFunction): boolean {
};
}
}
return hasChanges;
return terminalMapping.size > 0 ? terminalMapping : null;
}
function instructionMayThrow(instr: Instruction): boolean {
@@ -1,13 +0,0 @@
// @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions
function useSupportsTouchEvent() {
return useMemo(() => {
if (checkforTouchEvents) {
try {
document.createEvent("TouchEvent");
return true;
} catch {
return false;
}
}
}, []);
}
@@ -0,0 +1,64 @@
## Input
```javascript
// @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions
import { useMemo } from "react";
const checkforTouchEvents = true;
function useSupportsTouchEvent() {
return useMemo(() => {
if (checkforTouchEvents) {
try {
document.createEvent("TouchEvent");
return true;
} catch {
return false;
}
}
}, []);
}
export const FIXTURE_ENTRYPOINT = {
fn: useSupportsTouchEvent,
params: [],
};
```
## Code
```javascript
// @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions
import { useMemo } from "react";
const checkforTouchEvents = true;
function useSupportsTouchEvent() {
let t0;
bb15: {
if (checkforTouchEvents) {
try {
document.createEvent("TouchEvent");
t0 = true;
break bb15;
} catch {
t0 = false;
break bb15;
}
}
t0 = undefined;
}
return t0;
}
export const FIXTURE_ENTRYPOINT = {
fn: useSupportsTouchEvent,
params: [],
};
```
### Eval output
(kind: ok) true
@@ -1,8 +1,8 @@
## Input
```javascript
// @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions
import { useMemo } from "react";
const checkforTouchEvents = true;
function useSupportsTouchEvent() {
return useMemo(() => {
if (checkforTouchEvents) {
@@ -16,13 +16,7 @@ function useSupportsTouchEvent() {
}, []);
}
```
## Error
```
Cannot read properties of undefined (reading 'preds')
```
export const FIXTURE_ENTRYPOINT = {
fn: useSupportsTouchEvent,
params: [],
};