PTAL

https://codereview.chromium.org/17229005/diff/116001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/17229005/diff/116001/src/code-stubs-hydrogen.cc#newcode756
src/code-stubs-hydrogen.cc:756: Handle<Code> UnaryOpStub::GenerateCode()
{
On 2013/06/27 13:31:06, danno wrote:
Move this down after the BuildCode method for UnaryOpStub

Done.

https://codereview.chromium.org/17229005/diff/116001/src/code-stubs-hydrogen.cc#newcode761
src/code-stubs-hydrogen.cc:761: HInstruction*
CodeStubGraphBuilderBase::BuildUnaryOp(HValue* input,
On 2013/06/27 13:31:06, danno wrote:
I wouldn't put this on the BuilderBase. If there is really no way to
share this
code with the OptimizedGraphBuilder, then just make it a static
method.

Done.

https://codereview.chromium.org/17229005/diff/116001/src/code-stubs-hydrogen.cc#newcode798
src/code-stubs-hydrogen.cc:798: HInstruction* res = BuildUnaryOp(input,
type, stub->operation());
On 2013/06/27 13:31:06, danno wrote:
Yeah, I know it's a generated if, but I don't think indentation should
be
changed (here and below)

Done.

https://codereview.chromium.org/17229005/diff/116001/src/code-stubs-hydrogen.cc#newcode812
src/code-stubs-hydrogen.cc:812: Handle<Code>
CompareNilICStub::GenerateCode() {
On 2013/06/27 13:31:06, danno wrote:
Move this up to be under the CompareNil BuildCode method

Done.

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

https://codereview.chromium.org/17229005/diff/116001/src/code-stubs.h#newcode601
src/code-stubs.h:601: class OperatorBits      : public BitField<byte,
NUMBER_OF_TYPES, 1> {};
On 2013/06/27 13:31:06, danno wrote:
This isn't right. The NUMBER_OF_TYPES doesn't directly have to do
with the bit
you use, and I don't think there is an easy way to couple the two.
NUMBER_OF_TYPES is 2 but that's not a bit offset. There's also no
This should be:

class OperatorBits         : public BitField<Token, 0, 8> {};
class StateEnumSetBits  : public BitField<int, 8, 3> {};

And the rest of the code should use those.

I think the code is correct. The encoding of the state is an enum set,
which uses one bit per type, thus the NUMBER_OF_TYPES (=3) is indeed the
number of bits, the enum set uses. The operator is either minus or not,
thus i use just one bit to encode it. This operater bit comes after the
state, thus it has an offset of NUMBER_OF_TYPES to the left.

I didn't use the BitField class to store the operator, since i only ever
use 2 tokens of all the possible 8...

https://codereview.chromium.org/17229005/diff/116001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/17229005/diff/116001/src/hydrogen-instructions.h#newcode4073
src/hydrogen-instructions.h:4073:
On 2013/06/27 13:31:06, danno wrote:
Remove this extraneous whitespace change

https://codereview.chromium.org/17229005/diff/116001/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/17229005/diff/116001/src/hydrogen.h#newcode1019
src/hydrogen.h:1019: static Representation ToRepresentation(Handle<Type>
type);
On 2013/06/27 13:31:06, danno wrote:
Shouldn't this method be on the Type class?
I only pulled this method up from the OptimizedGraphBuilder, and its
only used in this hierarchy.

https://codereview.chromium.org/17229005/diff/116001/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):

https://codereview.chromium.org/17229005/diff/116001/src/ia32/lithium-ia32.cc#newcode1611
src/ia32/lithium-ia32.cc:1611: ASSERT(instr->right()->IsConstant() &&
On 2013/06/27 13:31:06, danno wrote:
Shouldn't you use BetterRightOperand here and on the line below?

Actually not, since i know the stub passes the constant as right
operand. But i should definitely not use betterLeftOperand, since that
might be the constant...

https://codereview.chromium.org/17229005/

--
--
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.


Reply via email to