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/
