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

2020-04-23 Thread Bernd Edlinger



On 4/22/20 8:20 PM, Jeff Law wrote:
> On Thu, 2020-03-26 at 04:23 +0100, Bernd Edlinger wrote:
>> Hi,
>>
>> I am pinging this because PR 91614 has been raised to P2 now:
>> https://gcc.gnu.org/legacy-ml/gcc-patches/2019-09/msg00370.html
> It's been nearly a month since the ping and many since the patch was posted
> originally.
> 
> I interpret Richard E's comments from Sept as a reject of the patch as-is.
> 
> I tend to agree with Richard in that testing assembly code for this is going 
> to
> be quite fragile and an execution test would be better.
> 

Yes, that is/was my understanding as well, I just wanted to make sure this was 
not
a mis-understanding or a message lost in transmission.
I think we can live with this test case as known fail.
Jeff, could you please make this tracker a P4 instead of P2 ?

FYI: I received no bounce, but any message can easily be dropped by spam 
filters...


Thanks,
Bernd.

> Jeff
> 


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

2020-04-22 Thread Jeff Law via Gcc-patches
On Thu, 2020-03-26 at 04:23 +0100, Bernd Edlinger wrote:
> Hi,
> 
> I am pinging this because PR 91614 has been raised to P2 now:
> https://gcc.gnu.org/legacy-ml/gcc-patches/2019-09/msg00370.html
It's been nearly a month since the ping and many since the patch was posted
originally.

I interpret Richard E's comments from Sept as a reject of the patch as-is.

I tend to agree with Richard in that testing assembly code for this is going to
be quite fragile and an execution test would be better.

Jeff



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

2020-03-25 Thread Bernd Edlinger
Hi,

I am pinging this because PR 91614 has been raised to P2 now:
https://gcc.gnu.org/legacy-ml/gcc-patches/2019-09/msg00370.html

Thanks
Bernd.


On 9/6/19 2:12 PM, Bernd Edlinger wrote:
> On 9/5/19 5:55 PM, Richard Earnshaw (lists) wrote:
>> 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  
>>> [(voidD.71 *)]+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  
>>> [(voidD.71 *)]+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  [(voidD.71 
>>> *)]+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  
>>> [(voidD.71 *)]+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  
>>> [(voidD.71 *)]+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  
>>
>> 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)
>> @@