Pass to eliminate redundant phis

This is an alternate take on phi elimination to the one we pursued over VC w @poteto driving. This version exploits the RPO ordering of blocks to do phi elimination in a single pass when there are no loops, and to minimize repeated visits when there are loops. The main difference is when redundant phis are removed. Rather than eagerly walking through the CFG for each pruned phi to rewrite its uses, we build up a mapping of rewritten identifiers. As we walk through subsequent instructions, we rewrite each place based on that mapping. We continue cycling through the blocks so long as a given iteration *both* added new rewrites (meaning there may be subsequent uses to rewrite) *and* there are back-edges. With no loops this results in a single visit of each block and of each instruction, but even with loops this is bounded.
2025-11-01 09:12:30 +00:00 · 2022-11-07 08:37:19 -08:00
parent 2c37aa78d3
commit 3a200fa4a1
12 changed files with 321 additions and 59 deletions
@@ -0,0 +1,139 @@
+/**
+ * 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.
+ */
+
+import invariant from "invariant";
+import { assertExhaustive } from "../Common/utils";
+import { BlockId, HIRFunction, Identifier, Place } from "./HIR";
+import { collectInputs } from "./InferMutableLifetimes";
+
+/**
+ * Pass to eliminate redundant phi nodes:
+ * - all operands are the same identifier, ie `x2 = phi(x1, x1, x1)`.
+ * - all operands are the same identifier *or* the output of the phi, ie `x2 = phi(x1, x2, x1, x2)`.
+ *
+ * In both these cases, the phi is eliminated and all usages of the phi identifier
+ * are replaced with the other operand (ie in both cases above, all usages of `x2` are replaced with `x1` .
+ *
+ * The algorithm is inspired by that in https://pp.info.uni-karlsruhe.de/uploads/publikationen/braun13cc.pdf
+ * but modified to reduce passes over the CFG. We visit the blocks in reverse postorder. Each time a redundant
+ * phi is encountered we add a mapping (eg x2 -> x1) to a rewrite table. Subsequent instructions, terminals,
+ * and phis rewrite all their identifiers based on this table. The algorithm loops over the CFG repeatedly
+ * until there are no new rewrites: for a CFG without back-edges it completes in a single pass.
+ */
+export function eliminateRedundantPhi(fn: HIRFunction) {
+  const ir = fn.body;
+  const rewrites: Map<Identifier, Identifier> = new Map();
+
+  // Whether or the CFG has a back-edge (a loop). We determine this dynamically
+  // during the first iteration over the CFG by recording which blocks were already
+  // visited, and checking if a block has any predecessors that weren't visited yet.
+  // Because blocks are in reverse postorder, the only time this can occur is a loop.
+  let hasBackEdge = false;
+  const visited: Set<BlockId> = new Set();
+
+  // size tracks the number of rewrites at the beginning of each iteration, so we can
+  // compare to see if any new rewrites were added in that iteration.
+  let size = rewrites.size;
+  do {
+    size = rewrites.size;
+    for (const [blockId, block] of ir.blocks) {
+      // On the first iteration of the loop check for any back-edges.
+      // if there aren't any then there won't be a second iteration
+      if (!hasBackEdge) {
+        for (const pred of block.preds) {
+          if (!visited.has(pred.id)) {
+            hasBackEdge = true;
+          }
+        }
+      }
+      visited.add(blockId);
+
+      // Find any redundant phis
+      phis: for (const phi of block.phis) {
+        let same: Identifier | null = null;
+        for (const [_, operand] of phi.operands) {
+          const ident = rewrites.get(operand) ?? operand;
+          if (
+            (same !== null && ident.id === same.id) ||
+            ident.id === phi.id.id
+          ) {
+            // This operand is the same as the phi or is the same as the
+            // previous non-phi operands
+            continue;
+          } else if (same !== null) {
+            // There are multiple operands not equal to the phi itself,
+            // this phi can't be eliminated.
+            continue phis;
+          } else {
+            // First non-phi operand
+            same = ident;
+          }
+        }
+        invariant(same !== null, "Expected phis to be non-empty");
+        rewrites.set(phi.id, same);
+        block.phis.delete(phi);
+      }
+
+      // Rewrite all instruction lvalues and operands
+      for (const instr of block.instructions) {
+        for (const place of collectInputs(instr)) {
+          rewritePlace(place, rewrites);
+        }
+        const { lvalue } = instr;
+        if (lvalue !== null) {
+          rewritePlace(lvalue.place, rewrites);
+        }
+      }
+
+      // Rewrite all terminal operands
+      const { terminal } = block;
+      switch (terminal.kind) {
+        case "if": {
+          rewritePlace(terminal.test, rewrites);
+          break;
+        }
+        case "switch": {
+          rewritePlace(terminal.test, rewrites);
+          for (const case_ of terminal.cases) {
+            if (case_.test === null) {
+              continue;
+            }
+            rewritePlace(case_.test, rewrites);
+          }
+          break;
+        }
+        case "return":
+        case "throw": {
+          if (terminal.value !== null) {
+            rewritePlace(terminal.value, rewrites);
+          }
+          break;
+        }
+        case "goto": {
+          // no-op
+          break;
+        }
+        default: {
+          assertExhaustive(
+            terminal,
+            `Unexpected terminal kind '${(terminal as any).kind}'`
+          );
+        }
+      }
+    }
+    // We only need to loop if there were newly eliminated phis in this iteration
+    // *and* the CFG has loops. If there are no loops, then all eliminated phis
+    // have already propagated forwards since we visit in reverse postorder.
+  } while (rewrites.size > size && hasBackEdge);
+}
+
+function rewritePlace(place: Place, rewrites: Map<Identifier, Identifier>) {
+  const rewrite = rewrites.get(place.identifier);
+  if (rewrite != null) {
+    place.identifier = rewrite;
+  }
+}
@@ -106,7 +106,7 @@ export function inferMutableRanges(func: HIRFunction) {
  }
 }

