This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] [ARM] Adjust test expectations of unaligned-memcpy-2/3.c (PR 91614)


On 03/09/2019 21:45, Bernd Edlinger wrote:
Hi,

due to the introduction of unaligned_loaddi and unaligned_storedi,
two test cases show some regression as PR 91614 points out.

I would like to change the test expectations if these two test cases,
since they seem to be bogus.

That is the test case already failed for arm_prefer_ldrd_strd targets,
since not a ldrd or strd is created but a ldm/stm instead, which
is probably not what is intended.

That is for arm_prefer_ldrd_strd the previously used instruction movdi:

(insn 11 10 12 2 (set (mem/c:DI (reg:SI 112) [0 MEM <charD.4[1:15]> [(voidD.71 *)&destD.5932]+0 S8 A32])
         (reg:DI 114)) "unaligned-memcpy-2.c":11:3 642 {*movdi_vfp}
      (nil))

is split up very early in subreg1 into:

(insn 21 10 22 2 (set (mem/c:SI (reg:SI 112) [0 MEM <charD.4[1:15]> [(voidD.71 *)&destD.5932]+0 S4 A32])
         (reg:SI 119)) "unaligned-memcpy-2.c":11:3 640 {*arm_movsi_vfp}
      (nil))
(insn 22 21 12 2 (set (mem/c:SI (plus:SI (reg:SI 112)
                 (const_int 4 [0x4])) [0 MEM <charD.4[1:15]> [(voidD.71 *)&destD.5932]+4 S4 A32])
         (reg:SI 120 [+4 ])) "unaligned-memcpy-2.c":11:3 640 {*arm_movsi_vfp}
      (nil))

which is finally replaced by:

(insn 35 10 12 2 (parallel [
             (set (mem/c:SI (reg/f:SI 3 r3 [111]) [0 MEM <charD.4[1:15]> [(voidD.71 *)&destD.5932]+0 S4 A32])
                 (reg:SI 1 r1))
             (set (mem/c:SI (plus:SI (reg/f:SI 3 r3 [111])
                         (const_int 4 [0x4])) [0 MEM <charD.4[1:15]> [(voidD.71 *)&destD.5932]+4 S4 A32])
                 (reg:SI 2 r2))
         ]) "unaligned-memcpy-2.c":11:3 378 {*stm2_}
      (expr_list:REG_DEAD (reg:SI 2 r2)
         (expr_list:REG_DEAD (reg:SI 1 r1)
             (nil))))

... so stm instead of strd.

Since the unalinged_storedi is an unspec, it is expanded as strd which is
kind of okay, but there is register pair spilled which was not there previously:

