[testsuite,AVR] test for PR46779, PR45291, PR41894

[testsuite,AVR] test for PR46779, PR45291, PR41894

[Georg-Johann Lay]

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

This tests are intended to reveal the respective PRs because the test
case is more stable under slight variations in code (both of
application or compiler).

This test case migh also be helpful for older versions of avr-gcc, in
particular if PR41894 is actually fixed.

2011-04-14  Georg-Johann Lay  <avr@gjlay.de>

	PR target/46779
	PR target/45291
	PR target/41894
	* gcc.target/avr/pr46779-1.c: New test case
	* gcc.target/avr/pr46779-2.c: New test case

[-- Attachment #2: test-pr46779.diff --]
[-- Type: text/x-patch, Size: 2493 bytes --]

Index: gcc.target/avr/pr46779-1.c
===================================================================
--- gcc.target/avr/pr46779-1.c	(Revision 0)
+++ gcc.target/avr/pr46779-1.c	(Revision 0)
@@ -0,0 +1,51 @@
+/* { dg-do run } */
+/* { dg-options "-Os -fsplit-wide-types" } */
+
+/* This testcase should uncover bugl like
+   PR46779
+   PR45291
+   PR41894
+
+   The inline asm just serves to direct y into the Y register.
+   Otherwise, it is hard to write a "stable" test case that
+   also fails with sligh variations in source code, middle- resp.
+   backend.
+
+   The problem is that Y is also the frame-pointer, and
+   avr.c:avr_hard_regno_mode_ok disallows QI to get in Y-reg.
+   However, the y.a = 0 generates a
+       (set (subreg:QI (reg:HI pseudo)) ...)
+   where pseudo gets allocated to Y.
+
+   Reload fails to generate the right spill.
+*/
+
+#include <stdlib.h>
+
+struct S
+{
+    unsigned char a, b;
+} ab = {12, 34};
+
+void yoo (struct S y)
+{
+    __asm volatile ("ldi %B0, 56" : "+y" (y));
+    y.a = 0;
+    __asm volatile ("; y = %0" : "+y" (y));
+    ab = y;
+}
+
+int main ()
+{
+    yoo (ab);
+
+    if (ab.a != 0)
+        abort();
+
+    if (ab.b != 56)
+        abort();
+
+    exit (0);
+    
+    return 0;
+}
Index: gcc.target/avr/pr46779-2.c
===================================================================
--- gcc.target/avr/pr46779-2.c	(Revision 0)
+++ gcc.target/avr/pr46779-2.c	(Revision 0)
@@ -0,0 +1,51 @@
+/* { dg-do run } */
+/* { dg-options "-Os -fno-split-wide-types" } */
+
+/* This testcase should uncover bugl like
+   PR46779
+   PR45291
+   PR41894
+
+   The inline asm just serves to direct y into the Y register.
+   Otherwise, it is hard to write a "stable" test case that
+   also fails with sligh variations in source code, middle- resp.
+   backend.
+
+   The problem is that Y is also the frame-pointer, and
+   avr.c:avr_hard_regno_mode_ok disallows QI to get in Y-reg.
+   However, the y.a = 0 generates a
+       (set (subreg:QI (reg:HI pseudo)) ...)
+   where pseudo gets allocated to Y.
+
+   Reload fails to generate the right spill.
+*/
+
+#include <stdlib.h>
+
+struct S
+{
+    unsigned char a, b;
+} ab = {12, 34};
+
+void yoo (struct S y)
+{
+    __asm volatile ("ldi %B0, 56" : "+y" (y));
+    y.a = 0;
+    __asm volatile ("; y = %0" : "+y" (y));
+    ab = y;
+}
+
+int main ()
+{
+    yoo (ab);
+
+    if (ab.a != 0)
+        abort();
+
+    if (ab.b != 56)
+        abort();
+
+    exit (0);
+    
+    return 0;
+}

Re: [testsuite,AVR] test for PR46779, PR45291, PR41894

[Denis Chertykov]

2011/4/14 Georg-Johann Lay <avr@gjlay.de>:
> This tests are intended to reveal the respective PRs because the test
> case is more stable under slight variations in code (both of
> application or compiler).
>
> This test case migh also be helpful for older versions of avr-gcc, in
> particular if PR41894 is actually fixed.
>
> 2011-04-14  Georg-Johann Lay  <avr@gjlay.de>
>
>        PR target/46779
>        PR target/45291
>        PR target/41894
>        * gcc.target/avr/pr46779-1.c: New test case
>        * gcc.target/avr/pr46779-2.c: New test case
>

__asm volatile ("ldi %B0, 56" : "+y" (y));

It's wrong expression. "+y" must be "=y".
Is this ("+y") required for working test case ?

Denis.

Re: [testsuite,AVR] test for PR46779, PR45291, PR41894

[Georg-Johann Lay]

Denis Chertykov schrieb:
> 2011/4/14 Georg-Johann Lay <avr@gjlay.de>:
>> This tests are intended to reveal the respective PRs because the test
>> case is more stable under slight variations in code (both of
>> application or compiler).
>>
>> This test case migh also be helpful for older versions of avr-gcc, in
>> particular if PR41894 is actually fixed.
>>
>> 2011-04-14  Georg-Johann Lay  <avr@gjlay.de>
>>
>>        PR target/46779
>>        PR target/45291
>>        PR target/41894
>>        * gcc.target/avr/pr46779-1.c: New test case
>>        * gcc.target/avr/pr46779-2.c: New test case
>>
> 
> __asm volatile ("ldi %B0, 56" : "+y" (y));
> 
> It's wrong expression. "+y" must be "=y".

The "+y" is needed because y is both input and output.

> Is this ("+y") required for working test case ?

The bug only shows up when just one byte of Y is accessed via SUBREG.
The intent of "+y" is to direct register allocator to actually use Y.
Otherwise, this simple test cases won't make the bug explicit.

With "+r", "+l", "+x", "+z" or "+d" the bug won't show up because RA
don't chose Y for struct S.

Same is true for plain C code: it's hard to make the bug explicit.

The problem with this bug is that it is very hard to give a test case
and that slight variation in code (both application or compiler) give
the false impression that the bug is fixed while it is actually
persisting. Moreover, this test case is simpler and easier to analyze
than the test cases in the original PRs.

The drawback of the testcase is that no frame pointer must be there,
therefore this tests only is applicable if optimization is on; I chose
two variations of -Os.

Johann

> 
> Denis.
> 

Re: [testsuite,AVR] test for PR46779, PR45291, PR41894

[Denis Chertykov]

2011/4/14 Georg-Johann Lay <avr@gjlay.de>:
> Denis Chertykov schrieb:
>> 2011/4/14 Georg-Johann Lay <avr@gjlay.de>:
>>> This tests are intended to reveal the respective PRs because the test
>>> case is more stable under slight variations in code (both of
>>> application or compiler).
>>>
>>> This test case migh also be helpful for older versions of avr-gcc, in
>>> particular if PR41894 is actually fixed.
>>>
>>> 2011-04-14  Georg-Johann Lay  <avr@gjlay.de>
>>>
>>>        PR target/46779
>>>        PR target/45291
>>>        PR target/41894
>>>        * gcc.target/avr/pr46779-1.c: New test case
>>>        * gcc.target/avr/pr46779-2.c: New test case
>>>
>>
>> __asm volatile ("ldi %B0, 56" : "+y" (y));
>>
>> It's wrong expression. "+y" must be "=y".
>
> The "+y" is needed because y is both input and output.

Yes, you are right.
I missed that %B is only part of Y.

Approved.

Denis.