ARM: Add tests for CopyBytes.

src/arm/macro-assembler-arm.cc

Index: src/arm/macro-assembler-arm.cc
diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc
index 7628f23a5be9f137b24bc184bc7c2ba664f8ef5c..fddbe17180a892ab6ab090fd030643e5c2af2ca8 100644
--- a/src/arm/macro-assembler-arm.cc
+++ b/src/arm/macro-assembler-arm.cc
@@ -3186,24 +3186,32 @@ void MacroAssembler::CopyFields(Register dst,
   }
 }
 
-
 void MacroAssembler::CopyBytes(Register src,
                                Register dst,
                                Register length,
                                Register scratch) {
-  Label align_loop, align_loop_1, word_loop, byte_loop, byte_loop_1, done;
+  Label word_loop, byte_loop, byte_loop_1, done;
+
+  ASSERT(!AreAliased(src, dst, length, scratch));
+
+  cmp(length, Operand(8));
+  add(length, src, Operand(length));
+  b(lt, &byte_loop);
+  sub(ip, length, Operand(4));

[Benedikt Meurer, 2013/09/10 11:29:21]

Hm, this use of ip doesn't sit well with me. Using ip in the ARM backend is like
calling for trouble, because it's not always obvious to anyone whether it is
preserved across a bunch of operations or not (an inherent design flaw in the
ARM backend).

That being said, I don't like the new code. Looking at this from a highlevel
point of view, I think the main optimization opportunity is copying by word. But
although we optimize depending on src alignment, we never check to see whether
dst is actually word aligned.

I also don't like the overlap with the custom MemCopy function in
codegen-arm.cc.

 
   // Align src before copying in word size chunks.
-  bind(&align_loop);
-  cmp(length, Operand::Zero());
-  b(eq, &done);
-  bind(&align_loop_1);
   tst(src, Operand(kPointerSize - 1));
   b(eq, &word_loop);
   ldrb(scratch, MemOperand(src, 1, PostIndex));
+  tst(src, Operand(kPointerSize - 1));
+  strb(scratch, MemOperand(dst, 1, PostIndex));
+  b(eq, &word_loop);
+  ldrb(scratch, MemOperand(src, 1, PostIndex));
+  tst(src, Operand(kPointerSize - 1));
+  strb(scratch, MemOperand(dst, 1, PostIndex));
+  b(eq, &word_loop);
+  ldrb(scratch, MemOperand(src, 1, PostIndex));
   strb(scratch, MemOperand(dst, 1, PostIndex));
-  sub(length, length, Operand(1), SetCC);
-  b(ne, &byte_loop_1);
 
   // Copy bytes in word size chunks.
   bind(&word_loop);
@@ -3211,12 +3219,21 @@ void MacroAssembler::CopyBytes(Register src,
     tst(src, Operand(kPointerSize - 1));
     Assert(eq, kExpectingAlignmentForCopyBytes);
   }
-  cmp(length, Operand(kPointerSize));
-  b(lt, &byte_loop);
-  ldr(scratch, MemOperand(src, kPointerSize, PostIndex));
   if (CpuFeatures::IsSupported(UNALIGNED_ACCESSES)) {
+    ldr(scratch, MemOperand(src, kPointerSize, PostIndex));
+    cmp(src, Operand(ip));
     str(scratch, MemOperand(dst, kPointerSize, PostIndex));
+    b(le, &word_loop);
+    sub(ip, length, Operand(src));
+    mov(ip, Operand(ip, LSL, 31), SetCC);
+    ldrh(scratch, MemOperand(src, 2, PostIndex), cs);
+    strh(scratch, MemOperand(dst, 2, PostIndex), cs);
+    ldrb(scratch, MemOperand(src, 1, PostIndex), ne);
+    strb(scratch, MemOperand(dst, 1, PostIndex), ne);
+    b(&done);
   } else {
+    ldr(scratch, MemOperand(src, kPointerSize, PostIndex));
+    cmp(src, Operand(ip));
     strb(scratch, MemOperand(dst, 1, PostIndex));
     mov(scratch, Operand(scratch, LSR, 8));
     strb(scratch, MemOperand(dst, 1, PostIndex));
@@ -3224,23 +3241,21 @@ void MacroAssembler::CopyBytes(Register src,
     strb(scratch, MemOperand(dst, 1, PostIndex));
     mov(scratch, Operand(scratch, LSR, 8));
     strb(scratch, MemOperand(dst, 1, PostIndex));
+    b(lt, &word_loop);

[ulan, 2013/09/13 09:26:27]

Any reason why this is not "b(le, &word_loop)"? I think it should work and would
be consistent with the unaligned_accesses case.

   }
-  sub(length, length, Operand(kPointerSize));
-  b(&word_loop);
 
   // Copy the last bytes if any left.
   bind(&byte_loop);
-  cmp(length, Operand::Zero());
+  cmp(src, Operand(length));
   b(eq, &done);
   bind(&byte_loop_1);
   ldrb(scratch, MemOperand(src, 1, PostIndex));
+  cmp(src, Operand(length));
   strb(scratch, MemOperand(dst, 1, PostIndex));
-  sub(length, length, Operand(1), SetCC);
-  b(ne, &byte_loop_1);
+  b(lt, &byte_loop_1);
   bind(&done);
 }
 
-
 void MacroAssembler::InitializeFieldsWithFiller(Register start_offset,
                                                 Register end_offset,
                                                 Register filler) {