Here are a few comments. I'll add full support for vabs when this lands.
Regards, Alexandre http://codereview.chromium.org/6347007/diff/1/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): http://codereview.chromium.org/6347007/diff/1/src/arm/lithium-codegen-arm.cc#newcode2365 src/arm/lithium-codegen-arm.cc:2365: __ mov(tmp, Operand(input)); In LCodeGen::DoMathAbs we know that input and result use the same register: 2428 ASSERT(instr->input()->Equals(instr->result())); There is no need to overwrite it in the safepoint slot in this case. http://codereview.chromium.org/6347007/diff/1/src/arm/lithium-codegen-arm.cc#newcode2371 src/arm/lithium-codegen-arm.cc:2371: __ LoadRoot(scratch, Heap::kHeapNumberMapRootIndex); Since we save all registers on the stack, we could use an additional scratch register to preserve the value HeapNumberMap loaded before and spare a load here. http://codereview.chromium.org/6347007/diff/1/src/arm/lithium-codegen-arm.cc#newcode2388 src/arm/lithium-codegen-arm.cc:2388: __ ldr(tmp2, FieldMemOperand(input, HeapNumber::kExponentOffset)); Same as before, we could use a scratch register to preserve this value from the earlier load. We may need to restore it after the callruntime though if it was clobbered. http://codereview.chromium.org/6347007/diff/1/src/arm/lithium-codegen-arm.cc#newcode2389 src/arm/lithium-codegen-arm.cc:2389: __ and_(tmp2, tmp2, Operand(~HeapNumber::kSignMask)); The bic (Bit Clear) instruction would be clearer. http://codereview.chromium.org/6347007/diff/1/src/arm/lithium-codegen-arm.cc#newcode2394 src/arm/lithium-codegen-arm.cc:2394: __ bind(&done); Can be moved after the next store, as input and output are the same register. http://codereview.chromium.org/6347007/diff/1/src/arm/lithium-codegen-arm.cc#newcode2396 src/arm/lithium-codegen-arm.cc:2396: __ str(tmp, MemOperand(sp, SpIndexForPushAll(input) * kPointerSize)); You can use MemOperand MacroAssembler::SafepointRegisterSlot(Register reg) (introduced after you uploaded the patch) http://codereview.chromium.org/6347007/diff/1/src/arm/lithium-codegen-arm.cc#newcode2407 src/arm/lithium-codegen-arm.cc:2407: __ rsb(input, input, Operand(0), SetCC); We could remove the is_positive label and the branch and use a conditional rsb(..., pl) instead. (cmp will clear the V flag so it is safe.) http://codereview.chromium.org/6347007/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
