Emit VMLS for multiply-subtract on ARM.

Emit VMLS for multiply-subtract on ARM.

BUG=none

Committed: https://code.google.com/p/v8/source/detail?r=13748

[hans, 2013-02-26 11:35:57]

I've had this in my pipe since the patch to use VMLA for multiply-add, so I
figured I might as well upload it.

It's not clear to me how much of a performance win this is. Using VMLS instead
of VMUL + VSUB saves one instruction, but at least on Nexus 10, it seems the
VMLS is about as slow as the other two instructions together.

On Nexus 4 it seems to make a bit more difference. I updated my matul.js
benchmark to do subtraction where it did addition, and I got 4% speed-up.

We do hit this in Octane, especially in Box2D, and I think even more in Kraken.

I'd like to hear your thoughts on this.

[Rodolph Perfetta, 2013-02-26 13:19:23]

The guideline is that vmls should be faster (even if by a small margin) than the
equivalent vsub, vmul sequence, so that sounds like a good idea.

https://codereview.chromium.org/12319113/diff/1/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):

https://codereview.chromium.org/12319113/diff/1/src/arm/simulator-arm.cc#newc...
src/arm/simulator-arm.cc:2788: const bool op = (instr->Opc3Value() & 0x1);
Use a more explicit name for op.

https://codereview.chromium.org/12319113/diff/1/src/arm/simulator-arm.cc#newc...
src/arm/simulator-arm.cc:2801: if (op)
in the rest of the file curly brackets tend to be used even for one liner
if/else.

[hans, 2013-02-26 13:47:32]

> The guideline is that vmls should be faster (even if by a small margin) than
the equivalent vsub, vmul sequence, so that sounds like a good idea.

OK, sounds good to me.

https://codereview.chromium.org/12319113/diff/1/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):

https://codereview.chromium.org/12319113/diff/1/src/arm/simulator-arm.cc#newc...
src/arm/simulator-arm.cc:2788: const bool op = (instr->Opc3Value() & 0x1);
On 2013/02/26 13:19:23, Rodolph Perfetta wrote:
> Use a more explicit name for op.

Done.

https://codereview.chromium.org/12319113/diff/1/src/arm/simulator-arm.cc#newc...
src/arm/simulator-arm.cc:2788: const bool op = (instr->Opc3Value() & 0x1);
On 2013/02/26 13:19:23, Rodolph Perfetta wrote:
> Use a more explicit name for op.

Done.

https://codereview.chromium.org/12319113/diff/1/src/arm/simulator-arm.cc#newc...
src/arm/simulator-arm.cc:2801: if (op)
On 2013/02/26 13:19:23, Rodolph Perfetta wrote:
> in the rest of the file curly brackets tend to be used even for one liner
> if/else.

Done.

[Rodolph Perfetta, 2013-02-26 15:59:13]

lgtm

[ulan, 2013-02-27 09:24:36]

LGTM with a nit.

https://codereview.chromium.org/12319113/diff/4001/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

https://codereview.chromium.org/12319113/diff/4001/src/arm/lithium-arm.cc#new...
src/arm/lithium-arm.cc:1413: return DoMultiplySub(instr->left(),
HMul::cast(instr->right()));
Style nit: this needs curly braces.

[hans, 2013-02-27 09:45:02]

Thanks! New version uploaded.

https://codereview.chromium.org/12319113/diff/4001/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

https://codereview.chromium.org/12319113/diff/4001/src/arm/lithium-arm.cc#new...
src/arm/lithium-arm.cc:1413: return DoMultiplySub(instr->left(),
HMul::cast(instr->right()));
On 2013/02/27 09:24:36, ulan wrote:
> Style nit: this needs curly braces.

Done. And I added braces in the corresponding line in DoAdd() as well.

[ulan, 2013-02-27 10:04:46]

Thank you, I will land it for you.

[ulan, 2013-02-27 10:24:50]

Committed patchset #3 manually as r13748 (presubmit successful).