aligned_dest:
	@ args = 0, pretend = 0, frame = 0
	@ frame_needed = 0, uses_anonymous_args = 0
	@ link register save eliminated.
	strd	r4, [sp, #-8]!
	ldr	r4, [r0]	@ unaligned
	movw	r3, #:lower16:.LANCHOR0
	ldr	r5, [r0, #4]	@ unaligned
	movt	r3, #:upper16:.LANCHOR0
	strd	r4, [r3]
	ldr	r2, [r0, #8]	@ unaligned
	str	r2, [r3, #8]
	ldrh	r2, [r0, #12]	@ unaligned
	strh	r2, [r3, #12]	@ movhi
	ldrb	r2, [r0, #14]	@ zero_extendqisi2
	strb	r2, [r3, #14]
	ldrd	r4, [sp]
	add	sp, sp, #8
	bx	lr

The patch filters out ldrd/strd that sp-relative, and adds a few
missing scan-assembler rules, in order to make the test results more stable.

Alternative could be to use two movsi instead of the unaligned_load/storedi,
but that would end up in ldm/stm which looks plain wrong to me.


Tested using various arm-linux-gnueabihf cross-compilers.
Is it OK for trunk?


Thanks
Bernd.

2019-09-03  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR target/91614
	* gcc.target/arm/unaligned-memcpy-2.c: Adjust test expectations.
	* gcc.target/arm/unaligned-memcpy-3.c: Likewise.

Index: gcc/testsuite/gcc.target/arm/unaligned-memcpy-2.c
===================================================================
--- gcc/testsuite/gcc.target/arm/unaligned-memcpy-2.c	(Revision 275341)
+++ gcc/testsuite/gcc.target/arm/unaligned-memcpy-2.c	(Arbeitskopie)
@@ -15,9 +15,11 @@
    loads/stores for the remainder.  */

 /* { dg-final { scan-assembler-times "ldmia" 0 } } */
-/* { dg-final { scan-assembler-times "ldrd" 0 } } */
+/* { dg-final { scan-assembler-times "ldrd\(?!\[^\\n\]*sp\)" 0 } } */
+/* { dg-final { scan-assembler-times "ldm\(?!ia\)" 0 } } */
/* { dg-final { scan-assembler-times "stmia" 1 { target { ! { arm_prefer_ldrd_strd } } } } } */ -/* { dg-final { scan-assembler-times "strd" 1 { target { arm_prefer_ldrd_strd } } } } */ +/* { dg-final { scan-assembler-times "strd\(?!\[^\\n\]*sp\)" 1 { target { arm_prefer_ldrd_strd } } } } */
+/* { dg-final { scan-assembler-times "stm\(?!ia\)" 0 } } */
 /* { dg-final { scan-assembler-times "ldrh" 1 } } */
 /* { dg-final { scan-assembler-times "strh" 1 } } */
 /* { dg-final { scan-assembler-times "ldrb" 1 } } */
Index: gcc/testsuite/gcc.target/arm/unaligned-memcpy-3.c
===================================================================
--- gcc/testsuite/gcc.target/arm/unaligned-memcpy-3.c	(Revision 275341)
+++ gcc/testsuite/gcc.target/arm/unaligned-memcpy-3.c	(Arbeitskopie)
@@ -15,10 +15,12 @@
    loads/stores for the remainder.  */

/* { dg-final { scan-assembler-times "ldmia" 1 { target { ! { arm_prefer_ldrd_strd } } } } } */ -/* { dg-final { scan-assembler-times "ldrd" 1 { target { arm_prefer_ldrd_strd } } } } */
-/* { dg-final { scan-assembler-times "strd" 0 } } */
-/* { dg-final { scan-assembler-times "stm" 0 } } */
-/* { dg-final { scan-assembler-times "ldrh" 1 { target { ! { arm_prefer_ldrd_strd } } } } } */ +/* { dg-final { scan-assembler-times "ldrd\(?!\[^\\n\]*sp\)" 1 { target { arm_prefer_ldrd_strd } } } } */
+/* { dg-final { scan-assembler-times "ldm\(?!ia\)" 0 } } */
+/* { dg-final { scan-assembler-times "stmia" 0 } } */
+/* { dg-final { scan-assembler-times "strd\(?!\[^\\n\]*sp\)" 0 } } */
+/* { dg-final { scan-assembler-times "stm\(?!ia\)" 0 } } */
+/* { dg-final { scan-assembler-times "ldrh" 1 } } */
 /* { dg-final { scan-assembler-times "strh" 1 } } */
-/* { dg-final { scan-assembler-times "ldrb" 1 { target { ! { arm_prefer_ldrd_strd } } } } } */
+/* { dg-final { scan-assembler-times "ldrb" 1 } } */
 /* { dg-final { scan-assembler-times "strb" 1 } } */

Ug!  That's horrible!

Also, it's testing things we really shouldn't be concerned about. For example, the ldrh+ldrb+strh+strb is probably better done as ldr [offset-1] + str [offset-1] to handle the tail (ie re-copying the byte 4th from the end). When someone does that, this test will then needlessly fail.

I'm not really convinced that this sort of thing can be properly tested with scan-assembler tests, there's just too much that might change. Perhaps we should just accept this and create a simple executable test that will fault if run on a target that doesn't support the code we generate...

R.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]