Skip to content

dart2js: optimization: path sensitive dominators for type propagation and GVN #28330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rakudrama opened this issue Jan 11, 2017 · 0 comments
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. dart2js-optimization web-dart2js

Comments

@rakudrama
Copy link
Member

dart2js should generate better code branches dependent on && and || conditions. There is a loss of useful information.

void update(String s) {
  if (s != null && s.trim().isNotEmpty) {
    print(s.trim().toUpperCase());
  }
}

main() {
  update(null);
  update(' five');
  update('  ');
}

The generated code for update has an interceptor call to s.trim():

    update: function(s) {
      var t1;
      if (s != null && C.JSString_methods.trim$0(s).length !== 0) {
        t1 = J.trim$0$s(s);
        H.printString(t1.toUpperCase());
      }
    },
  ...
  J.trim$0$s = function(receiver) {
    return J.getInterceptor$s(receiver).trim$0(receiver);
  };

Notice that the first call to trim is via a constant interceptor (J.JSString_methods). s is inferred to be String or Null (this would also be true from strong mode or --trust-type-annotations). The s != null check allows the type to be refined to non-null-String. The second call is compiled to J.trim$0$s(s) because this information is lost. The ssa block structure makes every expression, including the && expression, a single-entry-single-exit region, so the condition of the if-statement is a phi-node. The insertion of HTypeKnown removing Null from the possible types of s is only inserted in the condition rest of the condition:

  if (s != null) {
    A: cond = s.trim().isNotEmpty;   // s known not null here
  } else {
    cond = false;
  }
  if (cond) {  // the phi-node
    B: print(s.trim().toUpperCase());  // but not here
  }

One way to fix this is to generate the CFG for if (a && b) S; as if (a) if (b) S;.

Another way to fix this is to have an extended idea of domination, path sensitive dominators. The statement at B is only reachable if A was reached. Any effect-insensitive facts known at A are also true at B. When we insert HTypeKnown nodes at A, we can also insert them at B.
This is better than the first fix (if-if) since it works with code that did not originate as && syntactically in an if-expression, for example, an inlined function.

The second optimization we could do is a variant of GVN that introduces single-def variables to carry values from one dominated region to another. String.trim() is pure so we could transform the code to

  if (s != null) {
    A:
     s_trim = s.trim();
     cond = s_trim.isNotEmpty;
  } else {
    cond = false;
  }
  if (cont) {
    B: print(s_trim.toUpperCase());
  }

Discovering the path-sensitive dominators requires limited form of path-sensitive analysis. This would be more efficient after load-elimination and an initial pass of GVN to reduce the number of potentially tracked values. We would be path-sensitive only to values that flow via x != null, phi- and not-nodes to if-conditions, and are used in the controlled regions of the if. Perhaps a domain of 2^{True, False, Null, Other} would be adequate.

@vsmenon vsmenon added the area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. label Jul 20, 2019
copybara-service bot pushed a commit that referenced this issue Mar 24, 2025
Bug: #26835 #28330
Change-Id: Ic3bc05f1b5525e58173c335c5f3f4f91dd09e2f3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/417329
Reviewed-by: Mayank Patke <[email protected]>
Commit-Queue: Stephen Adams <[email protected]>
copybara-service bot pushed a commit that referenced this issue Mar 26, 2025
Bug: #26835 #28330
Change-Id: I2ea5d3022901a67b10fdb7ca0a6233a22ca233bf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/417328
Commit-Queue: Stephen Adams <[email protected]>
Reviewed-by: Mayank Patke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. dart2js-optimization web-dart2js
Projects
None yet
Development

No branches or pull requests

2 participants