Implement bigint absAdd, absSub, mulAdd, sqrAdd, estQuotientDigit intrinsics,

Implement bigint absAdd, absSub, mulAdd, sqrAdd, estQuotientDigit intrinsics,
and Montgomery mulMod intrinsics on x64.
Add support for mul, div, shld, adc, sbb instructions on x64.
Add assembler tests.

R=zra@google.com

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

[regis, 2014-10-10 15:29:24]

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

[zra, 2014-10-10 16:23:43]

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64.h
File runtime/vm/assembler_x64.h (right):

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64.h#n...
runtime/vm/assembler_x64.h:549: void addl(Register dst, const Address& address);
Please add assembler tests for addl, adcl, subl, and sbbl.

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
File runtime/vm/assembler_x64_test.cc (right):

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
runtime/vm/assembler_x64_test.cc:392: __ mull(RCX);
A comment about what is going on here might be good since 15 is sort of
non-intuitive as the expected result.

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
runtime/vm/assembler_x64_test.cc:528: __ movl(RDX,
Immediate(0xfffffffff0000000));
movq? Can you really give such a big immediate to movl? Does it set only the low
32-bits of RDX? If so why not get rid of the high bits of the immediate?

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
runtime/vm/assembler_x64_test.cc:530: __ divl(RCX);
A comment here too.

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
runtime/vm/assembler_x64_test.cc:939: __ movl(RDX,
Immediate(0xffffffffff000000));
Same question in the sequence with l vs. q versions of instructions with large
immediates.

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
runtime/vm/assembler_x64_test.cc:943: __ shldl(RDX, RAX, Immediate(2));
A comment here.

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

https://codereview.chromium.org/648613006/diff/1/runtime/vm/intrinsifier_x64....
runtime/vm/intrinsifier_x64.cc:849: __ decq(RCX);  // Does not affect carry
flag.
Is this supposed to be R8? If not, RCX appears unused after the subtract above,
so why is this needed?

https://codereview.chromium.org/648613006/diff/1/runtime/vm/intrinsifier_x64....
runtime/vm/intrinsifier_x64.cc:899: __ decq(RCX);  // Does not affect carry
flag.
Same question.

[regis, 2014-10-10 19:15:48]

Thanks!

Please, have another look so I can go on vacation :-)

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64.h
File runtime/vm/assembler_x64.h (right):

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64.h#n...
runtime/vm/assembler_x64.h:549: void addl(Register dst, const Address& address);
On 2014/10/10 16:23:43, zra wrote:
> Please add assembler tests for addl, adcl, subl, and sbbl.

Done.

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
File runtime/vm/assembler_x64_test.cc (right):

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
runtime/vm/assembler_x64_test.cc:392: __ mull(RCX);
On 2014/10/10 16:23:43, zra wrote:
> A comment about what is going on here might be good since 15 is sort of
> non-intuitive as the expected result.

Done.

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
runtime/vm/assembler_x64_test.cc:528: __ movl(RDX,
Immediate(0xfffffffff0000000));
On 2014/10/10 16:23:43, zra wrote:
> movq? Can you really give such a big immediate to movl? Does it set only the
low
> 32-bits of RDX? If so why not get rid of the high bits of the immediate?

If you do not specify the high bits, the value is positive instead of negative
and you get an assert fault !imm.is_int32()

I declared some locals to make it more readable.

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
runtime/vm/assembler_x64_test.cc:530: __ divl(RCX);
On 2014/10/10 16:23:43, zra wrote:
> A comment here too.

ditto

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
runtime/vm/assembler_x64_test.cc:939: __ movl(RDX,
Immediate(0xffffffffff000000));
On 2014/10/10 16:23:43, zra wrote:
> Same question in the sequence with l vs. q versions of instructions with large
> immediates.

ditto

https://codereview.chromium.org/648613006/diff/1/runtime/vm/assembler_x64_tes...
runtime/vm/assembler_x64_test.cc:943: __ shldl(RDX, RAX, Immediate(2));
On 2014/10/10 16:23:43, zra wrote:
> A comment here.

Done.

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

https://codereview.chromium.org/648613006/diff/1/runtime/vm/intrinsifier_x64....
runtime/vm/intrinsifier_x64.cc:849: __ decq(RCX);  // Does not affect carry
flag.
On 2014/10/10 16:23:43, zra wrote:
> Is this supposed to be R8? If not, RCX appears unused after the subtract
above,
> so why is this needed?

RCX is the counter for the first loop, R8 for the second loop.
Comment added.

https://codereview.chromium.org/648613006/diff/1/runtime/vm/intrinsifier_x64....
runtime/vm/intrinsifier_x64.cc:899: __ decq(RCX);  // Does not affect carry
flag.
On 2014/10/10 16:23:43, zra wrote:
> Same question.

ditto

[zra, 2014-10-10 19:34:56]

lgtm

Enjoy your vacation =)

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

https://codereview.chromium.org/648613006/diff/1/runtime/vm/intrinsifier_x64....
runtime/vm/intrinsifier_x64.cc:849: __ decq(RCX);  // Does not affect carry
flag.
On 2014/10/10 19:15:47, regis wrote:
> On 2014/10/10 16:23:43, zra wrote:
> > Is this supposed to be R8? If not, RCX appears unused after the subtract
> above,
> > so why is this needed?
> 
> RCX is the counter for the first loop, R8 for the second loop.
> Comment added. 

Oh, sorry. I was confused by our conversation the other day, and for some reason
thought that the decq wasn't setting any condition flags. I get it now. Thanks.

[regis, 2014-10-10 19:50:09]

Committed patchset #2 (id:200001) manually as r41050 (presubmit successful).