Vector ICs: Introduce Store and KeyedStore IC code stubs.

Vector ICs: Introduce Store and KeyedStore IC code stubs.

Also introduce new interface descriptors for the trampoline and full
versions of those stubs.

Currently, the stubs aren't functional.

BUG=

Committed: https://crrev.com/a86384f192f88458bb46eece0a8f2709979057b8
Cr-Commit-Position: refs/heads/master@{#28597}

[mvstanton, 2015-05-22 10:39:26]

mvstanton@chromium.org changed reviewers:
+ jkummerow@chromium.org

[mvstanton, 2015-05-22 10:39:26]

Hi Jakob,
Here is the 2nd one. It creates the necessary platform stubs (unimplemented),
and their descriptors, modeled very much on the LoadIC story...
--Michael

[Jakob Kummerow, 2015-05-22 11:38:17]

LGTM with comments, mostly nits.

The ia32 comments apply to all platform ports.

https://codereview.chromium.org/1149903005/diff/40001/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/1149903005/diff/40001/src/code-stubs.h#newcod...
src/code-stubs.h:2194: explicit VectorKeyedStoreICTrampolineStub(Isolate*
isolate,
nit: "explicit" is unnecessary

https://codereview.chromium.org/1149903005/diff/40001/src/code-stubs.h#newcod...
src/code-stubs.h:2285: explicit VectorStoreICStub(Isolate* isolate, const
StoreICState& state)
nit: "explicit" is unnecessary

https://codereview.chromium.org/1149903005/diff/40001/src/code-stubs.h#newcod...
src/code-stubs.h:2292: virtual Code::Kind GetCodeKind() const override { return
Code::STORE_IC; }
nit: the cool kids use only one of "virtual", "override", "final" these days,
specifically the most restrictive that's applicable. (Again several times
below.)

https://codereview.chromium.org/1149903005/diff/40001/src/code-stubs.h#newcod...
src/code-stubs.h:2310: explicit VectorKeyedStoreICStub(Isolate* isolate, const
StoreICState& state)
nit: "explicit" is unnecessary

https://codereview.chromium.org/1149903005/diff/40001/src/ia32/code-stubs-ia3...
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/1149903005/diff/40001/src/ia32/code-stubs-ia3...
src/ia32/code-stubs-ia32.cc:4680: // TODO(mvstanton): Implement.
If you put "UNIMPLEMENTED();" here, you'll get a loud and clear warning
(V8_Fatal) if you ever accidentally try to execute this. That may be helpful or
annoying for the development/debugging workflow you have in mind, so it's up to
you.

https://codereview.chromium.org/1149903005/diff/40001/src/ia32/interface-desc...
File src/ia32/interface-descriptors-ia32.cc (right):

https://codereview.chromium.org/1149903005/diff/40001/src/ia32/interface-desc...
src/ia32/interface-descriptors-ia32.cc:35: const Register
StoreTransitionDescriptor::MapRegister() { return ebx; }
Does this not clash? (Can always punt and fix it when it does.)

https://codereview.chromium.org/1149903005/diff/40001/src/interface-descripto...
File src/interface-descriptors.h (right):

https://codereview.chromium.org/1149903005/diff/40001/src/interface-descripto...
src/interface-descriptors.h:287: class LoadWithVectorDescriptor : public
LoadDescriptor {
Drive-by: maybe put a TODO here to unify these two, now that loads always use
the vector?

[mvstanton, 2015-05-22 12:04:29]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2015-05-22 12:04:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1149903005/#ps60001
(title: "Code comments.")

[commit-bot: I haz the power, 2015-05-22 12:04:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1149903005/60001

[mvstanton, 2015-05-22 12:06:17]

Thanks Jakob for the helpful comments! Submittin'...
--Michael

https://codereview.chromium.org/1149903005/diff/40001/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/1149903005/diff/40001/src/code-stubs.h#newcod...
src/code-stubs.h:2194: explicit VectorKeyedStoreICTrampolineStub(Isolate*
isolate,
On 2015/05/22 11:38:17, Jakob wrote:
> nit: "explicit" is unnecessary

Done.

https://codereview.chromium.org/1149903005/diff/40001/src/code-stubs.h#newcod...
src/code-stubs.h:2285: explicit VectorStoreICStub(Isolate* isolate, const
StoreICState& state)
On 2015/05/22 11:38:17, Jakob wrote:
> nit: "explicit" is unnecessary

Done.

https://codereview.chromium.org/1149903005/diff/40001/src/code-stubs.h#newcod...
src/code-stubs.h:2292: virtual Code::Kind GetCodeKind() const override { return
Code::STORE_IC; }
On 2015/05/22 11:38:17, Jakob wrote:
> nit: the cool kids use only one of "virtual", "override", "final" these days,
> specifically the most restrictive that's applicable. (Again several times
> below.)

W00t, looks cleaner..thx.

https://codereview.chromium.org/1149903005/diff/40001/src/code-stubs.h#newcod...
src/code-stubs.h:2310: explicit VectorKeyedStoreICStub(Isolate* isolate, const
StoreICState& state)
On 2015/05/22 11:38:17, Jakob wrote:
> nit: "explicit" is unnecessary

Done.

https://codereview.chromium.org/1149903005/diff/40001/src/ia32/code-stubs-ia3...
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/1149903005/diff/40001/src/ia32/code-stubs-ia3...
src/ia32/code-stubs-ia32.cc:4680: // TODO(mvstanton): Implement.
On 2015/05/22 11:38:17, Jakob wrote:
> If you put "UNIMPLEMENTED();" here, you'll get a loud and clear warning
> (V8_Fatal) if you ever accidentally try to execute this. That may be helpful
or
> annoying for the development/debugging workflow you have in mind, so it's up
to
> you.

Cool. Actually, though I'm pleased enough with this. In the next couple of days
I'll be calling this thing, and I'm happy with it just missing. That way I can
test out components of the vector store pipeline, and this will act as a "pass
through" node.

https://codereview.chromium.org/1149903005/diff/40001/src/ia32/interface-desc...
File src/ia32/interface-descriptors-ia32.cc (right):

https://codereview.chromium.org/1149903005/diff/40001/src/ia32/interface-desc...
src/ia32/interface-descriptors-ia32.cc:35: const Register
StoreTransitionDescriptor::MapRegister() { return ebx; }
On 2015/05/22 11:38:17, Jakob wrote:
> Does this not clash? (Can always punt and fix it when it does.)

It definitely does...I noticed that. In fact, it indicates quite a little issue
to solve. I'll leave it for now but it's very much present for me in the next
days of implementation.

https://codereview.chromium.org/1149903005/diff/40001/src/interface-descripto...
File src/interface-descriptors.h (right):

https://codereview.chromium.org/1149903005/diff/40001/src/interface-descripto...
src/interface-descriptors.h:287: class LoadWithVectorDescriptor : public
LoadDescriptor {
On 2015/05/22 11:38:17, Jakob wrote:
> Drive-by: maybe put a TODO here to unify these two, now that loads always use
> the vector?

Ah, okay. No, this is appropriate and maybe it's a naming issue.
Now...LoadDescriptor is a thing that, being inherently vectorized has a
SlotRegister(). And LoadWithVectorDescriptor is for the version of the IC called
by optimized code where the vector is explicitly passed, rather than retrieved
from the frame.

So, "Vector" here, now, means actually pass the vector. The absence of the word
means, please get it from the frame.

The new StoreICs will go through a similar naming convolution, first using the
word Vector to indicate a type of thing...a "vectorized ic", and later using it
to mean something more humdrum. :p

[commit-bot: I haz the power, 2015-05-22 12:21:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-22 12:21:06]

Try jobs failed on following builders:
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)

[mvstanton, 2015-05-22 14:08:22]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2015-05-22 14:08:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1149903005/#ps80001
(title: "REBASE.")

[commit-bot: I haz the power, 2015-05-22 14:08:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1149903005/80001

[commit-bot: I haz the power, 2015-05-22 14:36:55]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-05-22 14:37:13]

Patchset 5 (id:??) landed as
https://crrev.com/a86384f192f88458bb46eece0a8f2709979057b8
Cr-Commit-Position: refs/heads/master@{#28597}