Re: arm memcpy of aligned data

[Kyrill Tkachov <kyrylo.tkachov@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 3840 bytes --]


On 29/05/15 11:15, Kyrill Tkachov wrote:
> On 29/05/15 10:08, Kyrill Tkachov wrote:
>> Hi Mike,
>>
>> On 28/05/15 22:15, Mike Stump wrote:
>>> So, the arm memcpy code of aligned data isn’t as good as it can be.
>>>
>>> void *memcpy(void *dest, const void *src, unsigned int n);
>>>
>>> void foo(char *dst, int i) {
>>>      memcpy (dst, &i, sizeof (i));
>>> }
>>>
>>> generates horrible code, but, it we are willing to notice the src or the destination are aligned, we can do much better:
>>>
>>> $ ./cc1 -fschedule-fusion -fdump-tree-all-all -da -march=armv7ve -mcpu=cortex-m4 -fomit-frame-pointer -quiet -O2 /tmp/t.c -o t.s
>>> $ cat t.s
>>> [ … ]
>>> foo:
>>> 	@ args = 0, pretend = 0, frame = 4
>>> 	@ frame_needed = 0, uses_anonymous_args = 0
>>> 	@ link register save eliminated.
>>> 	sub	sp, sp, #4
>>> 	str	r1, [r0]	@ unaligned
>>> 	add	sp, sp, #4
>> I think there's something to do with cpu tuning here as well.
> That being said, I do think this is a good idea.
> I'll give it a test.

The patch passes bootstrap and testing ok and I've seen it
improve codegen in a few places in SPEC.
I've added a testcase all marked up.

Mike, I'll commit the attached patch in 24 hours unless somebody objects.

Thanks,
Kyrill

2015-06-15  Mike Stump  <mikestump@comcast.net>

     * config/arm/arm.c (arm_block_move_unaligned_straight):
     Emit normal move instead of unaligned load when source or destination
     are appropriately aligned.

2015-06-15 Mike Stump  <mikestump@comcast.net>
            Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * gcc.target/arm/memcpy-aligned-1.c: New test.

>
> Kyrill
>
>> For the code you've given compiled with -O2 -mcpu=cortex-a53 I get:
>>            sub     sp, sp, #8
>>            mov     r2, r0
>>            add     r3, sp, #8
>>            str     r1, [r3, #-4]!
>>            ldr     r0, [r3]        @ unaligned
>>            str     r0, [r2]        @ unaligned
>>            add     sp, sp, #8
>>            @ sp needed
>>            bx      lr
>>
>> whereas for -O2 -mcpu=cortex-a57 I get the much better:
>>            sub     sp, sp, #8
>>            str     r1, [r0]        @ unaligned
>>            add     sp, sp, #8
>>            @ sp needed
>>            bx      lr
>>
>> Kyrill
>>
>>
>>> Index: gcc/config/arm/arm.c
>>> ===================================================================
>>> --- gcc/config/arm/arm.c	(revision 223842)
>>> +++ gcc/config/arm/arm.c	(working copy)
>>> @@ -14376,7 +14376,10 @@ arm_block_move_unaligned_straight (rtx d
>>>     				srcoffset + j * UNITS_PER_WORD - src_autoinc);
>>>     	  mem = adjust_automodify_address (srcbase, SImode, addr,
>>>     					   srcoffset + j * UNITS_PER_WORD);
>>> -	  emit_insn (gen_unaligned_loadsi (regs[j], mem));
>>> +	  if (src_aligned)
>>> +	    emit_move_insn (regs[j], mem);
>>> +	  else
>>> +	    emit_insn (gen_unaligned_loadsi (regs[j], mem));
>>>     	}
>>>           srcoffset += words * UNITS_PER_WORD;
>>>         }
>>> @@ -14395,7 +14398,10 @@ arm_block_move_unaligned_straight (rtx d
>>>     				dstoffset + j * UNITS_PER_WORD - dst_autoinc);
>>>     	  mem = adjust_automodify_address (dstbase, SImode, addr,
>>>     					   dstoffset + j * UNITS_PER_WORD);
>>> -	  emit_insn (gen_unaligned_storesi (mem, regs[j]));
>>> +	  if (dst_aligned)
>>> +	    emit_move_insn (mem, regs[j]);
>>> +	  else
>>> +	    emit_insn (gen_unaligned_storesi (mem, regs[j]));
>>>     	}
>>>           dstoffset += words * UNITS_PER_WORD;
>>>         }
>>>
>>>
>>> Ok?
>>>
>>> Can someone spin this through an arm test suite run for me, I was doing this by inspection and cross compile on a system with no arm bits.  Bonus points if you can check it in with the test case above marked up as appropriate.
>>>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: arm-memcpy-aligned.patch --]
[-- Type: text/x-patch; name=arm-memcpy-aligned.patch, Size: 1855 bytes --]

commit 77191f4224c8729d014a9150bd9364f95ff704b0
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Fri May 29 10:44:21 2015 +0100

    [ARM] arm memcpy of aligned data

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 638d659..3a33c26 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -14283,7 +14283,10 @@ arm_block_move_unaligned_straight (rtx dstbase, rtx srcbase,
 				srcoffset + j * UNITS_PER_WORD - src_autoinc);
 	  mem = adjust_automodify_address (srcbase, SImode, addr,
 					   srcoffset + j * UNITS_PER_WORD);
-	  emit_insn (gen_unaligned_loadsi (regs[j], mem));
+	  if (src_aligned)
+	    emit_move_insn (regs[j], mem);
+	  else
+	    emit_insn (gen_unaligned_loadsi (regs[j], mem));
 	}
       srcoffset += words * UNITS_PER_WORD;
     }
@@ -14302,7 +14305,10 @@ arm_block_move_unaligned_straight (rtx dstbase, rtx srcbase,
 				dstoffset + j * UNITS_PER_WORD - dst_autoinc);
 	  mem = adjust_automodify_address (dstbase, SImode, addr,
 					   dstoffset + j * UNITS_PER_WORD);
-	  emit_insn (gen_unaligned_storesi (mem, regs[j]));
+	  if (dst_aligned)
+	    emit_move_insn (mem, regs[j]);
+	  else
+	    emit_insn (gen_unaligned_storesi (mem, regs[j]));
 	}
       dstoffset += words * UNITS_PER_WORD;
     }
diff --git a/gcc/testsuite/gcc.target/arm/memcpy-aligned-1.c b/gcc/testsuite/gcc.target/arm/memcpy-aligned-1.c
new file mode 100644
index 0000000..852b391
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/memcpy-aligned-1.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -save-temps" } */
+
+void *memcpy (void *dest, const void *src, unsigned int n);
+
+void foo (char *dst, int i)
+{
+  memcpy (dst, &i, sizeof (i));
+}
+
+/* { dg-final { scan-assembler-times "str\t" 1 } } */
+/* { dg-final { scan-assembler-not "ldr\t" } } */