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

AOT: implement non-speculative int64 versions of MOD and TRUNCDIV #33967

Closed
mraleph opened this issue Jul 24, 2018 · 6 comments
Closed

AOT: implement non-speculative int64 versions of MOD and TRUNCDIV #33967

mraleph opened this issue Jul 24, 2018 · 6 comments
Assignees
@aartbik
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P2 A bug or feature request we're likely to work on
Projects
Modern Dart VM Pipeline

Comments

@mraleph
Copy link
Member

mraleph commented Jul 24, 2018

Our compiler does not inline / specialize int.% or int.~/ even on platforms where there are short enough instruction sequences.

On ARM64 we should probably always emit sdiv for TRUNCDIV and sdiv+msub for MOD.

On ARM32 we probably emit whatever we emit for IntegerDivision or maybe a call to __aeabi_ldivmod - if that is better.
@mraleph mraleph added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P2 A bug or feature request we're likely to work on labels Jul 24, 2018
@mraleph mraleph mentioned this issue Jul 24, 2018
Closed
@aartbik
Copy link
Contributor

aartbik commented Jul 27, 2018

This is a blocking AOT optimization of any integer operation chain with % or ~/. We should start supporting this at the very least on 64-bit architectures.

@aartbik aartbik self-assigned this Jul 27, 2018
@aartbik
Copy link
Contributor

aartbik commented Jul 27, 2018 •
edited
Loading

While working on this I noticed the current VM is not completely accurate for

0x8000000000000000 % -1 
0x8000000000000000 ~/ -1

(logical outcomes are 0 and 0x8000000000000000) but our 64-bit arch crash with a floating-point exceptions).

@aartbik aartbik closed this as completed Jul 27, 2018
@aartbik aartbik added this to Done in Modern Dart VM Pipeline via automation Jul 27, 2018
@aartbik aartbik reopened this Jul 27, 2018
@aartbik
Copy link
Contributor

aartbik commented Jul 27, 2018

Still getting used to the buttons :-) I obviously hit the "close and comment" rather than just the "comment".

dart-bot pushed a commit that referenced this issue Jul 28, 2018
@aartbik
[vm/compiler] Bug fix in 64-bit MOD (%) operator.
4482d13 
Rationale:
While writing tests for the ongoing native 64-bit
MOD/TRUNCDIV support in AOT, I noticed a floating-point
crash in our VM due to evaluating the constant mod case:

  Expect.equals(0, mod(minInt64, -1));
  Expect.equals(minInt64, truncdiv(minInt64, -1));

This fixes the constant evaluation part.

#33967

Change-Id: I9a4e6b3cd4d0d0dee39c690d2b981b5812501be4
Reviewed-on: https://dart-review.googlesource.com/67281
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Aart Bik <ajcbik@google.com>
@aartbik aartbik mentioned this issue Jul 28, 2018
Closed
dart-bot pushed a commit that referenced this issue Jul 31, 2018
@aartbik @athomas
[vm/compiler] Bug fix in 64-bit MOD (%) operator.
559afec 
Rationale:
While writing tests for the ongoing native 64-bit
MOD/TRUNCDIV support in AOT, I noticed a floating-point
crash in our VM due to evaluating the constant mod case:

  Expect.equals(0, mod(minInt64, -1));
  Expect.equals(minInt64, truncdiv(minInt64, -1));

This fixes the constant evaluation part.

#33967

Change-Id: I9a4e6b3cd4d0d0dee39c690d2b981b5812501be4
Reviewed-on: https://dart-review.googlesource.com/67281
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Aart Bik <ajcbik@google.com>
dart-bot pushed a commit that referenced this issue Aug 1, 2018
@aartbik
[vm/compiler] Non-speculative X64 long division/remainder.
8bad5c7 
Rationale:
Having a non-speculative implementation avoids deopting
under JIT and enables AOT.

#33967

Change-Id: Ib38200502f5a8f63912ff5d7a808ea6e27f9001d
Reviewed-on: https://dart-review.googlesource.com/67502
Commit-Queue: Aart Bik <ajcbik@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
dart-bot pushed a commit that referenced this issue Aug 2, 2018
@aartbik
[vm/compiler] Non-speculative ARM64 long division/remainder.
206f955 
Rationale:
Having a non-speculative implementation avoids deopting
under JIT and enables AOT. Done for X64 and now also ARM64.

#33967

Change-Id: I83302a5950aa2dc1a7220367755af748cfaa4393
Reviewed-on: https://dart-review.googlesource.com/67920
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Aart Bik <ajcbik@google.com>
dart-bot pushed a commit that referenced this issue Aug 3, 2018
@aartbik
[vm/compiler] Various 64-bit operator improvements.
bfa890d 
Rationale:
Improves the slow path of 64-bit REM/TRUNCDIV on
X64 and ARM64. Also introduces 64-bit negate operator,
which was needed to expose negative contants
(viz. x / -3 was represented as x / - (3) first).
The negate operator is not recognized in AOT mode yet,
since shifts by out-of-range constants should learn
how to throw rather than deopt....


#33967
flutter/flutter#19677

Change-Id: I7d81c9b1c72d99e8c4018f68c0501c7b599e073f
Reviewed-on: https://dart-review.googlesource.com/68280
Commit-Queue: Aart Bik <ajcbik@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
dart-bot pushed a commit that referenced this issue Aug 10, 2018
@aartbik
[vm/compiler] "fast"-path for x64 division
fe6daed 
Rationale:
Since 64-bit division requires twice as many cycles
and has much higher latency compared to the 32-bit
division, even for a non-speculative 64-bit path,
a 32-bit "fast path" makes sense.

Speedup:
About 2x for cases that fit 32-bits. No noticable
slowdown for the 64-bit cases.

flutter/flutter#19677
#33967

Change-Id: I0d3b44564fee2cda03fc36f089a4424084732de0
Reviewed-on: https://dart-review.googlesource.com/69200
Commit-Queue: Aart Bik <ajcbik@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
@mkustermann
Copy link
Member

@aartbik If you've already handled this issue, consider closing it.

@aartbik
Copy link
Contributor

aartbik commented Sep 13, 2018

I kept this open for an ARM32 implementation (probably a lib call).

@aartbik
Copy link
Contributor

aartbik commented Feb 12, 2019

Closing this issue after all. The 64-bit cases are now fully optimized, as requested. Handling the arm32 case with a lib call (compared to a call into the runtime) does not seem worth the engineering effort.

@aartbik aartbik closed this as completed Feb 12, 2019
@aartbik aartbik mentioned this issue Aug 31, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aartbik aartbik

Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P2 A bug or feature request we're likely to work on
Projects
Modern Dart VM Pipeline
  
Done
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mraleph @mkustermann @aartbik