Use IC register definitions in platform files.

Use IC register definitions in platform files.

R=jkummerow@chromium.org

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

[Jakob Kummerow, 2014-06-25 10:59:12]

Yes! More of this, please! :-)

https://codereview.chromium.org/356713003/diff/60001/src/ia32/full-codegen-ia...
File src/ia32/full-codegen-ia32.cc (right):

https://codereview.chromium.org/356713003/diff/60001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:1421: // Use inline caching. Variable name is
passed in ecx and the global
nit: outdated comment. I think you can just delete it, it doesn't really provide
insight.

https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):

https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc#newc...
src/ia32/ic-ia32.cc:960: ASSERT(edx.is(ReceiverRegister()));
I'm not too happy with these ASSERTs, as opposed to using ReceiverRegister()
instead of raw "edx" below. But I concede that it's hard to decide where to draw
the line. I can live with the ASSERT, at least for now.

https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc#newc...
src/ia32/ic-ia32.cc:983: static const Register LoadIC_TempRegister() { return
ebx; }
As discussed, I think medium-term we want to define this globally like
ReceiverRegister() and friends. But for now this is OK.

https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc#newc...
src/ia32/ic-ia32.cc:988: //  -- ReceiverRegister()  : receiver
Arguably this entire comment block could now be reduced to:
// Return address is on the stack (esp[0]).
But I'm fine with keeping it.

[mvstanton, 2014-06-25 15:10:27]

I've explored new dimensions of tedium, and invite you to do the same :). PTAL,
--Michael

https://codereview.chromium.org/356713003/diff/60001/src/ia32/full-codegen-ia...
File src/ia32/full-codegen-ia32.cc (right):

https://codereview.chromium.org/356713003/diff/60001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:1421: // Use inline caching. Variable name is
passed in ecx and the global
On 2014/06/25 10:59:12, Jakob wrote:
> nit: outdated comment. I think you can just delete it, it doesn't really
provide
> insight.

Done.

https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):

https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc#newc...
src/ia32/ic-ia32.cc:960: ASSERT(edx.is(ReceiverRegister()));
On 2014/06/25 10:59:12, Jakob wrote:
> I'm not too happy with these ASSERTs, as opposed to using ReceiverRegister()
> instead of raw "edx" below. But I concede that it's hard to decide where to
draw
> the line. I can live with the ASSERT, at least for now.

Okay, yep. I'm cautious because when code starts mentioned 4, 5 or 6 registers 
within a few lines I feel like the reader is best served by knowing what those
registers are. It's extra cognitive load to remember "which one is
ReceiverRegister()? Oh yeah." That is, the reader will actually turn it back
into a register.

Plus when you consider that one of the IC spec registers plays another role
(say, it's the accumulator and a call is made), then you really have to be
careful against obscuring the plain old register name.

At any rate, I'll be back in this code in order to unify LoadIC and KeyedLoadIC
register specs. I'll pay the price for any failure to fully push through a
useful abstraction! And in doing that work I'll push the abstraction deeper
wherever possible.

https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc#newc...
src/ia32/ic-ia32.cc:983: static const Register LoadIC_TempRegister() { return
ebx; }
On 2014/06/25 10:59:12, Jakob wrote:
> As discussed, I think medium-term we want to define this globally like
> ReceiverRegister() and friends. But for now this is OK.

Okay, thanks. I will keep it this way for now. I'm reluctant to elevate temp
registers as a concept for the platform independent file to be concerned with.
Lets keep playing the refactoring out and the right home for these will become
clearer.

https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc#newc...
src/ia32/ic-ia32.cc:988: //  -- ReceiverRegister()  : receiver
On 2014/06/25 10:59:12, Jakob wrote:
> Arguably this entire comment block could now be reduced to:
> // Return address is on the stack (esp[0]).
> But I'm fine with keeping it.

It's a super-redundant comment, so I'll take it out. :D

[Jakob Kummerow, 2014-06-25 16:17:41]

LGTM.

There are many more places where we should use the new register definitions, but
that can wait. (arm64 ic/stub generation quite consistently uses symbolic names
already; other platforms should follow suit.)

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

https://codereview.chromium.org/356713003/diff/100001/src/arm/lithium-codegen...
src/arm/lithium-codegen-arm.cc:3073: ASSERT(ToRegister(instr->object()).is(r0));
don't need this any more

https://codereview.chromium.org/356713003/diff/100001/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):

https://codereview.chromium.org/356713003/diff/100001/src/ia32/ic-ia32.cc#new...
src/ia32/ic-ia32.cc:575: Register receiver = edx;
awww... come on!

[mvstanton, 2014-06-26 09:18:12]

Thanks Jakob. I pushed the use of Receiver/NameRegister() further in the places
we discussed. Eliminated hilarious lack-of-usage. All the best, submitting,
--Michael

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

https://codereview.chromium.org/356713003/diff/100001/src/arm/lithium-codegen...
src/arm/lithium-codegen-arm.cc:3073: ASSERT(ToRegister(instr->object()).is(r0));
On 2014/06/25 16:17:40, Jakob wrote:
> don't need this any more

thx, missed that one!

https://codereview.chromium.org/356713003/diff/100001/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):

https://codereview.chromium.org/356713003/diff/100001/src/ia32/ic-ia32.cc#new...
src/ia32/ic-ia32.cc:575: Register receiver = edx;
On 2014/06/25 16:17:40, Jakob wrote:
> awww... come on!

Haha! Okay I will try and do a little better :).

[Jakob Kummerow, 2014-06-26 09:54:06]

LGTM, thanks!

[mvstanton, 2014-06-26 10:01:00]

Committed patchset #6 manually as r22035 (presubmit successful).