The property IC compiler might have less-than-ideal naming conventions
(from its
point of view, "keyed" == "elements"), but that's no reason to make it even
more
confusing.
https://codereview.chromium.org/755513003/diff/40001/src/ic/ic-compiler.cc
File src/ic/ic-compiler.cc (right):
https://codereview.chromium.org/755513003/diff/40001/src/ic/ic-compiler.cc#newcode107
src/ic/ic-compiler.cc:107: isolate->factory()->empty_string(), ELEMENT);
See how it says ELEMENT here? This method is only used to compile
ELEMENT access ICs, and that's the only thing it *can* handle, so
there's really no need to pretend that it's more flexible than that.
Let's not add unnecessary complexity.
https://codereview.chromium.org/755513003/diff/40001/src/ic/ic-compiler.cc#newcode288
src/ic/ic-compiler.cc:288: key_type);
So here you're replacing the hard-coded ELEMENT with the supposedly
variable key_type, but at the only call site of this function, ELEMENT
is always passed in. Again, let's not add complexity that's not
required.
https://codereview.chromium.org/755513003/diff/40001/src/ic/ic-state.h
File src/ic/ic-state.h (right):
https://codereview.chromium.org/755513003/diff/40001/src/ic/ic-state.h#newcode239
src/ic/ic-state.h:239:
nit: no need to add these empty lines
https://codereview.chromium.org/755513003/diff/40001/src/ic/ic.h
File src/ic/ic.h (right):
https://codereview.chromium.org/755513003/diff/40001/src/ic/ic.h#newcode438
src/ic/ic.h:438: // ExtraICState bits
nit: duplicate comment, drop it.
https://codereview.chromium.org/755513003/diff/40001/test/mjsunit/generic-monorphic-keyed-load.js
File test/mjsunit/generic-monorphic-keyed-load.js (right):
https://codereview.chromium.org/755513003/diff/40001/test/mjsunit/generic-monorphic-keyed-load.js#newcode44
test/mjsunit/generic-monorphic-keyed-load.js:44: // generate
KeyedLoadGeneric, instead of element access and deopt check
nit: missing punctuation at the end.
Or you could just drop the comment. The test case doesn't really care
what code is generated.
https://codereview.chromium.org/755513003/diff/40001/test/mjsunit/generic-monorphic-keyed-load.js#newcode48
test/mjsunit/generic-monorphic-keyed-load.js:48: // Should not
deoptimize
nit: same here. Again, just drop it. It's obvious what the next line
does.
https://codereview.chromium.org/755513003/diff/40001/test/mjsunit/generic-monorphic-keyed-load.js#newcode49
test/mjsunit/generic-monorphic-keyed-load.js:49:
assertTrue(%GetOptimizationStatus(get) != 2);
Use "assertOptimized(get);" here.
https://codereview.chromium.org/755513003/
--
--
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.
