[arm] Drop useless branches in full and lithium codegen.

[arm] Drop useless branches in full and lithium codegen.

R=svenpanne@chromium.org

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

[Benedikt Meurer, 2013-12-12 13:07:52]

Hey Sven,
Here's another cleanup of useless branches in ARM full and lithium codegen.
PTAL
-- Benedikt

[Sven Panne, 2013-12-12 13:43:23]

LGTM in principle, but I would like to see Rodolph's comments regarding the
performance impact, too.

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

https://codereview.chromium.org/100483006/diff/1/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:1418: __ cmp(right, Operand(-1), eq);
Is conditional non-execution of 2 instructions really better than a probably
correctly predicted jump around them? I think it is (IIRC the threshold was
around 3 instructions), but I'll let our ARM black belts decide... :-)

https://codereview.chromium.org/100483006/diff/1/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:1519: __ cmp(right, Operand(-1), eq);
Same here...

https://codereview.chromium.org/100483006/diff/1/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:3445: __ ldr(result, MemOperand(fp,
StandardFrameConstants::kCallerFPOffset), ne);
... and here (3 instructions now)

https://codereview.chromium.org/100483006/diff/1/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:3875: __ vadd(result, input, kDoubleRegZero, ne);
... and here.

[Rodolph Perfetta, 2013-12-13 11:31:14]

My rule of thumb would be similar to Sven's one: conditional non-execution of
2-3 "simple" instructions is equivalent to a probably correctly predicted jump
around them. If the jump is not easily predicted then you can add 1-2 simple
instructions.

Of course there are other factors to take into account: the branch predictor has
limited resources and has such can only track a limited number of conditional
branches, with a limited density. Using conditional instructions instead of
branches free some resources. On some earlier cores we had performance issues
due to the branch predictor being overwhelmed (octane/crypto was the culprit
IIRC)

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

https://codereview.chromium.org/100483006/diff/1/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:1418: __ cmp(right, Operand(-1), eq);
On 2013/12/12 13:43:23, Sven Panne wrote:
> Is conditional non-execution of 2 instructions really better than a probably
> correctly predicted jump around them? I think it is (IIRC the threshold was
> around 3 instructions), but I'll let our ARM black belts decide... :-)

conditional execution of two "simple" instructions will be comparable to the
branch case. Since we save space, this is good.

https://codereview.chromium.org/100483006/diff/1/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:3445: __ ldr(result, MemOperand(fp,
StandardFrameConstants::kCallerFPOffset), ne);
On 2013/12/12 13:43:23, Sven Panne wrote:
> ... and here (3 instructions now)

Here the answer would depend on the surrounding code (e.g. how many loads/stores
around). If ArgumentsLength is not too common (I believe it is the case) then we
are fine.

https://codereview.chromium.org/100483006/diff/1/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:3875: __ vadd(result, input, kDoubleRegZero, ne);
On 2013/12/12 13:43:23, Sven Panne wrote:
> ... and here.

vadd and especially vsqrt are complex instructions. Complex instructions don't
interact too well with conditions and the CPU could perform the entire operation
before discarding the result if the condition is false. So here we'll be slower
on most modern CPU. If we were in full-codegen that would be acceptable though.

[Benedikt Meurer, 2014-01-02 06:49:16]

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

[Benedikt Meurer, 2014-01-02 06:49:28]

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

https://codereview.chromium.org/100483006/diff/1/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:3445: __ ldr(result, MemOperand(fp,
StandardFrameConstants::kCallerFPOffset), ne);
Reverted this one.

https://codereview.chromium.org/100483006/diff/1/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:3875: __ vadd(result, input, kDoubleRegZero, ne);
On 2013/12/13 11:31:14, Rodolph Perfetta wrote:
> On 2013/12/12 13:43:23, Sven Panne wrote:
> > ... and here.
> 
> vadd and especially vsqrt are complex instructions. Complex instructions don't
> interact too well with conditions and the CPU could perform the entire
operation
> before discarding the result if the condition is false. So here we'll be
slower
> on most modern CPU. If we were in full-codegen that would be acceptable
though.

Done.