Thanks for the review! Alexandre
http://codereview.chromium.org/6366016/diff/1/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): http://codereview.chromium.org/6366016/diff/1/src/arm/lithium-codegen-arm.cc#newcode2536 src/arm/lithium-codegen-arm.cc:2536: Register tmp1 = input.is(r0) ? r1 : r0; I like that! Done. On 2011/01/27 08:27:56, Mads Ager wrote:
Kevin had a nicer suggestion for these. We intend to use r1,r2,r3,r4
for
tmp1,tmp2,tmp3,tmp4. Input can only be one of them and if it is we
will use r0
instead.
Register tmp1 = input.is(r1) ? r0 : r1; Register tmp2 = input.is(r2) ? r0 : r2; Register tmp3 = input.is(r3) ? r0 : r3;
http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc File src/arm/macro-assembler-arm.cc (right): http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc#newcode475 src/arm/macro-assembler-arm.cc:475: PushSafepointRegisters(); Reversed. We usually want to patch general purpose registers on the stack and don't touch double registers. I thought having them last on the stack would make it easier to patch them, as they would always (with or without double regs) lie just before sp. I thought there might be some issue, but it did not break the tests. I don't know if that would be easy to do, but it may be worth adding a test for this. On 2011/01/27 08:27:56, Mads Ager wrote:
What is the motivation for changing this? Please undo this change.
This will
break stack-traversal at safepoints because the traversal code expects
the
registers that can contain pointers to be pushed first.
http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc#newcode480 src/arm/macro-assembler-arm.cc:480: PopSafepointRegisters(); On 2011/01/27 08:27:56, Mads Ager wrote:
Undo please. :)
Done. http://codereview.chromium.org/6366016/diff/1/src/arm/macro-assembler-arm.cc#newcode518 src/arm/macro-assembler-arm.cc:518: return MemOperand(sp, SafepointRegisterStackIndex(reg.code()) * kInstrSize); On 2011/01/27 08:27:56, Mads Ager wrote:
kInstrSize -> kPointerSize. This is an index into the stack and not
the
instruction stream. Also, please undo your changes to where the
doubles are
pushed and take that into account here when computing the offset
instead. Done. http://codereview.chromium.org/6366016/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
