Port %ClassOf() optimization to X64 and ARM.

Port %ClassOf() optimization to X64 and ARM.

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

[Erik Corry, 2009-07-02 13:48:31]

http://codereview.chromium.org/150226/diff/5/1006
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/150226/diff/5/1006#newcode3187
Line 3187: null.Branch(eq);
Ideally I'd like to do this as a conditional move followed by a conditional
branch to done, but how to explain that to the register allocator?

http://codereview.chromium.org/150226/diff/5/1006#newcode3193
Line 3193: __ cmp(r1, Operand(FIRST_JS_OBJECT_TYPE));
You can do these 3 in one with __ CompareObjectType(r0, r0, r1, JS_OBJECT_TYPE)

http://codereview.chromium.org/150226/diff/5/1006#newcode3206
Line 3206: __ CompareObjectType(r0, r1, r1, JS_FUNCTION_TYPE);
r0 = Object.
r1 = Map.
r1 = type (overwrites map)
Is that what you wanted?

http://codereview.chromium.org/150226/diff/5/1006#newcode3209
Line 3209: // The map register now contains the constructor function. Grab the
Map register is r0?  I think r0 is the constructor here.

http://codereview.chromium.org/150226/diff/5/1005
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/150226/diff/5/1005#newcode3378
Line 3378: __ testl(obj.reg(), Immediate(kSmiTagMask));
Do we need an l here?  Will that preclude an 8 bit immediate being used?

http://codereview.chromium.org/150226/diff/5/1005#newcode3386
Line 3386: __ cmpb(tmp.reg(), Immediate(FIRST_JS_OBJECT_TYPE));
CmpObjectType?

[Erik Corry, 2009-07-02 13:50:54]

LGTM

[Mads Ager (chromium), 2009-07-02 14:14:08]

Thanks!

http://codereview.chromium.org/150226/diff/5/1006
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/150226/diff/5/1006#newcode3193
Line 3193: __ cmp(r1, Operand(FIRST_JS_OBJECT_TYPE));
On 2009/07/02 13:48:31, Erik Corry wrote:
> You can do these 3 in one with __ CompareObjectType(r0, r0, r1,
JS_OBJECT_TYPE)

Ah, yes, there is one more register parameter to CmpObjectType on ARM. Done.

http://codereview.chromium.org/150226/diff/5/1006#newcode3206
Line 3206: __ CompareObjectType(r0, r1, r1, JS_FUNCTION_TYPE);
On 2009/07/02 13:48:31, Erik Corry wrote:
> r0 = Object.
> r1 = Map.
> r1 = type (overwrites map)
> Is that what you wanted?

Yes, that is fine.  All I need is for the constructor to survive in r0.

http://codereview.chromium.org/150226/diff/5/1006#newcode3209
Line 3209: // The map register now contains the constructor function. Grab the
On 2009/07/02 13:48:31, Erik Corry wrote:
> Map register is r0?  I think r0 is the constructor here.

Yeah, strange comment.  Changed it to 'The r0 register ...'.

http://codereview.chromium.org/150226/diff/5/1005
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/150226/diff/5/1005#newcode3378
Line 3378: __ testl(obj.reg(), Immediate(kSmiTagMask));
On 2009/07/02 13:48:31, Erik Corry wrote:
> Do we need an l here?  Will that preclude an 8 bit immediate being used?

I'm using testl for consistency with the rest of the code.  I believe the
encoding for testl is shorter.

http://codereview.chromium.org/150226/diff/5/1005#newcode3386
Line 3386: __ cmpb(tmp.reg(), Immediate(FIRST_JS_OBJECT_TYPE));
On 2009/07/02 13:48:31, Erik Corry wrote:
> CmpObjectType?

Can't on x64/ia32 because the map register is clobbered by CmpObjectType.  Maybe
we should add another argument to CmpObjectType as on ARM.

[William Hesse, 2009-07-02 14:44:25]

http://codereview.chromium.org/150226/diff/5/1005
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/150226/diff/5/1005#newcode3378
Line 3378: __ testl(obj.reg(), Immediate(kSmiTagMask));
On 2009/07/02 14:14:08, Mads Ager wrote:
> On 2009/07/02 13:48:31, Erik Corry wrote:
> > Do we need an l here?  Will that preclude an 8 bit immediate being used?
> 
> I'm using testl for consistency with the rest of the code.  I believe the
> encoding for testl is shorter.
> 

testl does not preclude an 8-bit encoding of the immediate.  testb would work
just as well, but no better.  We just want to avoid the REX prefix in cases
where we don't need it.

http://codereview.chromium.org/150226/diff/5/1005#newcode3386
Line 3386: __ cmpb(tmp.reg(), Immediate(FIRST_JS_OBJECT_TYPE));
On 2009/07/02 14:14:08, Mads Ager wrote:
> On 2009/07/02 13:48:31, Erik Corry wrote:
> > CmpObjectType?
> 
> Can't on x64/ia32 because the map register is clobbered by CmpObjectType. 
Maybe
> we should add another argument to CmpObjectType as on ARM.

On x64 and ia32, the map register isn't clobbered by CmpObjectType anymore, so
CmpObjectType can be followed by a CmpInstanceType that uses the map register
found by it.

CmpObjectType(obj.reg(), FIRST_JS_OBJECT_TYPE, tmp_reg());
null.Branch(less);
CmpInstanceType(tmp.reg(), JS_FUNCTION_TYPE);
function.Branch(equal);

should work fine on both ia32 and x64.  On x64 one can also use
kScratchRegister.

http://codereview.chromium.org/150226/diff/5/1005#newcode3401
Line 3401: __ CmpObjectType(obj.reg(), JS_FUNCTION_TYPE, tmp.reg());
kScratchRegister, rather than tmp, can be used here.