Implement bigint absAdd, bigint absSub, and Montgomery mulMod intrinsics on ARM.

Implement bigint absAdd, bigint absSub, and Montgomery mulMod intrinsics on ARM.

R=johnmccutchan@google.com, zra@google.com

Committed: https://code.google.com/p/dart/source/detail?r=41036

[regis, 2014-10-09 14:14:08]

regis@google.com changed reviewers:
+ johnmccutchan@google.com, zra@google.com

[Cutch, 2014-10-09 14:46:05]

lgtm

[zra, 2014-10-09 15:39:29]

lgtm.

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm.cc
File runtime/vm/intrinsifier_arm.cc (right):

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm....
runtime/vm/intrinsifier_arm.cc:882: if (TargetCPUFeatures::arm_version() ==
ARMv7) {
ldrd is okay on all versions of arm we support. Here and below.

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm....
runtime/vm/intrinsifier_arm.cc:918: if (TargetCPUFeatures::arm_version() !=
ARMv7) {
This looks safe to enable on ARMv5TE and ARMv6.

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm....
runtime/vm/intrinsifier_arm.cc:950: __ teq(R3, Operand(R7));  // Does not affect
carry flag.
It probably won't make a difference, but you can swap the teq and str
instructions here (and below) to put more distance between the adcs and str and
the teq and the branch.

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm....
runtime/vm/intrinsifier_arm.cc:974: if (TargetCPUFeatures::arm_version() !=
ARMv7) {
This looks safe to enable on ARMv5TE and ARMv6.

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm....
runtime/vm/intrinsifier_arm.cc:1006: __ teq(R3, Operand(R7));  // Does not
affect carry flag.
Same comment as loop in absAdd.

[regis, 2014-10-10 07:42:39]

Thanks!

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm.cc
File runtime/vm/intrinsifier_arm.cc (right):

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm....
runtime/vm/intrinsifier_arm.cc:882: if (TargetCPUFeatures::arm_version() ==
ARMv7) {
On 2014/10/09 15:39:29, zra wrote:
> ldrd is okay on all versions of arm we support. Here and below.

Test removed. Thanks!
I remember checking TEQ and LDRD for absAdd, and adding the test. Later, I added
it to the setters, because they were using LDRD, forgetting I added the test for
TEQ only.

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm....
runtime/vm/intrinsifier_arm.cc:918: if (TargetCPUFeatures::arm_version() !=
ARMv7) {
On 2014/10/09 15:39:28, zra wrote:
> This looks safe to enable on ARMv5TE and ARMv6.

You are right. Test removed.
TEQ is ARMv6 with Thumb encoding only.

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm....
runtime/vm/intrinsifier_arm.cc:950: __ teq(R3, Operand(R7));  // Does not affect
carry flag.
On 2014/10/09 15:39:28, zra wrote:
> It probably won't make a difference, but you can swap the teq and str
> instructions here (and below) to put more distance between the adcs and str
and
> the teq and the branch.

Done.

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm....
runtime/vm/intrinsifier_arm.cc:974: if (TargetCPUFeatures::arm_version() !=
ARMv7) {
On 2014/10/09 15:39:28, zra wrote:
> This looks safe to enable on ARMv5TE and ARMv6.

Done.

https://codereview.chromium.org/646493003/diff/1/runtime/vm/intrinsifier_arm....
runtime/vm/intrinsifier_arm.cc:1006: __ teq(R3, Operand(R7));  // Does not
affect carry flag.
On 2014/10/09 15:39:28, zra wrote:
> Same comment as loop in absAdd.

Done.

[regis, 2014-10-10 07:49:33]

Committed patchset #2 (id:20001) manually as r41036 (presubmit successful).