The IC should define its input registers

Create a RegisterSpec class inside of the IC that provides:
1) symbolic names for the register (like, edx == receiver)
2) can return an array of registers
3) defines ordering when passed on the stack

Code that implements or uses the IC should use this RegisterSpec instead of "knowing" what the registers are. Or at least have the RegisterSpec to validate it's assumptions.

R=jkummerow@chromium.org

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

[mvstanton, 2014-06-19 08:14:47]

Hi guys,
Could you have a quick look at this structure for declaring a register
specification in the IC? Thanks!
--Michael

ps - Jakob, I know I'm not solving the issue of (proliferating) static
initializers yet, but I will think about how to do that.

[mvstanton, 2014-06-20 08:08:08]

Addressed some feedback. Instead of a baroque macro-assisted helper class, I
just have simple static constant registers. PTAL, thanks!

Also extended usage to code-stubs-hydrogen.cc.
--Michael

[Jakob Kummerow, 2014-06-20 10:50:27]

I think this is going in the right direction. A few comments:

(1) KeyedLoadIC::GetRegisterArray() vs. KeyedLoadStubCompiler::registers() vs.
KeyedLoadFastElementStub::InitializeInterfaceDescriptor() feels like a case of
http://xkcd.com/927/. Do you have plans to unify them further? (It's fine to
postpone this to a future CL.)

(2) I'm unhappy about adding more static register arrays. IIRC TSan even needs
custom exceptions or it'll complain about those. Any ideas what we can do about
that?

Happy to discuss offline.

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

https://codereview.chromium.org/340363002/diff/60001/src/ia32/ic-ia32.cc#newc...
src/ia32/ic-ia32.cc:1017: __ push(edx);  // receiver
Y U NO use KeyedLoadIC::kReceiverRegister here (and elsewhere in this file)?

https://codereview.chromium.org/340363002/diff/60001/src/ic.h
File src/ic.h (right):

https://codereview.chromium.org/340363002/diff/60001/src/ic.h#newcode402
src/ic.h:402: kRegisterArgumentCount = 2,
nit: we usually put the "count" value of enums in the end (which would even make
it unnecessary to assign 0, 1, 2 explicitly, but it's fine to keep them for
clarity).

[Jakob Kummerow, 2014-06-23 12:49:09]

LGTM. I like this!

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

https://codereview.chromium.org/340363002/diff/120001/src/ia32/ic-ia32.cc#new...
src/ia32/ic-ia32.cc:1032: LoadIC::kReceiverRegister, LoadIC::kNameRegister
nit: does it still compile if you omit "LoadIC::" here? I think I'd prefer that,
if the compiler lets us.

[mvstanton, 2014-06-23 12:56:30]

Hi Jakob, 
Here is a version that

1) avoids static initialization (Registers can be initialized as const arrays,
avoiding the dreaded thread safety problems. I'll propagate this into another CL
for other constant arrays in the system.

2) doesn't yet widely use the new constants beyond what you see here. There are
a lot of changes to be made in full codegen-<platform> and ic-<platform> and
elsewhere, and that would swamp this CL: follow-on!

3) doesn't directly address the hilarious xkcd issue, though at least I avoided
creating a new standard (ie, competing structure) :).

Thanks for the help,
--Michael

https://codereview.chromium.org/340363002/diff/60001/src/ic.h
File src/ic.h (right):

https://codereview.chromium.org/340363002/diff/60001/src/ic.h#newcode402
src/ic.h:402: kRegisterArgumentCount = 2,
On 2014/06/20 10:50:27, Jakob wrote:
> nit: we usually put the "count" value of enums in the end (which would even
make
> it unnecessary to assign 0, 1, 2 explicitly, but it's fine to keep them for
> clarity).

Done.

[mvstanton, 2014-06-23 13:42:54]

Committed patchset #7 manually as r21939 (presubmit successful).