| Message ID | 4DA6E233.1010207@gjlay.de |
|---|---|
| State | New |
| Headers | show |
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.
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. >
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.
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; +}