| Message ID | 5165C84D.3090608@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 10, 2013 at 10:15 PM, Vladimir Makarov <vmakarov@redhat.com> wrote: > The following patch fixes > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56903 > > In this test case reload pass gets correct value HARD_REGNO_MODE_OK because > it can not create pseudos (can_create_pseudo) and this was actually used > that we are in reload (or after reload). LRA can create pseudos therefore > it got the wrong answer. The patch fixes it. Actually, the problem was with *lea_general_2 that allows QImode values in non-QImode registers. This is OK, since these registers will be split to paradoxical SImode registers and handled with lea insn. Apparently, LRA checks if registers can hold this mode through ix86_hard_regno_mode_ok, which fails in this particular case. BTW: there are many insn where QImode can live in non-QImode reg, e.g. the last alternative of *addqi_1, and similar insn patterns, that are split to lea after reload. The proposed patch is OK, although I'd propose to change it a bit with a more descriptive comment: if (!TARGET_PARTIAL_REG_STALL) return true; + /* LRA checks if the hard register is OK for the given mode. + QImode values can live in non-QI regs, so we + allow all registers here. */ + if (lra_in_progress) + return true; return !can_create_pseudo_p (); > 2013-04-10 Vladimir Makarov <vmakarov@redhat.com> > > PR tree-optimization/56903 This is target issue, so PR target/56903. > * config/i386/i386.c (ix86_hard_regno_mode_ok): Add > lra_in_progress for return. > > 2013-04-10 Vladimir Makarov <vmakarov@redhat.com> > > PR tree-optimization/56903 And here. > * gcc.target/i386/pr56903.c: New test. > > > OK for the trunk? > The patch is OK with these changes for mainline and 4.8 branch. Thanks, Uros.
On 13-04-11 12:51 PM, Uros Bizjak wrote: > On Wed, Apr 10, 2013 at 10:15 PM, Vladimir Makarov <vmakarov@redhat.com> wrote: >> The following patch fixes >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56903 >> >> In this test case reload pass gets correct value HARD_REGNO_MODE_OK because >> it can not create pseudos (can_create_pseudo) and this was actually used >> that we are in reload (or after reload). LRA can create pseudos therefore >> it got the wrong answer. The patch fixes it. > Actually, the problem was with *lea_general_2 that allows QImode > values in non-QImode registers. This is OK, since these registers will > be split to paradoxical SImode registers and handled with lea insn. > Apparently, LRA checks if registers can hold this mode through > ix86_hard_regno_mode_ok, which fails in this particular case. BTW: > there are many insn where QImode can live in non-QImode reg, e.g. the > last alternative of *addqi_1, and similar insn patterns, that are > split to lea after reload. > > The proposed patch is OK, although I'd propose to change it a bit with > a more descriptive comment: > > if (!TARGET_PARTIAL_REG_STALL) > return true; > + /* LRA checks if the hard register is OK for the given mode. > + QImode values can live in non-QI regs, so we > + allow all registers here. */ > + if (lra_in_progress) > + return true; > return !can_create_pseudo_p (); > > >> 2013-04-10 Vladimir Makarov <vmakarov@redhat.com> >> >> PR tree-optimization/56903 > This is target issue, so PR target/56903. > >> * config/i386/i386.c (ix86_hard_regno_mode_ok): Add >> lra_in_progress for return. >> >> 2013-04-10 Vladimir Makarov <vmakarov@redhat.com> >> >> PR tree-optimization/56903 > And here. > >> * gcc.target/i386/pr56903.c: New test. >> >> >> OK for the trunk? >> > The patch is OK with these changes for mainline and 4.8 branch. > > Thanks. Committed with your proposals as rev. 197927 (trunk) and 197928 (4.8 branch).
Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 197679) +++ config/i386/i386.c (working copy) @@ -33976,7 +33976,9 @@ ix86_hard_regno_mode_ok (int regno, enum return true; if (!TARGET_PARTIAL_REG_STALL) return true; - return !can_create_pseudo_p (); + /* LRA can create pseudos but it does not mean that it does not + need to know that the hard register in given mode is OK. */ + return lra_in_progress || !can_create_pseudo_p (); } /* We handle both integer and floats in the general purpose registers. */ else if (VALID_INT_MODE_P (mode)) Index: testsuite/gcc.target/i386/pr56903.c =================================================================== --- testsuite/gcc.target/i386/pr56903.c (revision 0) +++ testsuite/gcc.target/i386/pr56903.c (working copy) @@ -0,0 +1,18 @@ +/* PR rtl-optimization/56903 */ +/* { dg-do compile } */ +/* { dg-options "-Os" } */ +/* { dg-additional-options "-march=pentium3" { target ia32 } } */ + +int a, *b, c; +struct S { int s : 1; } *fn1 (void); +extern int fn3 (void), fn4 (int *); + +void +fn2 (void) +{ + int e = fn3 (); + char f = c + fn1 ()->s * 4; + if (*b && f == e) + a = *b; + fn4 (b); +}
The following patch fixes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56903 In this test case reload pass gets correct value HARD_REGNO_MODE_OK because it can not create pseudos (can_create_pseudo) and this was actually used that we are in reload (or after reload). LRA can create pseudos therefore it got the wrong answer. The patch fixes it. OK for the trunk? 2013-04-10 Vladimir Makarov <vmakarov@redhat.com> PR tree-optimization/56903 * config/i386/i386.c (ix86_hard_regno_mode_ok): Add lra_in_progress for return. 2013-04-10 Vladimir Makarov <vmakarov@redhat.com> PR tree-optimization/56903 * gcc.target/i386/pr56903.c: New test.