ARM: Implement Math.abs in lithium code generator for the integer and

src/arm/lithium-codegen-arm.cc

Index: src/arm/lithium-codegen-arm.cc
diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index 08f58cb57e61be198f911da122b2328098ff48d4..89fb2369e99815d237edbd2c9064b55b8dd07cf4 100644
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -2339,12 +2339,111 @@ void LCodeGen::DoCallConstantFunction(LCallConstantFunction* instr) {
 
 
 void LCodeGen::DoDeferredMathAbsTaggedHeapNumber(LUnaryMathOperation* instr) {
-  Abort("DoDeferredMathAbsTaggedHeapNumber unimplemented.");
+  Register input = ToRegister(instr->input());
+  Register scratch = scratch0();
+
+  // Deoptimize if not a heap number.
+  __ ldr(scratch, FieldMemOperand(input, HeapObject::kMapOffset));
+  __ LoadRoot(ip, Heap::kHeapNumberMapRootIndex);
+  __ cmp(scratch, Operand(ip));
+  DeoptimizeIf(ne, instr->environment());
+
+  Label done;
+  Register tmp = input.is(r0) ? r1 : r0;
+  Register tmp2 = r2;
+  Register tmp3 = r3;
+
+  // Preserve the value of all registers.
+  __ PushSafepointRegisters();
+
+  Label negative;
+  __ ldr(tmp, FieldMemOperand(input, HeapNumber::kExponentOffset));
+  // Check the sign of the argument. If the argument is positive,
+  // just return it.
+  __ tst(tmp, Operand(HeapNumber::kSignMask));
+  __ b(ne, &negative);
+  __ mov(tmp, Operand(input));

[Alexandre, 2011/01/19 16:34:45]

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.

[Mads Ager (chromium), 2011/01/20 07:32:17]

Good catch. And that applies on ia32 as well. Change done for both platforms.

+  __ jmp(&done);
+
+  __ bind(&negative);
+
+  Label allocated, slow;
+  __ LoadRoot(scratch, Heap::kHeapNumberMapRootIndex);

[Alexandre, 2011/01/19 16:34:45]

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.

[Mads Ager (chromium), 2011/01/20 07:32:17]

For this one it is not so clear because the heap number map is loaded (into ip)
*before* we push all registers.

+  __ AllocateHeapNumber(tmp, tmp2, tmp3, scratch, &slow);
+  __ b(&allocated);
+
+  // Slow case: Call the runtime system to do the number allocation.
+  __ bind(&slow);
+
+  __ CallRuntimeSaveDoubles(Runtime::kAllocateHeapNumber);
+  RecordSafepointWithRegisters(
+      instr->pointer_map(), 0, Safepoint::kNoDeoptimizationIndex);
+  // Set the pointer to the new heap number in tmp.
+  if (!tmp.is(r0)) __ mov(tmp, Operand(r0));
+
+  // Restore input_reg after call to runtime.
+  __ ldr(input, MemOperand(sp, SpIndexForPushAll(input) * kPointerSize));
+
+  __ bind(&allocated);
+  __ ldr(tmp2, FieldMemOperand(input, HeapNumber::kExponentOffset));

[Alexandre, 2011/01/19 16:34:45]

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.

[Mads Ager (chromium), 2011/01/20 07:32:17]

I'd rather not do that because of the runtime call in between.

+  __ and_(tmp2, tmp2, Operand(~HeapNumber::kSignMask));

[Alexandre, 2011/01/19 16:34:45]

The bic (Bit Clear) instruction would be clearer.

[Mads Ager (chromium), 2011/01/20 07:32:17]

Done.

+  __ str(tmp2, FieldMemOperand(tmp, HeapNumber::kExponentOffset));
+  __ ldr(tmp2, FieldMemOperand(input, HeapNumber::kMantissaOffset));
+  __ str(tmp2, FieldMemOperand(tmp, HeapNumber::kMantissaOffset));
+
+  __ bind(&done);

[Alexandre, 2011/01/19 16:34:45]

Can be moved after the next store, as input and output are the same register.

[Mads Ager (chromium), 2011/01/20 07:32:17]

Done.

+
+  __ str(tmp, MemOperand(sp, SpIndexForPushAll(input) * kPointerSize));

[Alexandre, 2011/01/19 16:34:45]

You can use 
MemOperand MacroAssembler::SafepointRegisterSlot(Register reg)
(introduced after you uploaded the patch)

[Mads Ager (chromium), 2011/01/20 07:32:17]

Thanks, I'll do that. I have changed kInstrSize to kPointerSize in the
implementation though. SafepointRegisterSlot should multiply by kPointerSize and
not kInstrSize.

+  __ PopSafepointRegisters();
+}
+
+
+void LCodeGen::EmitIntegerMathAbs(LUnaryMathOperation* instr) {
+  Label is_positive;
+  uint32_t kSignMask = 0x80000000u;
+  Register input = ToRegister(instr->input());
+  __ tst(input, Operand(kSignMask));
+  __ b(eq, &is_positive);
+  __ rsb(input, input, Operand(0), SetCC);

[Alexandre, 2011/01/19 16:34:45]

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

[Mads Ager (chromium), 2011/01/20 07:32:17]

I find that a little too indirect for my liking. Do you think it will be much
faster? I wouldn't think so and I would like to keep the code clearer by having
it explicit.

+  // Deoptimize on overflow.
+  DeoptimizeIf(vs, instr->environment());
+  __ bind(&is_positive);
 }
 
 
 void LCodeGen::DoMathAbs(LUnaryMathOperation* instr) {
-  Abort("DoMathAbs unimplemented.");
+  // Class for deferred case.
+  class DeferredMathAbsTaggedHeapNumber: public LDeferredCode {
+   public:
+    DeferredMathAbsTaggedHeapNumber(LCodeGen* codegen,
+                                    LUnaryMathOperation* instr)
+        : LDeferredCode(codegen), instr_(instr) { }
+    virtual void Generate() {
+      codegen()->DoDeferredMathAbsTaggedHeapNumber(instr_);
+    }
+   private:
+    LUnaryMathOperation* instr_;
+  };
+
+  ASSERT(instr->input()->Equals(instr->result()));
+  Representation r = instr->hydrogen()->value()->representation();
+  if (r.IsDouble()) {
+    DwVfpRegister input = ToDoubleRegister(instr->input());
+    // __ vabs(input, input);
+    Abort("Double DoMathAbs unimplemented");
+  } else if (r.IsInteger32()) {
+    EmitIntegerMathAbs(instr);
+  } else {
+    // Representation is tagged.
+    DeferredMathAbsTaggedHeapNumber* deferred =
+        new DeferredMathAbsTaggedHeapNumber(this, instr);
+    Register input = ToRegister(instr->input());
+    // Smi check.
+    __ BranchOnNotSmi(input, deferred->entry());
+    // If smi, handle it directly.
+    EmitIntegerMathAbs(instr);
+    __ bind(deferred->exit());
+  }
 }