@samitouri / QOS-React-2 / commits / 41d99ffcb9

PropagateScopeDeps uses visitor infra

PropagateScopeDependencies is one of the few places we don't use the new visitor infra for traversing ReactiveFunction. Or rather it _was_! Note that there's a bit less value here than in other places since we have to handle each terminal variant with custom logic, but at least it's more consistent with the rest of the codebase now.

Joe Savona committed Apr 28, 2023 at 17:14 UTC 41d99ffcb94dd8976d782be1b79ce9d4d7f04cd5
1 file changed +202 -211
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+202 -211
@@ -12,12 +12,12 @@ import {
12 InstructionKind,
13 makeInstructionId,
14 Place,
15 - ReactiveBlock,
15 ReactiveFunction,
16 ReactiveInstruction,
17 ReactiveScope,
18 ReactiveScopeBlock,
19 ReactiveScopeDependency,
20 + ReactiveTerminalStatement,
21 ReactiveValue,
22 } from "../HIR/HIR";
23 import {
@@ -58,7 +58,7 @@ export function propagateScopeDependencies(fn: ReactiveFunction): void {
58 scope: empty(),
59 });
60 }
61 - visit(context, fn.body);
61 + visitReactiveFunction(fn, new PropagationVisitor(), context);
62 }
63
64 type TemporariesUsedOutsideDefiningScope = {
@@ -421,237 +421,228 @@ class Context {
421 }
422 }
423
424 -function visit(context: Context, block: ReactiveBlock): void {
425 - for (const item of block) {
426 - switch (item.kind) {
427 - case "scope": {
428 - const scopeDependencies = context.enter(item.scope, () => {
429 - visit(context, item.instructions);
424 +class PropagationVisitor extends ReactiveFunctionVisitor<Context> {
425 + override visitScope(scope: ReactiveScopeBlock, context: Context): void {
426 + const scopeDependencies = context.enter(scope.scope, () => {
427 + this.visitBlock(scope.instructions, context);
428 + });
429 + scope.scope.dependencies = scopeDependencies;
430 + }
431 +
432 + override visitInstruction(
433 + instruction: ReactiveInstruction,
434 + context: Context
435 + ): void {
436 + const { id, value, lvalue } = instruction;
437 + this.visitInstructionValue(context, id, value, lvalue);
438 + if (lvalue == null) {
439 + return;
440 + }
441 + context.declare(lvalue.identifier, {
442 + id,
443 + scope: context.currentScope,
444 + });
445 + }
446 +
447 + visitReactiveValue(
448 + context: Context,
449 + id: InstructionId,
450 + value: ReactiveValue
451 + ): void {
452 + switch (value.kind) {
453 + case "OptionalCall": {
454 + context.enterConditional(() => {
455 + this.visitReactiveValue(context, id, value.call);
456 });
431 - item.scope.dependencies = scopeDependencies;
457 break;
458 }
434 - case "instruction": {
435 - visitInstruction(context, item.instruction);
459 + case "LogicalExpression": {
460 + this.visitReactiveValue(context, id, value.left);
461 + context.enterConditional(() => {
462 + this.visitReactiveValue(context, id, value.right);
463 + });
464 break;
465 }
438 - case "terminal": {
439 - const terminal = item.terminal;
440 - switch (terminal.kind) {
441 - case "break":
442 - case "continue": {
443 - break;
444 - }
445 - case "return": {
446 - context.visitOperand(terminal.value);
447 - break;
448 - }
449 - case "throw": {
450 - context.visitOperand(terminal.value);
451 - break;
452 - }
453 - case "for": {
454 - visitReactiveValue(context, terminal.id, terminal.init);
455 - visitReactiveValue(context, terminal.id, terminal.test);
456 - context.enterConditional(() => {
457 - if (terminal.update !== null) {
458 - visitReactiveValue(context, terminal.id, terminal.update);
459 - }
460 - visit(context, terminal.loop);
461 - });
462 - break;
463 - }
464 - case "for-of": {
465 - visitReactiveValue(context, terminal.id, terminal.init);
466 - context.enterConditional(() => {
467 - visit(context, terminal.loop);
468 - });
469 - break;
470 - }
471 - case "do-while": {
472 - visit(context, terminal.loop);
473 - context.enterConditional(() => {
474 - visitReactiveValue(context, terminal.id, terminal.test);
475 - });
476 - break;
477 - }
478 - case "while": {
479 - visitReactiveValue(context, terminal.id, terminal.test);
480 - context.enterConditional(() => {
481 - visit(context, terminal.loop);
482 - });
483 - break;
484 - }
485 - case "if": {
486 - context.visitOperand(terminal.test);
487 - const { consequent, alternate } = terminal;
488 - const depsInIf = context.enterConditional(() => {
489 - visit(context, consequent);
490 - });
491 - if (alternate !== null) {
492 - const depsInElse = context.enterConditional(() => {
493 - visit(context, alternate);
494 - });
495 - context.promoteDepsFromExhaustiveConditionals([
496 - depsInIf,
497 - depsInElse,
498 - ]);
499 - }
500 - break;
501 - }
502 - case "switch": {
503 - context.visitOperand(terminal.test);
504 - const depsInCases = [];
505 - let foundDefault = false;
506 - // This can underestimate unconditional accesses due to the current
507 - // CFG representation for fallthrough. This is safe. It only
508 - // reduces granularity of dependencies.
509 - for (const { test, block } of terminal.cases) {
510 - if (test == null) {
511 - foundDefault = true;
512 - }
513 - if (block !== undefined) {
514 - depsInCases.push(
515 - context.enterConditional(() => {
516 - visit(context, block);
517 - })
518 - );
519 - }
520 - }
521 - if (foundDefault) {
522 - context.promoteDepsFromExhaustiveConditionals(depsInCases);
523 - }
524 - break;
525 - }
526 - case "label": {
527 - visit(context, terminal.block);
528 - break;
529 - }
530 - default: {
531 - assertExhaustive(
532 - terminal,
533 - `Unexpected terminal kind '${(terminal as any).kind}'`
534 - );
535 - }
466 + case "ConditionalExpression": {
467 + this.visitReactiveValue(context, id, value.test);
468 +
469 + const consequentDeps = context.enterConditional(() => {
470 + this.visitReactiveValue(context, id, value.consequent);
471 + });
472 + const alternateDeps = context.enterConditional(() => {
473 + this.visitReactiveValue(context, id, value.alternate);
474 + });
475 + context.promoteDepsFromExhaustiveConditionals([
476 + consequentDeps,
477 + alternateDeps,
478 + ]);
479 + break;
480 + }
481 + case "SequenceExpression": {
482 + for (const instr of value.instructions) {
483 + this.visitInstruction(instr, context);
484 }
485 + this.visitInstructionValue(context, id, value.value, null);
486 break;
487 }
488 default: {
540 - assertExhaustive(item, `Unexpected item`);
489 + for (const operand of eachInstructionValueOperand(value)) {
490 + context.visitOperand(operand);
491 + }
492 }
493 }
494 }
544 -}
495
546 -function visitReactiveValue(
547 - context: Context,
548 - id: InstructionId,
549 - value: ReactiveValue
550 -): void {
551 - switch (value.kind) {
552 - case "OptionalCall": {
553 - context.enterConditional(() => {
554 - visitReactiveValue(context, id, value.call);
555 - });
556 - break;
557 - }
558 - case "LogicalExpression": {
559 - visitReactiveValue(context, id, value.left);
560 - context.enterConditional(() => {
561 - visitReactiveValue(context, id, value.right);
562 - });
563 - break;
564 - }
565 - case "ConditionalExpression": {
566 - visitReactiveValue(context, id, value.test);
567 -
568 - const consequentDeps = context.enterConditional(() => {
569 - visitReactiveValue(context, id, value.consequent);
570 - });
571 - const alternateDeps = context.enterConditional(() => {
572 - visitReactiveValue(context, id, value.alternate);
573 - });
574 - context.promoteDepsFromExhaustiveConditionals([
575 - consequentDeps,
576 - alternateDeps,
577 - ]);
578 - break;
579 - }
580 - case "SequenceExpression": {
581 - for (const instr of value.instructions) {
582 - visitInstruction(context, instr);
496 + visitInstructionValue(
497 + context: Context,
498 + id: InstructionId,
499 + value: ReactiveValue,
500 + lvalue: Place | null
501 + ): void {
502 + if (value.kind === "LoadLocal" && lvalue !== null) {
503 + if (
504 + value.place.identifier.name !== null &&
505 + lvalue.identifier.name === null &&
506 + !context.isUsedOutsideDeclaringScope(lvalue)
507 + ) {
508 + context.declareTemporary(lvalue, value.place);
509 + } else {
510 + context.visitOperand(value.place);
511 }
584 - visitInstructionValue(context, id, value.value, null);
585 - break;
586 - }
587 - default: {
588 - for (const operand of eachInstructionValueOperand(value)) {
589 - context.visitOperand(operand);
512 + } else if (value.kind === "PropertyLoad") {
513 + if (lvalue !== null && !context.isUsedOutsideDeclaringScope(lvalue)) {
514 + context.declareProperty(
515 + lvalue,
516 + value.object,
517 + value.property,
518 + value.optional
519 + );
520 + } else {
521 + context.visitProperty(value.object, value.property, value.optional);
522 }
591 - }
592 - }
593 -}
594 -
595 -function visitInstructionValue(
596 - context: Context,
597 - id: InstructionId,
598 - value: ReactiveValue,
599 - lvalue: Place | null
600 -): void {
601 - if (value.kind === "LoadLocal" && lvalue !== null) {
602 - if (
603 - value.place.identifier.name !== null &&
604 - lvalue.identifier.name === null &&
605 - !context.isUsedOutsideDeclaringScope(lvalue)
606 - ) {
607 - context.declareTemporary(lvalue, value.place);
608 - } else {
609 - context.visitOperand(value.place);
610 - }
611 - } else if (value.kind === "PropertyLoad") {
612 - if (lvalue !== null && !context.isUsedOutsideDeclaringScope(lvalue)) {
613 - context.declareProperty(
614 - lvalue,
615 - value.object,
616 - value.property,
617 - value.optional
618 - );
619 - } else {
620 - context.visitProperty(value.object, value.property, value.optional);
621 - }
622 - } else if (value.kind === "StoreLocal") {
623 - context.visitOperand(value.value);
624 - if (value.lvalue.kind === InstructionKind.Reassign) {
625 - context.visitReassignment(value.lvalue.place);
626 - }
627 - context.declare(value.lvalue.place.identifier, {
628 - id,
629 - scope: context.currentScope,
630 - });
631 - } else if (value.kind === "Destructure") {
632 - context.visitOperand(value.value);
633 - for (const place of eachPatternOperand(value.lvalue.pattern)) {
523 + } else if (value.kind === "StoreLocal") {
524 + context.visitOperand(value.value);
525 if (value.lvalue.kind === InstructionKind.Reassign) {
635 - context.visitReassignment(place);
526 + context.visitReassignment(value.lvalue.place);
527 }
637 - context.declare(place.identifier, {
528 + context.declare(value.lvalue.place.identifier, {
529 id,
530 scope: context.currentScope,
531 });
532 + } else if (value.kind === "Destructure") {
533 + context.visitOperand(value.value);
534 + for (const place of eachPatternOperand(value.lvalue.pattern)) {
535 + if (value.lvalue.kind === InstructionKind.Reassign) {
536 + context.visitReassignment(place);
537 + }
538 + context.declare(place.identifier, {
539 + id,
540 + scope: context.currentScope,
541 + });
542 + }
543 + } else {
544 + this.visitReactiveValue(context, id, value);
545 }
642 - } else {
643 - visitReactiveValue(context, id, value);
546 }
645 -}
547
647 -function visitInstruction(context: Context, instr: ReactiveInstruction): void {
648 - const { lvalue } = instr;
649 - visitInstructionValue(context, instr.id, instr.value, lvalue);
650 - if (lvalue == null) {
651 - return;
548 + override visitTerminal(
549 + stmt: ReactiveTerminalStatement,
550 + context: Context
551 + ): void {
552 + const terminal = stmt.terminal;
553 + switch (terminal.kind) {
554 + case "break":
555 + case "continue": {
556 + break;
557 + }
558 + case "return": {
559 + context.visitOperand(terminal.value);
560 + break;
561 + }
562 + case "throw": {
563 + context.visitOperand(terminal.value);
564 + break;
565 + }
566 + case "for": {
567 + this.visitReactiveValue(context, terminal.id, terminal.init);
568 + this.visitReactiveValue(context, terminal.id, terminal.test);
569 + context.enterConditional(() => {
570 + if (terminal.update !== null) {
571 + this.visitReactiveValue(context, terminal.id, terminal.update);
572 + }
573 + this.visitBlock(terminal.loop, context);
574 + });
575 + break;
576 + }
577 + case "for-of": {
578 + this.visitReactiveValue(context, terminal.id, terminal.init);
579 + context.enterConditional(() => {
580 + this.visitBlock(terminal.loop, context);
581 + });
582 + break;
583 + }
584 + case "do-while": {
585 + this.visitBlock(terminal.loop, context);
586 + context.enterConditional(() => {
587 + this.visitReactiveValue(context, terminal.id, terminal.test);
588 + });
589 + break;
590 + }
591 + case "while": {
592 + this.visitReactiveValue(context, terminal.id, terminal.test);
593 + context.enterConditional(() => {
594 + this.visitBlock(terminal.loop, context);
595 + });
596 + break;
597 + }
598 + case "if": {
599 + context.visitOperand(terminal.test);
600 + const { consequent, alternate } = terminal;
601 + const depsInIf = context.enterConditional(() => {
602 + this.visitBlock(consequent, context);
603 + });
604 + if (alternate !== null) {
605 + const depsInElse = context.enterConditional(() => {
606 + this.visitBlock(alternate, context);
607 + });
608 + context.promoteDepsFromExhaustiveConditionals([depsInIf, depsInElse]);
609 + }
610 + break;
611 + }
612 + case "switch": {
613 + context.visitOperand(terminal.test);
614 + const depsInCases = [];
615 + let foundDefault = false;
616 + // This can underestimate unconditional accesses due to the current
617 + // CFG representation for fallthrough. This is safe. It only
618 + // reduces granularity of dependencies.
619 + for (const { test, block } of terminal.cases) {
620 + if (test == null) {
621 + foundDefault = true;
622 + }
623 + if (block !== undefined) {
624 + depsInCases.push(
625 + context.enterConditional(() => {
626 + this.visitBlock(block, context);
627 + })
628 + );
629 + }
630 + }
631 + if (foundDefault) {
632 + context.promoteDepsFromExhaustiveConditionals(depsInCases);
633 + }
634 + break;
635 + }
636 + case "label": {
637 + this.visitBlock(terminal.block, context);
638 + break;
639 + }
640 + default: {
641 + assertExhaustive(
642 + terminal,
643 + `Unexpected terminal kind '${(terminal as any).kind}'`
644 + );
645 + }
646 + }
647 }
653 - context.declare(lvalue.identifier, {
654 - id: instr.id,
655 - scope: context.currentScope,
656 - });
648 }