@samitouri / QOS-React-2 / commits / 84a356274a

Stop shrinking HIR to preserve fallthrough nodes

This is part of a stack to fix some edge cases in inlining of useMemo closures. In this first step, I'm disabling `shrink()` in order to retain more information about the data flow. For example, ``` label: if (cond) { break label; } return foo; ``` Would previously have shrunk away the if body, making the IfTerminal.consequent point directly to the fallthrough block (w the return). Now we retain a separate block.

Joe Savona committed May 17, 2023 at 15:55 UTC 84a356274a86fec51dd4c46441605b2723e66c16
11 files changed +80 -31
compiler/forget/src/HIR/HIRBuilder.ts
+6 -6
@@ -9,7 +9,6 @@ import { Binding, NodePath } from "@babel/traverse";
9 import * as t from "@babel/types";
10 import invariant from "invariant";
11 import { CompilerError } from "../CompilerError";
12 -import { logHIR } from "../Utils/logger";
12 import { assertExhaustive } from "../Utils/utils";
13 import { Environment } from "./Environment";
14 import { Global } from "./Globals";
@@ -283,10 +282,11 @@ export default class HIRBuilder {
282 blocks: this.#completed,
283 entry: this.#entry,
284 };
286 - logHIR("Build (pre-shrink)", ir);
287 - // First reduce indirections
288 - shrink(ir);
289 - logHIR("Build (shrunk)", ir);
285 + // logHIR("Build (pre-shrink)", ir);
286 + // // First reduce indirections
287 + // shrink(ir);
288 + // logHIR("Build (shrunk)", ir);
289 +
290 // then convert to reverse postorder
291 reversePostorderBlocks(ir);
292 removeUnreachableForUpdates(ir);
@@ -500,7 +500,7 @@ export default class HIRBuilder {
500 /**
501 * Helper to shrink a CFG eliminate jump-only blocks.
502 */
503 -export function shrink(func: HIR): void {
503 +function _shrink(func: HIR): void {
504 const gotos = new Map();
505 /**
506 * Given a target block for some terminator, resolves the ideal block that should be
compiler/forget/src/HIR/index.ts
-1
@@ -14,7 +14,6 @@ export {
14 markPredecessors,
15 removeUnreachableFallthroughs,
16 reversePostorderBlocks,
17 - shrink,
17 } from "./HIRBuilder";
18 export { Hook } from "./Hooks";
19 export { mergeConsecutiveBlocks } from "./MergeConsecutiveBlocks";
compiler/forget/src/Inference/InlineUseMemo.ts
-2
@@ -24,7 +24,6 @@ import {
24 makeInstructionId,
25 makeType,
26 reversePostorderBlocks,
27 - shrink,
27 } from "../HIR";
28 import { markInstructionIds, markPredecessors } from "../HIR/HIRBuilder";
29 import { assertExhaustive, retainWhere } from "../Utils/utils";
@@ -236,7 +235,6 @@ export function inlineUseMemo(fn: HIRFunction): void {
235
236 // If terminals have changed then blocks may have become newly unreachable.
237 // Re-run minification of the graph (incl reordering instruction ids)
239 - shrink(fn.body);
238 reversePostorderBlocks(fn.body);
239 markInstructionIds(fn.body);
240 markPredecessors(fn.body);
compiler/forget/src/Optimization/ConstantPropagation.ts
-2
@@ -20,7 +20,6 @@ import {
20 Primitive,
21 removeUnreachableFallthroughs,
22 reversePostorderBlocks,
23 - shrink,
23 validateConsistentIdentifiers,
24 validateTerminalSuccessors,
25 } from "../HIR";
@@ -52,7 +51,6 @@ export function constantPropagation(fn: HIRFunction): void {
51 if (haveTerminalsChanged) {
52 // If terminals have changed then blocks may have become newly unreachable.
53 // Re-run minification of the graph (incl reordering instruction ids)
55 - shrink(fn.body);
54 reversePostorderBlocks(fn.body);
55 removeUnreachableFallthroughs(fn.body);
56 removeUnreachableForUpdates(fn.body);
compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts
+14 -3
@@ -6,6 +6,7 @@
6 */
7
8 import invariant from "invariant";
9 +import { CompilerError } from "../CompilerError";
10 import {
11 BasicBlock,
12 BlockId,
@@ -539,9 +540,19 @@ class Driver {
540 scheduleIds.push(scheduleId);
541 }
542
542 - const block = this.traverseBlock(
543 - this.cx.ir.blocks.get(terminal.block)!
544 - );
543 + let block: ReactiveBlock;
544 + if (this.cx.isScheduled(terminal.block)) {
545 + const break_ = this.visitBreak(terminal.block, null);
546 + if (break_ === null) {
547 + CompilerError.invariant(
548 + "Expected a break target for a label whose body is already scheduled",
549 + terminal.loc
550 + );
551 + }
552 + block = [break_];
553 + } else {
554 + block = this.traverseBlock(this.cx.ir.blocks.get(terminal.block)!);
555 + }
556
557 this.cx.unscheduleAll(scheduleIds);
558 blockValue.push({
compiler/forget/src/__tests__/fixtures/compiler/complex-while.expect.md
+6 -4
@@ -19,10 +19,12 @@ function foo(a, b, c) {
19
20 ```javascript
21 function foo(a, b, c) {
22 - if (a) {
23 - while (b) {
24 - if (c) {
25 - break;
22 + bb1: {
23 + if (a) {
24 + while (b) {
25 + if (c) {
26 + break bb1;
27 + }
28 }
29 }
30 }
compiler/forget/src/__tests__/fixtures/compiler/dominator.expect.md
+10 -7
@@ -43,19 +43,22 @@ function Component(props) {
43 ```javascript
44 function Component(props) {
45 let x = 0;
46 - if (props.a) {
47 - x = 1;
48 - } else {
49 - if (props.b) {
50 - x = 3;
46 + bb1: {
47 + if (props.a) {
48 + x = 1;
49 } else {
50 + if (props.b) {
51 + x = 3;
52 + } else {
53 + break bb1;
54 + }
55 }
56 }
57 bb10: {
55 - switch (props.c) {
58 + bb12: switch (props.c) {
59 case "a": {
60 x = 4;
58 - break bb10;
61 + break bb12;
62 }
63 case "b": {
64 break bb10;
compiler/forget/src/__tests__/fixtures/compiler/reverse-postorder.expect.md
+4 -4
@@ -36,13 +36,13 @@ function Component(props) {
36
37 ```javascript
38 function Component(props) {
39 - bb1: if (props.cond) {
40 - switch (props.test) {
39 + if (props.cond) {
40 + bb3: switch (props.test) {
41 case 0: {
42 - break bb1;
42 + break bb3;
43 }
44 case 1: {
45 - break bb1;
45 + break bb3;
46 }
47 case 2: {
48 }
compiler/forget/src/__tests__/fixtures/compiler/rules-of-hooks/rules-of-hooks-c1e8c7f4c191.expect.md
+40
@@ -147,84 +147,124 @@ function MyComponent() {
147 // Is valid but hard to compute by brute-forcing
148 function MyComponent() {
149 if (c) {
150 + } else {
151 }
152 if (c) {
153 + } else {
154 }
155 if (c) {
156 + } else {
157 }
158 if (c) {
159 + } else {
160 }
161 if (c) {
162 + } else {
163 }
164 if (c) {
165 + } else {
166 }
167 if (c) {
168 + } else {
169 }
170 if (c) {
171 + } else {
172 }
173 if (c) {
174 + } else {
175 }
176 if (c) {
177 + } else {
178 }
179 if (c) {
180 + } else {
181 }
182 if (c) {
183 + } else {
184 }
185 if (c) {
186 + } else {
187 }
188 if (c) {
189 + } else {
190 }
191 if (c) {
192 + } else {
193 }
194 if (c) {
195 + } else {
196 }
197 if (c) {
198 + } else {
199 }
200 if (c) {
201 + } else {
202 }
203 if (c) {
204 + } else {
205 }
206 if (c) {
207 + } else {
208 }
209 if (c) {
210 + } else {
211 }
212 if (c) {
213 + } else {
214 }
215 if (c) {
216 + } else {
217 }
218 if (c) {
219 + } else {
220 }
221 if (c) {
222 + } else {
223 }
224 if (c) {
225 + } else {
226 }
227 if (c) {
228 + } else {
229 }
230 if (c) {
231 + } else {
232 }
233 if (c) {
234 + } else {
235 }
236 if (c) {
237 + } else {
238 }
239 if (c) {
240 + } else {
241 }
242 if (c) {
243 + } else {
244 }
245 if (c) {
246 + } else {
247 }
248 if (c) {
249 + } else {
250 }
251 if (c) {
252 + } else {
253 }
254 if (c) {
255 + } else {
256 }
257 if (c) {
258 + } else {
259 }
260 if (c) {
261 + } else {
262 }
263 if (c) {
264 + } else {
265 }
266 if (c) {
267 + } else {
268 }
269
270 useHook();
compiler/forget/src/__tests__/fixtures/compiler/switch-non-final-default.expect.md
-1
@@ -53,7 +53,6 @@ function Component(props) {
53 t0 = $[3];
54 }
55 y = t0;
56 - break bb1;
56 }
57 default: {
58 break bb1;
compiler/forget/src/__tests__/fixtures/compiler/switch-with-fallthrough.expect.md
-1
@@ -40,7 +40,6 @@ function foo(x) {
40 case 0: {
41 }
42 case 1: {
43 - break bb1;
43 }
44 case 2: {
45 break bb1;