ARM: Add type-feedback recording for compare...

ARM: Add type-feedback recording for compare

Change the comparison in the full code generator to use CompareIC instead of the CompareStub to record the types. This also implements the patching in the full code generator where the inlined smi code is de-activated by default to call the CompareIC once and then activating the inlined smi code by patching the code.

Fixed the smi comparison in the ICCompareStub.

Fixed ToBooleanStub to ensure that the scratch register used is not the input. Use r9 as default as that will never be input with Crankshaft.

Implemented lithium instruction CmpTAndBranch.

Make sure that the lithium instruction CmpID have operands in registrers as the current optimized code expects that.

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

[Mads Ager (chromium), 2011-02-09 13:55:19]

LGTM

[Alexandre, 2011-02-09 13:57:16]

Just a few comments.

The v8 benchmarks performance improvement are impressive!

Alexandre

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

http://codereview.chromium.org/6461017/diff/1/src/arm/assembler-arm.cc#newcod...
src/arm/assembler-arm.cc:355: Condition Assembler::GetCondition(Instr instr) {
Same as Instruction::ConditionField(). Do we want to redefine this?

http://codereview.chromium.org/6461017/diff/1/src/arm/assembler-arm.cc#newcod...
src/arm/assembler-arm.cc:514: 
Not sure, but should we move these to the Instruction class in constants-arm.* ?

http://codereview.chromium.org/6461017/diff/1/src/arm/assembler-arm.h
File src/arm/assembler-arm.h (right):

http://codereview.chromium.org/6461017/diff/1/src/arm/assembler-arm.h#newcode...
src/arm/assembler-arm.h:1103: static Condition GetCondition(Instr instr);
Same as Instruction::ConditionField(). Do we want to redefine this?

http://codereview.chromium.org/6461017/diff/1/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

http://codereview.chromium.org/6461017/diff/1/src/arm/code-stubs-arm.cc#newco...
src/arm/code-stubs-arm.cc:1301: Register scratch = r9;
Is this more readable?
Register scratch = r9.is(tos_) ? r7 : r9;

http://codereview.chromium.org/6461017/diff/1/src/arm/constants-arm.h
File src/arm/constants-arm.h (right):

http://codereview.chromium.org/6461017/diff/1/src/arm/constants-arm.h#newcode144
src/arm/constants-arm.h:144: kConditionMask = 15 << 28      // Mask for the
condition field.
kCondMask was already defined  (~line 259).
I think we should keep only one; but I think any of them is fine though.

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

http://codereview.chromium.org/6461017/diff/1/src/arm/full-codegen-arm.cc#new...
src/arm/full-codegen-arm.cc:50: // class has a number of methods the emit the
code which is patchable and the
typo: "the emit the code" -> "to emit the code" ?

[Karl Klose, 2011-02-09 14:35:29]

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

http://codereview.chromium.org/6461017/diff/1/src/arm/full-codegen-arm.cc#new...
src/arm/full-codegen-arm.cc:92: __ cmp_raw_immediate(r0, delta_to_patch_site %
kOff12Mask);
The destination register of the cmp should be reg.

[Søren Thygesen Gjesse, 2011-02-09 14:48:28]

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

http://codereview.chromium.org/6461017/diff/1/src/arm/assembler-arm.cc#newcod...
src/arm/assembler-arm.cc:355: Condition Assembler::GetCondition(Instr instr) {
On 2011/02/09 13:57:16, Alexandre wrote:
> Same as Instruction::ConditionField(). Do we want to redefine this?

This now calls Instruction::ConditionField()

http://codereview.chromium.org/6461017/diff/1/src/arm/assembler-arm.cc#newcod...
src/arm/assembler-arm.cc:514: 
On 2011/02/09 13:57:16, Alexandre wrote:
> Not sure, but should we move these to the Instruction class in constants-arm.*
?

I think it will be fine to have these in the Instruction class instead of the
assembler. However leaving them here for now.

http://codereview.chromium.org/6461017/diff/1/src/arm/assembler-arm.h
File src/arm/assembler-arm.h (right):

http://codereview.chromium.org/6461017/diff/1/src/arm/assembler-arm.h#newcode...
src/arm/assembler-arm.h:1103: static Condition GetCondition(Instr instr);
On 2011/02/09 13:57:16, Alexandre wrote:
> Same as Instruction::ConditionField(). Do we want to redefine this?

This now calls Instruction::ConditionField().

http://codereview.chromium.org/6461017/diff/1/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):

http://codereview.chromium.org/6461017/diff/1/src/arm/code-stubs-arm.cc#newco...
src/arm/code-stubs-arm.cc:1301: Register scratch = r9;
On 2011/02/09 13:57:16, Alexandre wrote:
> Is this more readable?
> Register scratch = r9.is(tos_) ? r7 : r9;

Done.

http://codereview.chromium.org/6461017/diff/1/src/arm/constants-arm.h
File src/arm/constants-arm.h (right):

http://codereview.chromium.org/6461017/diff/1/src/arm/constants-arm.h#newcode144
src/arm/constants-arm.h:144: kConditionMask = 15 << 28      // Mask for the
condition field.
On 2011/02/09 13:57:16, Alexandre wrote:
> kCondMask was already defined  (~line 259).
> I think we should keep only one; but I think any of them is fine though.

Removed this again.

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

http://codereview.chromium.org/6461017/diff/1/src/arm/full-codegen-arm.cc#new...
src/arm/full-codegen-arm.cc:50: // class has a number of methods the emit the
code which is patchable and the
On 2011/02/09 13:57:16, Alexandre wrote:
> typo: "the emit the code" -> "to emit the code" ?

Done.

http://codereview.chromium.org/6461017/diff/1/src/arm/full-codegen-arm.cc#new...
src/arm/full-codegen-arm.cc:92: __ cmp_raw_immediate(r0, delta_to_patch_site %
kOff12Mask);
On 2011/02/09 14:35:30, Karl Klose wrote:
> The destination register of the cmp should be reg.

Good catch, fixed.