ARM: Implement DoIsObject and DoIsObjectAndBranch in the lithium code generator..

ARM: Implement DoIsObject and DoIsObjectAndBranch in the lithium code generator.

BUG=none
TEST=none

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

[Søren Thygesen Gjesse, 2011-01-21 07:38:13]

LGTM

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

http://codereview.chromium.org/6308012/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1781: Condition true_cond = EmitIsObject(reg,
temp1, temp2, false_label, true_label);
Long line.

[Rodolph Perfetta, 2011-01-21 09:26:21]

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

http://codereview.chromium.org/6308012/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1747: return gt;
Shouldn't it return le (lower or equal)?

http://codereview.chromium.org/6308012/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1765: __ LoadRoot(result,
Heap::kTrueValueRootIndex, true_cond);
The condition is unnecessary, you only jump here if it is true.

Alternatively you can write:
LoadRoot(result, Heap::kFalseValueRootIndex, NegateCondition(true_cond));
LoadRoot(result, Heap::kFalseValueRootIndex, true_cond);
Then you don't need the branches.

[Karl Klose, 2011-01-21 18:04:09]

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

http://codereview.chromium.org/6308012/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1747: return gt;
On 2011/01/21 09:26:21, Rodolph Perfetta wrote:
> Shouldn't it return le (lower or equal)?

Done.

http://codereview.chromium.org/6308012/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1765: __ LoadRoot(result,
Heap::kTrueValueRootIndex, true_cond);
I removed the condition.

It was a leftover from writing the code in the form you propose. The problem is
that EmitIsObject may directly jump to the labels provided, so I decided to keep
the labels.
On 2011/01/21 09:26:21, Rodolph Perfetta wrote:
> The condition is unnecessary, you only jump here if it is true.
> 
> Alternatively you can write:
> LoadRoot(result, Heap::kFalseValueRootIndex, NegateCondition(true_cond));
> LoadRoot(result, Heap::kFalseValueRootIndex, true_cond);
> Then you don't need the branches.

http://codereview.chromium.org/6308012/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1781: Condition true_cond = EmitIsObject(reg,
temp1, temp2, false_label, true_label);
On 2011/01/21 07:38:13, Søren Gjesse wrote:
> Long line.

Done.

[Mads Ager (chromium), 2011-01-24 12:24:46]

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

http://codereview.chromium.org/6308012/diff/6001/src/arm/lithium-codegen-arm....
src/arm/lithium-codegen-arm.cc:1765: __ b(al, &done);
This is the same as __ b(&done);

I'll change that and then land. :)