ARM: Add support for DoMathAbs with double inputs....

ARM: Add support for DoMathAbs with double inputs.

Adds vabs instruction to simulator, assembler, disassembler and tests.

BUG=none
TEST=Added to cctest.

Committed: http://code.google.com/p/v8/source/detail?r=6531

[Mads Ager (chromium), 2011-01-27 08:27:56]

Please undo the change to the way safepoint registers and doubles are pushed on
the stack. That will break stack traversal at safepoints. The rest looks good.
:)

http://codereview.chromium.org/6366016/diff/1/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/6366016/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:2536: Register tmp1 = input.is(r0) ? r1 : r0;
Kevin had a nicer suggestion for these. We intend to use r1,r2,r3,r4 for
tmp1,tmp2,tmp3,tmp4. Input can only be one of them and if it is we will use r0
instead.

Register tmp1 = input.is(r1) ? r0 : r1;
Register tmp2 = input.is(r2) ? r0 : r2;
Register tmp3 = input.is(r3) ? r0 : r3;

http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc#...
src/arm/macro-assembler-arm.cc:475: PushSafepointRegisters();
What is the motivation for changing this? Please undo this change. This will
break stack-traversal at safepoints because the traversal code expects the
registers that can contain pointers to be pushed first.

http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc#...
src/arm/macro-assembler-arm.cc:480: PopSafepointRegisters();
Undo please. :)

http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc#...
src/arm/macro-assembler-arm.cc:518: return MemOperand(sp,
SafepointRegisterStackIndex(reg.code()) * kInstrSize);
kInstrSize -> kPointerSize. This is an index into the stack and not the
instruction stream. Also, please undo your changes to where the doubles are
pushed and take that into account here when computing the offset instead.

[Alexandre, 2011-01-28 13:57:59]

Thanks for the review!

Alexandre

http://codereview.chromium.org/6366016/diff/1/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/6366016/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:2536: Register tmp1 = input.is(r0) ? r1 : r0;
I like that! Done.
On 2011/01/27 08:27:56, Mads Ager wrote:
> Kevin had a nicer suggestion for these. We intend to use r1,r2,r3,r4 for
> tmp1,tmp2,tmp3,tmp4. Input can only be one of them and if it is we will use r0
> instead.
> 
> Register tmp1 = input.is(r1) ? r0 : r1;
> Register tmp2 = input.is(r2) ? r0 : r2;
> Register tmp3 = input.is(r3) ? r0 : r3;
>

http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc#...
src/arm/macro-assembler-arm.cc:475: PushSafepointRegisters();
Reversed.

We usually want to patch general purpose registers on the stack and don't touch
double registers. I thought having them last on the stack would make it easier
to patch them, as they would always (with or without double regs) lie just
before sp.

I thought there might be some issue, but it did not break the tests. I don't
know if that would be easy to do, but it may be worth adding a test for this.

On 2011/01/27 08:27:56, Mads Ager wrote:
> What is the motivation for changing this? Please undo this change. This will
> break stack-traversal at safepoints because the traversal code expects the
> registers that can contain pointers to be pushed first.

http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc#...
src/arm/macro-assembler-arm.cc:480: PopSafepointRegisters();
On 2011/01/27 08:27:56, Mads Ager wrote:
> Undo please. :)

Done.

http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc#...
src/arm/macro-assembler-arm.cc:518: return MemOperand(sp,
SafepointRegisterStackIndex(reg.code()) * kInstrSize);
On 2011/01/27 08:27:56, Mads Ager wrote:
> kInstrSize -> kPointerSize. This is an index into the stack and not the
> instruction stream. Also, please undo your changes to where the doubles are
> pushed and take that into account here when computing the offset instead.

Done.

[Mads Ager (chromium), 2011-01-31 10:10:41]

LGTM