Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[dart2js] SsaValueRangeAnalyzer bug #53078

Closed
rakudrama opened this issue Jul 31, 2023 · 1 comment
Closed

[dart2js] SsaValueRangeAnalyzer bug #53078

rakudrama opened this issue Jul 31, 2023 · 1 comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. dart2js-optimization dart2js-ssa P0 A serious issue requiring immediate resolution type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dart2js

Comments

@rakudrama
Copy link
Member

From flutter/flutter#131543

void main() {
  const int y = 3;
  int x = y;
  for (int i = 0; i < 3; i++) {
    final v = x >= y;
    print('$x >= $y is ${v}');
    if (i != 0 && v) throw 'Something is wrong';
    if (v) {
      x = 0;
    }
    x++;
  }
}

Commenting out SsaValueRangeAnalyzer(closedWorld, this), in ssa/optimize.dart causes the program to work.
@rakudrama rakudrama added web-dart2js P0 A serious issue requiring immediate resolution dart2js-optimization dart2js-ssa type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jul 31, 2023
@lrhn lrhn added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Aug 1, 2023
@rakudrama
Copy link
Member Author

Duplicate issue #48465

@androidseb androidseb mentioned this issue Aug 5, 2023
Closed
2 tasks
copybara-service bot pushed a commit that referenced this issue Aug 29, 2023
@rakudrama
[dart2js] Fix SSA value range analysis bugs
404edd9 
Treat the loop update marker more like an ordinary symbolic value. The old symbolic marker attempted to do widening 'on the fly', which could lead to incorrect results when the update could reset the value to a constant lower than the starting value. The new version moves the widening to a single place, at the loop update.

Fix #53355 by caching intermediate results so that long chains of diamond control flow are not explored exponentially.

There are very few changes in apps. There is one change in a Flutter app that is like the changed codegen/value_range_test where the bounds check can be eliminated because the loop index may be decremented, but not more than the increment, so is still weakly monotonic. There is one change in a large ACX app where a lower bounds check is no longer removed but I *think* it was previously removed incorrectly, though it is hard to tell since it is in a huge function.

Issue: #48465
Issue: #53078
Issue: #53355
Change-Id: Ib125cd6bb30cef52f8dfcd53eaa13e439f26316c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/322594
Reviewed-by: Sigmund Cherem <[email protected]>
Commit-Queue: Stephen Adams <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. dart2js-optimization dart2js-ssa P0 A serious issue requiring immediate resolution type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dart2js
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rakudrama @lrhn