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#newcode2194
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#newcode2285
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#newcode2292
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#newcode2310
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-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):
https://codereview.chromium.org/1149903005/diff/40001/src/ia32/code-stubs-ia32.cc#newcode4680
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-descriptors-ia32.cc
File src/ia32/interface-descriptors-ia32.cc (right):
https://codereview.chromium.org/1149903005/diff/40001/src/ia32/interface-descriptors-ia32.cc#newcode35
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-descriptors.h
File src/interface-descriptors.h (right):
https://codereview.chromium.org/1149903005/diff/40001/src/interface-descriptors.h#newcode287
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
https://codereview.chromium.org/1149903005/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
