Reviewers: Sven Panne,
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.cc
File src/code-stubs.cc (right):
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.cc#newcode459
src/code-stubs.cc:459: void CompareNilICStub::Types::Print(StringStream*
stream) const {
On 2013/05/13 14:24:50, Sven Panne wrote:
This doesn't produce very readable output when Types contains more
than a single
element.
ok, done
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.h
File src/code-stubs.h (right):
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.h#newcode1045
src/code-stubs.h:1045: class Types {
On 2013/05/13 14:24:50, Sven Panne wrote:
Instead of inventing tons of ad-hoc helper functions, publicly derive
from
EnumSet<Type,byte> and add just Print(StringStream*), perhaps
FullCompare().
Nothing else is really needed.
Good point -> Done
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.h#newcode1135
src/code-stubs.h:1135: virtual Code::ExtraICState GetExtraICState() {
On 2013/05/13 14:24:50, Sven Panne wrote:
Use the BitField template instead of doing the bit fiddling by hand.
They are BitFields, except for the types, since those are just the lsb,
so no need to encode anything there.
Or maybe i don't fully get the bitField templates?
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.h#newcode1146
src/code-stubs.h:1146:
On 2013/05/13 14:24:50, Sven Panne wrote:
Do we really need all those tiny helper functions below? One reason
could be
avoiding "train wrecks" like foo()->bar()->baz(), but I am not sure if
this is
the case here.
i mostly introduced them to make the callees code more readable, but i
agree, having two different accessors is bad
https://codereview.chromium.org/14862009/diff/1/src/code-stubs.h#newcode1152
src/code-stubs.h:1152: const Types GetTypes() const { return types_; }
On 2013/05/13 14:24:50, Sven Panne wrote:
Why 2 consts?
1. the returned types shall be const
2. the method does not change the stub itself -> const
https://codereview.chromium.org/14862009/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/14862009/diff/1/src/hydrogen.cc#newcode10687
src/hydrogen.cc:10687: if (nil == kNullValue) {
On 2013/05/13 14:24:50, Sven Panne wrote:
Using a ternary ?: is clearer here.
ok
Description:
Encapsulating Type information in the CompareICStub
Encapsulate type information in a convenient wrapper instead of storing it
in a
naked bitfield. This especially facilitates transitioning to a new state and
converting from/to the extraICState representation. There is some code
duplication from ToBooleanICStub::Types, which might be pulled to a common
template later.
BUG=
Please review this at https://codereview.chromium.org/14862009/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/code-stubs.h
M src/code-stubs.cc
M src/hydrogen.cc
M src/ic.h
M src/ic.cc
M src/objects.h
M src/objects.cc
M src/string-stream.h
M src/string-stream.cc
M src/stub-cache.h
M src/stub-cache.cc
M src/type-info.cc
M test/cctest/cctest.gyp
A test/cctest/test-compare-nil-ic-stub.cc
--
--
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/groups/opt_out.
