@samitouri / QOS-React-2 / commits / 5b11372901

Optimize DCE for destructuring

In #1287 i implemented basic handling for destructuring in DCE: if any of the pattern values are used, we retained the whole instruction as-is. However, ideally we could prune out unused elements from the pattern. There are pretty simple rules: * ArrayPattern we can eliminate unused elements from the end. * ObjectPattern we can eliminate any unused element, but only if there is no rest element.

Joe Savona committed Mar 3, 2023 at 17:16 UTC 5b1137290192f9810a1dac551078bb6a3da93c4b
11 files changed +195 -10
compiler/forget/src/Optimization/DeadCodeElimination.ts
+82 -4
@@ -5,7 +5,15 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 -import { BlockId, HIRFunction, Identifier, InstructionValue } from "../HIR";
8 +import {
9 + ArrayPattern,
10 + BlockId,
11 + HIRFunction,
12 + Identifier,
13 + Instruction,
14 + InstructionValue,
15 + ObjectPattern,
16 +} from "../HIR";
17 import {
18 eachInstructionValueOperand,
19 eachPatternOperand,
@@ -48,9 +56,7 @@ export function deadCodeElimination(fn: HIRFunction): void {
56 continue;
57 }
58 used.add(instr.lvalue.identifier);
51 - for (const operand of eachInstructionValueOperand(instr.value)) {
52 - used.add(operand.identifier);
53 - }
59 + visitInstruction(instr, used);
60 }
61 for (const phi of block.phis) {
62 if (used.has(phi.id)) {
@@ -73,6 +79,78 @@ export function deadCodeElimination(fn: HIRFunction): void {
79 }
80 }
81
82 +function visitInstruction(instr: Instruction, used: Set<Identifier>): void {
83 + if (instr.value.kind === "Destructure") {
84 + // Mark the value as used, not the lvalues
85 + used.add(instr.value.value.identifier);
86 + // Remove unused lvalues
87 + switch (instr.value.lvalue.pattern.kind) {
88 + case "ArrayPattern": {
89 + // For arrays, we can only eliminate unused items from the end of the array,
90 + // so we iterate from the end and break once we find a used item. Note that
91 + // we already know at least one item is used, from the pruneableValue check.
92 + let nextItems: ArrayPattern["items"] | null = null;
93 + const originalItems = instr.value.lvalue.pattern.items;
94 + for (let i = originalItems.length - 1; i >= 0; i--) {
95 + const item = originalItems[i];
96 + if (item.kind === "Identifier") {
97 + if (used.has(item.identifier)) {
98 + nextItems = originalItems.slice(0, i + 1);
99 + break;
100 + }
101 + } else {
102 + if (used.has(item.place.identifier)) {
103 + nextItems = originalItems.slice(0, i + 1);
104 + break;
105 + }
106 + }
107 + }
108 + if (nextItems !== null) {
109 + instr.value.lvalue.pattern.items = nextItems;
110 + }
111 + break;
112 + }
113 + case "ObjectPattern": {
114 + // For objects we can prune any unused properties so long as there is no used rest element
115 + // (`const {x, ...y} = z`). If a rest element exists and is used, then nothing can be pruned
116 + // because it would change the set of properties which are copied into the rest value.
117 + // In the `const {x, ...y} = z` example, removing the `x` property would mean that `y` now
118 + // has an `x` property, changing the semantics.
119 + let nextProperties: ObjectPattern["properties"] | null = null;
120 + for (const property of instr.value.lvalue.pattern.properties) {
121 + if (property.kind === "ObjectProperty") {
122 + if (used.has(property.place.identifier)) {
123 + nextProperties ??= [];
124 + nextProperties.push(property);
125 + }
126 + } else {
127 + if (used.has(property.place.identifier)) {
128 + nextProperties = null;
129 + break;
130 + }
131 + }
132 + }
133 + if (nextProperties !== null) {
134 + instr.value.lvalue.pattern.properties = nextProperties;
135 + }
136 + break;
137 + }
138 + default: {
139 + assertExhaustive(
140 + instr.value.lvalue.pattern,
141 + `Unexpected pattern kind '${
142 + (instr.value.lvalue.pattern as any).kind
143 + }'`
144 + );
145 + }
146 + }
147 + } else {
148 + for (const operand of eachInstructionValueOperand(instr.value)) {
149 + used.add(operand.identifier);
150 + }
151 + }
152 +}
153 +
154 /**
155 * Returns true if it is safe to prune an instruction with the given value.
156 * Functions which may have side-
compiler/forget/src/__tests__/fixtures/hir/destructure-direct-reassignment.expect.md
+10 -6
@@ -5,6 +5,7 @@
5 function foo(props) {
6 let x, y;
7 ({ x, y } = { x: props.a, y: props.b });
8 + console.log(x); // prevent DCE from eliminating `x` altogether
9 x = props.c;
10 return x + y;
11 }
@@ -15,19 +16,22 @@ function foo(props) {
16
17 ```javascript
18 function foo(props) {
18 - const $ = React.unstable_useMemoCache(3);
19 + const $ = React.unstable_useMemoCache(4);
20 const c_0 = $[0] !== props.a;
21 const c_1 = $[1] !== props.b;
21 - let t0;
22 + let x;
23 + let y;
24 if (c_0 || c_1) {
23 - t0 = { x: props.a, y: props.b };
25 + ({ x, y } = { x: props.a, y: props.b });
26 + console.log(x);
27 $[0] = props.a;
28 $[1] = props.b;
26 - $[2] = t0;
29 + $[2] = x;
30 + $[3] = y;
31 } else {
28 - t0 = $[2];
32 + x = $[2];
33 + y = $[3];
34 }
30 - let { x, y } = t0;
35 x = props.c;
36 return x + y;
37 }
compiler/forget/src/__tests__/fixtures/hir/destructure-direct-reassignment.js
+1
@@ -1,6 +1,7 @@
1 function foo(props) {
2 let x, y;
3 ({ x, y } = { x: props.a, y: props.b });
4 + console.log(x); // prevent DCE from eliminating `x` altogether
5 x = props.c;
6 return x + y;
7 }
compiler/forget/src/__tests__/fixtures/hir/unused-array-middle-element.expect.md new
+21
@@ -0,0 +1,21 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function foo(props) {
6 + const [x, unused, y] = props.a;
7 + return x + y;
8 +}
9 +
10 +```
11 +
12 +## Code
13 +
14 +```javascript
15 +function foo(props) {
16 + const [x, unused, y] = props.a;
17 + return x + y;
18 +}
19 +
20 +```
21 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/unused-array-middle-element.js new
+4
@@ -0,0 +1,4 @@
1 +function foo(props) {
2 + const [x, unused, y] = props.a;
3 + return x + y;
4 +}
compiler/forget/src/__tests__/fixtures/hir/unused-array-rest-element.expect.md new
+21
@@ -0,0 +1,21 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function foo(props) {
6 + const [x, y, ...z] = props.a;
7 + return x + y;
8 +}
9 +
10 +```
11 +
12 +## Code
13 +
14 +```javascript
15 +function foo(props) {
16 + const [x, y] = props.a;
17 + return x + y;
18 +}
19 +
20 +```
21 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/unused-array-rest-element.js new
+4
@@ -0,0 +1,4 @@
1 +function foo(props) {
2 + const [x, y, ...z] = props.a;
3 + return x + y;
4 +}
compiler/forget/src/__tests__/fixtures/hir/unused-object-element-with-rest.expect.md new
+22
@@ -0,0 +1,22 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Foo(props) {
6 + // can't remove `unused` since it affects which properties are copied into `rest`
7 + const { unused, ...rest } = props.a;
8 + return rest;
9 +}
10 +
11 +```
12 +
13 +## Code
14 +
15 +```javascript
16 +function Foo(props) {
17 + const { unused, ...rest } = props.a;
18 + return rest;
19 +}
20 +
21 +```
22 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/unused-object-element-with-rest.js new
+5
@@ -0,0 +1,5 @@
1 +function Foo(props) {
2 + // can't remove `unused` since it affects which properties are copied into `rest`
3 + const { unused, ...rest } = props.a;
4 + return rest;
5 +}
compiler/forget/src/__tests__/fixtures/hir/unused-object-element.expect.md new
+21
@@ -0,0 +1,21 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Foo(props) {
6 + const { x, y, ...z } = props.a;
7 + return x;
8 +}
9 +
10 +```
11 +
12 +## Code
13 +
14 +```javascript
15 +function Foo(props) {
16 + const { x } = props.a;
17 + return x;
18 +}
19 +
20 +```
21 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/unused-object-element.js new
+4
@@ -0,0 +1,4 @@
1 +function Foo(props) {
2 + const { x, y, ...z } = props.a;
3 + return x;
4 +}