ARM: Add tests for CopyBytes.

src/arm/macro-assembler-arm.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 3168 matching lines @@
 
   int remain = field_count % (DwVfpRegister::kSizeInBytes / kPointerSize);
   if (remain != 0) {
     vldr(double_scratch.low(),
          FieldMemOperand(src, (field_count - 1) * kPointerSize));
     vstr(double_scratch.low(),
          FieldMemOperand(dst, (field_count - 1) * kPointerSize));
   }
 }
 
-
 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));
-  sub(length, length, Operand(1), SetCC);
-  b(ne, &byte_loop_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));
+  strb(scratch, MemOperand(dst, 1, PostIndex));
 
   // Copy bytes in word size chunks.
   bind(&word_loop);
   if (emit_debug_code()) {
     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));
     mov(scratch, Operand(scratch, LSR, 8));
     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) {
   Label loop, entry;
   b(&entry);
   bind(&loop);
   str(filler, MemOperand(start_offset, kPointerSize, PostIndex));
   bind(&entry);
   cmp(start_offset, end_offset);
   b(lt, &loop);
@@ skipping 625 matching lines @@
 void CodePatcher::EmitCondition(Condition cond) {
   Instr instr = Assembler::instr_at(masm_.pc_);
   instr = (instr & ~kCondMask) | cond;
   masm_.emit(instr);
 }
 
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_ARM