Skip to content

Shifts deopt on negative constants #34072

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

Closed
aartbik opened this issue Aug 3, 2018 · 0 comments
Closed

Shifts deopt on negative constants #34072

aartbik opened this issue Aug 3, 2018 · 0 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@aartbik
Copy link
Contributor

aartbik commented Aug 3, 2018

The dart compiler current replaces negative shifts (when they will throw) with a deopt, which seems an acceptable detour for JIT but is devastating for AOT. Apparently this was not an issue until now since the dart compiler does not deal very well with negative constants (viz -3 really is -(3)).

A recent improvement in recognizing static calls to unary neg as a native operation exposes this shortcoming, since it improves constant folding (and range analysis) and changes some of the shifts in the tests to deopts, a big nono for AOT.

Before I can improve the negate, this needs to be dealt with.

@aartbik aartbik added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Aug 3, 2018
@aartbik aartbik self-assigned this Aug 3, 2018
dart-bot pushed a commit that referenced this issue Aug 9, 2018
Rationale:
This improves JIT and AOT performance of unary minus
and also improves constant folding and range analysis
on negative constant (viz. x / -3 is often x / - (3)).
The SHIFT operator needed some special treatment, since
we have to avoid converting a NON-speculative shifts
back into a deopt.

#34072

Change-Id: I230c9cfda98297f683bbba53688e57c2cc659360
Reviewed-on: https://dart-review.googlesource.com/68434
Reviewed-by: Alexander Markov <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
Commit-Queue: Aart Bik <[email protected]>
@aartbik aartbik closed this as completed Aug 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

1 participant