-function* collectInputs(instr: Instruction) {
+export function* collectInputs(instr: Instruction) {
  const instrValue = instr.value;
  switch (instrValue.kind) {
    case "NewExpression":
@@ -142,6 +142,12 @@ function* collectInputs(instr: Instruction) {
      }
      break;
    }
+    case "JsxFragment": {
+      for (const c of instrValue.children) {
+        yield c;
+      }
+      break;
+    }
    case "ObjectExpression": {
      if (instrValue.properties !== null) {
        const props = instrValue.properties;
@@ -163,7 +169,10 @@ function* collectInputs(instr: Instruction) {
      break;
    }
    default: {
-      console.log(`unhandled instruction: ${printInstruction(instr)}`);
+      assertExhaustive(
+        instrValue,
+        `Unexpected instruction kind '${(instrValue as any).kind}'`
+      );
    }
  }
 }
@@ -37,23 +37,18 @@ function Component(props) {
 bb0:
  [1] Const mutate items$27 = read props$26.items
  [2] Const mutate maxItems$28 = read props$26.maxItems
-  [3] Const mutate renderedItems$29 = Array []
-  [4] Const mutate seen$30 = New mutate Set$6()
+  [3] Const mutate renderedItems$29[3:14] = Array []
+  [4] Const mutate seen$30[4:11] = New mutate Set$6()
  [5] Const mutate $31 = 0
  [6] Const mutate max$32 = Call mutate Math$8.max(read $31, read maxItems$28)
  Goto bb1
 bb1:
  predecessor blocks: bb0 bb4 bb7
-  items$33: phi(bb0: items$27, bb4: items$33, bb7: items$33)
-  item$35[-1:11]: phi(bb0: item$10, bb4: item$35, bb7: item$35)
-  seen$38[-1:11]: phi(bb0: seen$30, bb4: seen$38, bb7: seen$38)
-  renderedItems$46[-1:14]: phi(bb0: renderedItems$29, bb4: renderedItems$46, bb7: renderedItems$46)
-  max$48: phi(bb0: max$32, bb4: max$48, bb7: max$48)
-  If (read items$33) then:bb3 else:bb2
+  If (read items$27) then:bb3 else:bb2
 bb3:
  predecessor blocks: bb1
  [7] Const mutate $34 = null
-  [8] Const mutate $36 = Binary read item$35 == read $34
+  [8] Const mutate $36 = Binary read item$10 == read $34
  If (read $36) then:bb8 else:bb9
 bb8:
  predecessor blocks: bb3
@@ -61,7 +56,7 @@ bb8:
  Goto bb7
 bb9:
  predecessor blocks: bb3
-  [10] Const mutate $39 = Call mutate seen$38.has(mutate item$35)
+  [10] Const mutate $39 = Call mutate seen$30.has(mutate item$10)
  Goto bb7
 bb7:
  predecessor blocks: bb9 bb8
@@ -69,15 +64,15 @@ bb7:
  If (read $40) then:bb1 else:bb4
 bb4:
  predecessor blocks: bb7
-  [11] Call mutate seen$38.add(mutate item$35)
+  [11] Call mutate seen$30.add(mutate item$10)
  [12] Const mutate $43 = "div"
-  [13] Const mutate $44 = JSX <read $43>{read item$35}</read $43>
-  [14] Call mutate renderedItems$46.push(read $44)
-  [15] Const mutate $49 = Binary read renderedItems$46.length >= read max$48
+  [13] Const mutate $44 = JSX <read $43>{read item$10}</read $43>
+  [14] Call mutate renderedItems$29.push(read $44)
+  [15] Const mutate $49 = Binary read renderedItems$29.length >= read max$32
  If (read $49) then:bb2 else:bb1
 bb2:
  predecessor blocks: bb1 bb4
-  [16] Const mutate count$52 = read renderedItems$46.length
+  [16] Const mutate count$52 = read renderedItems$29.length
  [17] Const mutate $53 = "div"
  [18] Const mutate $54 = "\n      "
  [19] Const mutate $55 = "h1"
@@ -85,7 +80,7 @@ bb2:
  [21] Const mutate $57 = JSX <read $55>{freeze count$52}{read $56}</read $55>
  [22] Const mutate $58 = "\n      "
  [23] Const mutate $59 = "\n    "
-  [24] Const mutate $60 = JSX <read $53>{read $54}{read $57}{read $58}{freeze renderedItems$46}{read $59}</read $53>
+  [24] Const mutate $60 = JSX <read $53>{read $54}{read $57}{read $58}{freeze renderedItems$29}{read $59}</read $53>
  Return read $60
 ```

@@ -84,8 +84,6 @@ bb1:
  b$20: phi(bb0: b$14, bb3: b$23)
  c$22: phi(bb0: c$15, bb3: c$25)
  d$24: phi(bb0: d$16, bb3: d$26)
-  mutate$27[-1:14]: phi(bb0: mutate$7, bb3: mutate$27)
-  cond$28[-1:12]: phi(bb0: cond$8, bb3: cond$28)
  [5] Const mutate $17 = true
  If (read $17) then:bb3 else:bb2
 bb3:
@@ -95,8 +93,8 @@ bb3:
  [8] Reassign mutate b$23[8:11] = read c$22
  [9] Reassign mutate c$25 = read d$24
  [10] Reassign mutate d$26 = read z$19
-  [11] Call mutate mutate$27(mutate a$21, mutate b$23)
-  [12] Const mutate $29 = Call mutate cond$28(mutate a$21)
+  [11] Call mutate mutate$7(mutate a$21, mutate b$23)
+  [12] Const mutate $29 = Call mutate cond$8(mutate a$21)
  If (read $29) then:bb2 else:bb1
 bb2:
  predecessor blocks: bb1 bb3
@@ -117,7 +115,7 @@ bb11:
 bb13:
  predecessor blocks: bb11
  [13] Const mutate $34 = null
-  [14] Call mutate mutate$27(mutate d$33, read $34)
+  [14] Call mutate mutate$7(mutate d$33, read $34)
  Return
 ```

@@ -68,42 +68,36 @@ function cond$0() {

 ```
 bb0:
-  [1] Let mutate a$12 = Object {  }
-  [2] Let mutate b$13 = Object {  }
+  [1] Let mutate a$12[1:7] = Object {  }
+  [2] Let mutate b$13[2:6] = Object {  }
  [3] Let mutate c$14 = Object {  }
-  [4] Let mutate d$15 = Object {  }
+  [4] Let mutate d$15[4:9] = Object {  }
  Goto bb1
 bb1:
  predecessor blocks: bb0 bb3
-  mutate$17[-1:9]: phi(bb0: mutate$6, bb3: mutate$17)
-  a$18[-1:7]: phi(bb0: a$12, bb3: a$18)
-  b$19[-1:6]: phi(bb0: b$13, bb3: b$19)
-  cond$20[-1:7]: phi(bb0: cond$7, bb3: cond$20)
-  c$25: phi(bb0: c$14, bb3: c$25)
-  d$27[-1:9]: phi(bb0: d$15, bb3: d$27)
  [5] Const mutate $16 = true
  If (read $16) then:bb3 else:bb2
 bb3:
  predecessor blocks: bb1
-  [6] Call mutate mutate$17(mutate a$18, mutate b$19)
-  [7] Const mutate $21 = Call mutate cond$20(mutate a$18)
+  [6] Call mutate mutate$6(mutate a$12, mutate b$13)
+  [7] Const mutate $21 = Call mutate cond$7(mutate a$12)
  If (read $21) then:bb2 else:bb1
 bb2:
  predecessor blocks: bb1 bb3
-  If (read a$18) then:bb7 else:bb7
+  If (read a$12) then:bb7 else:bb7
 bb7:
  predecessor blocks: bb2
-  If (read b$19) then:bb9 else:bb9
+  If (read b$13) then:bb9 else:bb9
 bb9:
  predecessor blocks: bb7
-  If (read c$25) then:bb11 else:bb11
+  If (read c$14) then:bb11 else:bb11
 bb11:
  predecessor blocks: bb9
-  If (read d$27) then:bb13 else:bb13
+  If (read d$15) then:bb13 else:bb13
 bb13:
  predecessor blocks: bb11
  [8] Const mutate $28 = null
-  [9] Call mutate mutate$17(mutate d$27, read $28)
+  [9] Call mutate mutate$6(mutate d$15, read $28)
  Return
 ```

@@ -23,20 +23,18 @@ bb0:
  Goto bb1
 bb1:
  predecessor blocks: bb0 bb3 bb5
-  items$6: phi(bb0: items$5, bb3: items$6, bb5: items$6)
-  cond$8: phi(bb0: cond$4, bb3: cond$8, bb5: cond$8)
-  If (read items$6) then:bb3 else:bb2
+  If (read items$5) then:bb3 else:bb2
 bb3:
  predecessor blocks: bb1
  [2] Let mutate y$7 = 0
-  If (read cond$8) then:bb5 else:bb1
+  If (read cond$4) then:bb5 else:bb1
 bb5:
  predecessor blocks: bb3
  [3] Reassign mutate y$9 = 1
  Goto bb1
 bb2:
  predecessor blocks: bb1
-  Return freeze items$6
+  Return freeze items$5
 ```

 ## Code
@@ -0,0 +1,53 @@
+
+## Input
+
+```javascript
+function foo(a, b, c) {
+  let x = 0;
+  while (a) {
+    while (b) {
+      while (c) {
+        x + 1;
+      }
+    }
+  }
+  return x;
+}
+
+```
+
+## HIR
+
+```
+bb0:
+  [1] Let mutate x$9 = 0
+  Goto bb1
+bb1:
+  predecessor blocks: bb0 bb4
+  If (read a$6) then:bb4 else:bb2
+bb4:
+  predecessor blocks: bb1 bb7
+  If (read b$7) then:bb7 else:bb1
+bb7:
+  predecessor blocks: bb4 bb9
+  If (read c$8) then:bb9 else:bb4
+bb9:
+  predecessor blocks: bb7
+  [2] Const mutate $17 = 1
+  [3] Binary read x$9 + read $17
+  Goto bb7
+bb2:
+  predecessor blocks: bb1
+  Return read x$9
+```
+
+## Code
+
+```javascript
+function foo$0(a$6, b$7, c$8) {
+  let x$9 = 0;
+  ("<<TODO: handle complex control flow in codegen>>");
+}
+
+```
+      
@@ -0,0 +1,11 @@
+function foo(a, b, c) {
+  let x = 0;
+  while (a) {
+    while (b) {
+      while (c) {
+        x + 1;
+      }
+    }
+  }
+  return x;
+}
@@ -0,0 +1,46 @@
+
+## Input
+
+```javascript
+function foo() {
+  let x = 1;
+  while (x < 10) {
+    x + 1;
+  }
+
+  return x;
+}
+
+```
+
+## HIR
+
+```
+bb0:
+  [1] Let mutate x$5 = 1
+  Goto bb1
+bb1:
+  predecessor blocks: bb0 bb3
+  [2] Const mutate $6 = 10
+  [3] Const mutate $8 = Binary read x$5 < read $6
+  If (read $8) then:bb3 else:bb2
+bb3:
+  predecessor blocks: bb1
+  [4] Const mutate $9 = 1
+  [5] Binary read x$5 + read $9
+  Goto bb1
+bb2:
+  predecessor blocks: bb1
+  Return read x$5
+```
+
+## Code
+
+```javascript
+function foo$0() {
+  let x$5 = 1;
+  ("<<TODO: handle complex control flow in codegen>>");
+}
+
+```
+      
@@ -0,0 +1,8 @@
+function foo() {
+  let x = 1;
+  while (x < 10) {
+    x + 1;
+  }
+
+  return x;
+}
@@ -16,6 +16,7 @@ import path from "path";
 import prettier from "prettier";
 import { lower } from "../HIR/BuildHIR";
 import codegen from "../HIR/Codegen";
+import { eliminateRedundantPhi } from "../HIR/EliminateRedundantPhi";
 import { HIRFunction } from "../HIR/HIR";
 import { Environment } from "../HIR/HIRBuilder";
 import { inferMutableRanges } from "../HIR/InferMutableLifetimes";
@@ -64,6 +65,7 @@ describe("React Forget (HIR version)", () => {
            const env: Environment = new Environment();
            const ir: HIRFunction = lower(nodePath, env);
            buildSSA(ir, env);
+            eliminateRedundantPhi(ir);
            inferReferenceEffects(ir);
            inferMutableRanges(ir);
            // const lifetimeGraph = buildDefUseGraph(ir);
@@ -83,12 +83,12 @@
    "@jridgewell/gen-mapping" "^0.3.2"
    jsesc "^2.5.1"

-"@babel/generator@^7.19.6":
-  version "7.19.6"
-  resolved "https://registry.yarnpkg.com/@babel/generator/-/generator-7.19.6.tgz#9e481a3fe9ca6261c972645ae3904ec0f9b34a1d"
-  integrity sha512-oHGRUQeoX1QrKeJIKVe0hwjGqNnVYsM5Nep5zo0uE0m42sLH+Fsd2pStJ5sRM1bNyTUUoz0pe2lTeMJrb/taTA==
+"@babel/generator@^7.20.1":
+  version "7.20.2"
+  resolved "https://registry.yarnpkg.com/@babel/generator/-/generator-7.20.2.tgz#c2e89e22613a039285c1e7b749e2cd0b30b9a481"
+  integrity sha512-SD75PMIK6i9H8G/tfGvB4KKl4Nw6Ssos9nGgYwxbgyTP0iX/Z55DveoH86rmUB/YHTQQ+ZC0F7xxaY8l2OF44Q==
  dependencies:
-    "@babel/types" "^7.19.4"
+    "@babel/types" "^7.20.2"
    "@jridgewell/gen-mapping" "^0.3.2"
    jsesc "^2.5.1"

@@ -260,10 +260,10 @@
  resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.19.3.tgz#8dd36d17c53ff347f9e55c328710321b49479a9a"
  integrity sha512-pJ9xOlNWHiy9+FuFP09DEAFbAn4JskgRsVcc169w2xRBC3FRGuQEwjeIMMND9L2zc0iEhO/tGv4Zq+km+hxNpQ==

-"@babel/parser@^7.19.6":
-  version "7.19.6"
-  resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.19.6.tgz#b923430cb94f58a7eae8facbffa9efd19130e7f8"
-  integrity sha512-h1IUp81s2JYJ3mRkdxJgs4UvmSsRvDrx5ICSJbPvtWYv5i1nTBGcBpnog+89rAFMwvvru6E5NUHdBe01UeSzYA==
+"@babel/parser@^7.20.1":
+  version "7.20.2"
+  resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.20.2.tgz#9aeb9b92f64412b5f81064d46f6a1ac0881337f4"
+  integrity sha512-afk318kh2uKbo7BEj2QtEi8HVCGrwHUffrYDy7dgVcSa2j9lY3LDjPzcyGdpX7xgm35aWqvciZJ4WKmdF/SxYg==

 "@babel/plugin-syntax-async-generators@^7.8.4":
  version "7.8.4"
@@ -484,18 +484,18 @@
    lodash "^4.17.10"

 "@babel/traverse@^7.19.1":
-  version "7.19.6"
-  resolved "https://registry.yarnpkg.com/@babel/traverse/-/traverse-7.19.6.tgz#7b4c865611df6d99cb131eec2e8ac71656a490dc"
-  integrity sha512-6l5HrUCzFM04mfbG09AagtYyR2P0B71B1wN7PfSPiksDPz2k5H9CBC1tcZpz2M8OxbKTPccByoOJ22rUKbpmQQ==
+  version "7.20.1"
+  resolved "https://registry.yarnpkg.com/@babel/traverse/-/traverse-7.20.1.tgz#9b15ccbf882f6d107eeeecf263fbcdd208777ec8"
+  integrity sha512-d3tN8fkVJwFLkHkBN479SOsw4DMZnz8cdbL/gvuDuzy3TS6Nfw80HuQqhw1pITbIruHyh7d1fMA47kWzmcUEGA==
  dependencies:
    "@babel/code-frame" "^7.18.6"
-    "@babel/generator" "^7.19.6"
+    "@babel/generator" "^7.20.1"
    "@babel/helper-environment-visitor" "^7.18.9"
    "@babel/helper-function-name" "^7.19.0"
    "@babel/helper-hoist-variables" "^7.18.6"
    "@babel/helper-split-export-declaration" "^7.18.6"
-    "@babel/parser" "^7.19.6"
-    "@babel/types" "^7.19.4"
+    "@babel/parser" "^7.20.1"
+    "@babel/types" "^7.20.0"
    debug "^4.1.0"
    globals "^11.1.0"

@@ -534,6 +534,15 @@
    "@babel/helper-validator-identifier" "^7.19.1"
    to-fast-properties "^2.0.0"

+"@babel/types@^7.20.0", "@babel/types@^7.20.2":
+  version "7.20.2"
+  resolved "https://registry.yarnpkg.com/@babel/types/-/types-7.20.2.tgz#67ac09266606190f496322dbaff360fdaa5e7842"
+  integrity sha512-FnnvsNWgZCr232sqtXggapvlkk/tuwR/qhGzcmxI0GXLCjmPYQPzio2FbdlWuY6y1sHFfQKk+rRbUZ9VStQMog==
+  dependencies:
+    "@babel/helper-string-parser" "^7.19.4"
+    "@babel/helper-validator-identifier" "^7.19.1"
+    to-fast-properties "^2.0.0"
+
 "@bcoe/v8-coverage@^0.2.3":
  version "0.2.3"
  resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39"