http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-arm.h
File src/arm/lithium-arm.h (right):
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-arm.h#newcode1146
src/arm/lithium-arm.h:1146: Handle<Map> map_;
On 2011/01/04 09:17:19, Kevin Millikin wrote:
FYI you shouldn't need any of the fields map_, true_block_id_, or
false_block_id_. You can keep the accessors and just delegate them to
the
corresponding hydrogen instruction:
Handle<Map> map() const { return hydrogen()->map(); }
int true_block_id() const { return
hydrogen()->FirstSuccessor()->block_id(); }
Though HCompareMapAndBranch::FirstSuccessor is virtual so you may
prefer
duplicating the block ID in the Lithium instruction or else exposing a
non-virtual accessor on HCompareMapAndBranch.
You will need the macro invocation
DECLARE_HYDROGEN_ACCESSOR(CompareMapAndBranch)
in this class. I realize this is just ported from the ia32 code, but
you could
also feel free to clean it up at the same time you do the porting---a
hidden
benefit of writing the port.
Done.
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-codegen-arm.cc
File src/arm/lithium-codegen-arm.cc (right):
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-codegen-arm.cc#newcode1222
src/arm/lithium-codegen-arm.cc:1222: __ cmp(reg,
Operand(Factory::null_value()));
On 2011/01/04 11:23:34, Søren Gjesse wrote:
ou should use the root array for reading these fixed objects.
__ LoadRoot(ip, Heap::kNullValueRootIndex);
__ cmp(reg, ip);
Done.
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-codegen-arm.cc#newcode1223
src/arm/lithium-codegen-arm.cc:1223: if (instr->is_strict()) {
On 2011/01/04 11:23:34, Søren Gjesse wrote:
Two conditional LoadRoot should work here.
Done.
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-codegen-arm.cc#newcode1224
src/arm/lithium-codegen-arm.cc:1224: __ mov(result,
Operand(Handle<Object>(Heap::true_value())));
On 2011/01/04 11:23:34, Søren Gjesse wrote:
Use root array.
Done.
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-codegen-arm.cc#newcode1227
src/arm/lithium-codegen-arm.cc:1227: __ mov(result,
Operand(Handle<Object>(Heap::false_value())));
On 2011/01/04 11:23:34, Søren Gjesse wrote:
Use root array.
Done.
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-codegen-arm.cc#newcode1232
src/arm/lithium-codegen-arm.cc:1232: __ cmp(reg,
Operand(Factory::undefined_value()));
On 2011/01/04 11:23:34, Søren Gjesse wrote:
Use root array.
Done.
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-codegen-arm.cc#newcode1239
src/arm/lithium-codegen-arm.cc:1239: __ ldr(scratch,
FieldMemOperand(reg, HeapObject::kMapOffset));
On 2011/01/04 11:23:34, Søren Gjesse wrote:
Code in comment.
Done.
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-codegen-arm.cc#newcode1245
src/arm/lithium-codegen-arm.cc:1245: __ mov(result,
Operand(Handle<Object>(Heap::false_value())));
On 2011/01/04 11:23:34, Søren Gjesse wrote:
Use root array.
Done.
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-codegen-arm.cc#newcode1248
src/arm/lithium-codegen-arm.cc:1248: __ mov(result,
Operand(Handle<Object>(Heap::true_value())));
On 2011/01/04 11:23:34, Søren Gjesse wrote:
Use root array.
Done.
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-codegen-arm.cc#newcode1794
src/arm/lithium-codegen-arm.cc:1794: __ cmp(ToRegister(instr->index()),
ToOperand(instr->length()));
On 2011/01/04 11:23:34, Søren Gjesse wrote:
To much indention.
Done.
http://codereview.chromium.org/6069010/diff/4001/src/arm/lithium-codegen-arm.cc#newcode2149
src/arm/lithium-codegen-arm.cc:2149: void
LCodeGen::DoCheckPrototypeMaps(LCheckPrototypeMaps* instr) {
On 2011/01/04 11:23:34, Søren Gjesse wrote:
reg -> temp1, temp -> temp2?
Done.
http://codereview.chromium.org/6069010/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
