@samitouri / QOS-React-2 / commits / 0abc420c78

[be] Preds/phis use BlockId since blocks may change

Phi operands and Block predecessors currently use a `BasicBlock` reference rather than the BlockId. This diverges from other places (like terminals) where we use an id and not a direct object reference. Especially since blocks may get rewritten or pruned, it's a bit cleaner to use the block id in these places.

Joe Savona committed Jan 9, 2023 at 09:22 UTC 0abc420c78ecee971074bae51386312a5614e94a
6 files changed +22 -17
compiler/forget/src/HIR/HIR.ts
+2 -2
@@ -145,7 +145,7 @@ export type BasicBlock = {
145 id: BlockId;
146 instructions: Array<Instruction>;
147 terminal: Terminal;
148 - preds: Set<BasicBlock>;
148 + preds: Set<BlockId>;
149 phis: Set<Phi>;
150 };
151
@@ -272,7 +272,7 @@ export type InstructionValue =
272 export type Phi = {
273 kind: "Phi";
274 id: Identifier;
275 - operands: Map<BasicBlock, Identifier>;
275 + operands: Map<BlockId, Identifier>;
276 };
277
278 export type InstructionData =
compiler/forget/src/HIR/HIRBuilder.ts
+1 -1
@@ -540,7 +540,7 @@ function markPredecessors(func: HIR) {
540 function visit(blockId: BlockId, prevBlock: BasicBlock | null) {
541 const block = func.blocks.get(blockId)!;
542 if (prevBlock) {
543 - block.preds.add(prevBlock);
543 + block.preds.add(prevBlock.id);
544 }
545
546 if (visited.has(blockId)) {
compiler/forget/src/HIR/PrintHIR.ts
+3 -3
@@ -51,7 +51,7 @@ export default function printHIR(
51 if (block.preds.size > 0) {
52 const preds = ["predecessor blocks:"];
53 for (const pred of block.preds) {
54 - preds.push(`bb${pred.id}`);
54 + preds.push(`bb${pred}`);
55 }
56 push(preds.join(" "));
57 }
@@ -114,8 +114,8 @@ function printPhi(phi: Phi): string {
114 items.push(printMutableRange(phi.id));
115 items.push(": phi(");
116 const phis = [];
117 - for (const [block, id] of phi.operands) {
118 - phis.push(`bb${block.id}: ${printIdentifier(id)}`);
117 + for (const [blockId, id] of phi.operands) {
118 + phis.push(`bb${blockId}: ${printIdentifier(id)}`);
119 }
120
121 items.push(phis.join(", "));
compiler/forget/src/SSA/EliminateRedundantPhi.ts
+2 -2
@@ -43,8 +43,8 @@ export function eliminateRedundantPhi(fn: HIRFunction) {
43 // On the first iteration of the loop check for any back-edges.
44 // if there aren't any then there won't be a second iteration
45 if (!hasBackEdge) {
46 - for (const pred of block.preds) {
47 - if (!visited.has(pred.id)) {
46 + for (const predId of block.preds) {
47 + if (!visited.has(predId)) {
48 hasBackEdge = true;
49 }
50 }
compiler/forget/src/SSA/EnterSSA.ts
+12 -8
@@ -1,5 +1,6 @@
1 import {
2 BasicBlock,
3 + BlockId,
4 HIRFunction,
5 Identifier,
6 IdentifierId,
@@ -32,9 +33,11 @@ class SSABuilder {
33 #states: Map<BasicBlock, State> = new Map();
34 #current: BasicBlock | null = null;
35 unsealedPreds: Map<BasicBlock, number> = new Map();
36 + #blocks: Map<BlockId, BasicBlock>;
37 #env: Environment;
38
37 - constructor(env: Environment) {
39 + constructor(env: Environment, blocks: Map<BlockId, BasicBlock>) {
40 + this.#blocks = blocks;
41 this.#env = env;
42 }
43
@@ -74,15 +77,16 @@ class SSABuilder {
77 }
78
79 getPlace(oldPlace: Place): Place {
77 - const newId = this.getIdAt(oldPlace.identifier, this.#current!);
80 + const newId = this.getIdAt(oldPlace.identifier, this.#current!.id);
81 return {
82 ...oldPlace,
83 identifier: newId,
84 };
85 }
86
84 - getIdAt(oldId: Identifier, block: BasicBlock): Identifier {
87 + getIdAt(oldId: Identifier, blockId: BlockId): Identifier {
88 // check if Place is defined locally
89 + const block = this.#blocks.get(blockId)!;
90 const state = this.#states.get(block)!;
91
92 if (state.defs.has(oldId)) {
@@ -124,10 +128,10 @@ class SSABuilder {
128 }
129
130 addPhi(block: BasicBlock, oldId: Identifier, newId: Identifier): Identifier {
127 - const predDefs: Map<BasicBlock, Identifier> = new Map();
128 - for (const predBlock of block.preds) {
129 - const predId = this.getIdAt(oldId, predBlock);
130 - predDefs.set(predBlock, predId);
131 + const predDefs: Map<BlockId, Identifier> = new Map();
132 + for (const predBlockId of block.preds) {
133 + const predId = this.getIdAt(oldId, predBlockId);
134 + predDefs.set(predBlockId, predId);
135 }
136
137 const phi: Phi = {
@@ -179,7 +183,7 @@ class SSABuilder {
183
184 export default function enterSSA(func: HIRFunction, env: Environment) {
185 const visitedBlocks: Set<BasicBlock> = new Set();
182 - const builder = new SSABuilder(env);
186 + const builder = new SSABuilder(env, func.body.blocks);
187 for (const [blockId, block] of func.body.blocks) {
188 invariant(
189 !visitedBlocks.has(block),
compiler/forget/src/SSA/LeaveSSA.ts
+2 -1
@@ -216,10 +216,11 @@ export function leaveSSA(fn: HIRFunction) {
216 }
217
218 // Generate an assignment in each predecessor
219 - for (const [predecessor, operand] of phi.operands) {
219 + for (const [predecessorId, operand] of phi.operands) {
220 if (operand === initOperand) {
221 continue;
222 }
223 + const predecessor = fn.body.blocks.get(predecessorId)!;
224 const instr: Instruction = {
225 id: predecessor.terminal.id,
226 lvalue: {