Message ID | 4C87E254.7080209@redhat.com |
---|---|
State | New |
Headers | show |
On Wed, Sep 8, 2010 at 12:21 PM, Vladimir Makarov <vmakarov@redhat.com> wrote: > The following patch fixes PR40386. The problem in details is described on > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386. > > Is it ok to commit into trunk and gcc-4.4-branch? No 4.5? > 2010-09-08 Vladimir Makarov <vmakarov@redhat.com> > > PR middle-end/40386 > * ira.c (pseudo_for_reload_consideration_p): Don't use > flag_ira_share_spill_slots. > > 2010-09-08 Vladimir Makarov <vmakarov@redhat.com> > > PR middle-end/40386 > * gcc.c-torture/execute/{pr40386.c,pr40386.x}: New testcase. > > >
On 09/08/2010 03:26 PM, H.J. Lu wrote: > On Wed, Sep 8, 2010 at 12:21 PM, Vladimir Makarov<vmakarov@redhat.com> wrote: >> The following patch fixes PR40386. The problem in details is described on >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386. >> >> Is it ok to commit into trunk and gcc-4.4-branch? > No 4.5? > > Should be also 4.5. Thanks for the correction, H.J.
On 09/08/2010 09:21 PM, Vladimir Makarov wrote: > The following patch fixes PR40386. The problem in details is described > on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386. According to that, > The problem is in that pseudos (r121 in our case) spilled by IRA are > not added to live_throughout of reload chain. Is there a good reason to do this, or should we change that instead? Bernd
On 09/08/2010 04:20 PM, Bernd Schmidt wrote: > On 09/08/2010 09:21 PM, Vladimir Makarov wrote: >> The following patch fixes PR40386. The problem in details is described >> on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386. > According to that, > >> The problem is in that pseudos (r121 in our case) spilled by IRA are >> not added to live_throughout of reload chain. > Is there a good reason to do this, or should we change that instead? > We should add all pseudos (I guess it was my typo). The patch actually results in adding all pseudos (spilled or not) to live_throughout independently from value of flag_ira_share_spill_slots. For the old RA it was done only for non-spilled pseudos. IRA needs this because it can assign hard registers in reload to pseudos spilled by IRA which global.c never did.
On 09/08/2010 11:05 PM, Vladimir Makarov wrote: > On 09/08/2010 04:20 PM, Bernd Schmidt wrote: >> On 09/08/2010 09:21 PM, Vladimir Makarov wrote: >>> The following patch fixes PR40386. The problem in details is >>> described >>> on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386. >> According to that, >> >>> The problem is in that pseudos (r121 in our case) spilled by IRA are >>> not added to live_throughout of reload chain. >> Is there a good reason to do this, or should we change that instead? >> > We should add all pseudos (I guess it was my typo). The patch actually > results in adding all pseudos (spilled or not) to live_throughout > independently from value of flag_ira_share_spill_slots. For the old RA > it was done only for non-spilled pseudos. IRA needs this because it can > assign hard registers in reload to pseudos spilled by IRA which global.c > never did. Ok, that makes sense. Please install the patch. Bernd
Vladimir Makarov writes: > The following patch fixes PR40386. The problem in details is > described on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40386. > > Is it ok to commit into trunk and gcc-4.4-branch? > > 2010-09-08 Vladimir Makarov <vmakarov@redhat.com> > > PR middle-end/40386 > * ira.c (pseudo_for_reload_consideration_p): Don't use > flag_ira_share_spill_slots. > > 2010-09-08 Vladimir Makarov <vmakarov@redhat.com> > > PR middle-end/40386 > * gcc.c-torture/execute/{pr40386.c,pr40386.x}: New testcase. The version checked into gcc-4_4-branch has a broken test case which fails to compile due to redefinition errors. It looks like a patch to add the test case was applied twice, causing the files to contain duplicated text. Here's is what 4.4's pr40386.x looks like right now: > set additional_flags "-fno-ira-share-spill-slots" > return 0 > set additional_flags "-fno-ira-share-spill-slots" > return 0 pr40386.c has the same issue, which causes it to fail to compile. /Mikael
Index: ira.c =================================================================== --- ira.c (revision 164011) +++ ira.c (working copy) @@ -2742,8 +2742,7 @@ pseudo_for_reload_consideration_p (int r { /* Consider spilled pseudos too for IRA because they still have a chance to get hard-registers in the reload when IRA is used. */ - return (reg_renumber[regno] >= 0 - || (ira_conflicts_p && flag_ira_share_spill_slots)); + return (reg_renumber[regno] >= 0 || ira_conflicts_p); } /* Init LIVE_SUBREGS[ALLOCNUM] and LIVE_SUBREGS_USED[ALLOCNUM] using Index: testsuite/gcc.c-torture/execute/pr40386.c =================================================================== --- testsuite/gcc.c-torture/execute/pr40386.c (revision 0) +++ testsuite/gcc.c-torture/execute/pr40386.c (revision 0) @@ -0,0 +1,99 @@ +/* { dg-options "-fno-ira-share-spill-slots" } */ +#define CHAR_BIT 8 + +#define ROR(a,b) (((a) >> (b)) | ((a) << ((sizeof (a) * CHAR_BIT) - (b)))) +#define ROL(a,b) (((a) << (b)) | ((a) >> ((sizeof (a) * CHAR_BIT) - (b)))) + +#define CHAR_VALUE ((char)0xf234) +#define SHORT_VALUE ((short)0xf234) +#define INT_VALUE ((int)0xf234) +#define LONG_VALUE ((long)0xf2345678L) +#define LL_VALUE ((long long)0xf2345678abcdef0LL) + +#define SHIFT1 4 +#define SHIFT2 ((sizeof (long long) * CHAR_BIT) - SHIFT1) + +char c = CHAR_VALUE; +short s = SHORT_VALUE; +int i = INT_VALUE; +long l = LONG_VALUE; +long long ll = LL_VALUE; +int shift1 = SHIFT1; +int shift2 = SHIFT2; + +main () +{ + if (ROR (c, shift1) != ROR (CHAR_VALUE, SHIFT1)) + abort (); + + if (ROR (c, SHIFT1) != ROR (CHAR_VALUE, SHIFT1)) + abort (); + + if (ROR (s, shift1) != ROR (SHORT_VALUE, SHIFT1)) + abort (); + + if (ROR (s, SHIFT1) != ROR (SHORT_VALUE, SHIFT1)) + abort (); + + if (ROR (i, shift1) != ROR (INT_VALUE, SHIFT1)) + abort (); + + if (ROR (i, SHIFT1) != ROR (INT_VALUE, SHIFT1)) + abort (); + + if (ROR (l, shift1) != ROR (LONG_VALUE, SHIFT1)) + abort (); + + if (ROR (l, SHIFT1) != ROR (LONG_VALUE, SHIFT1)) + abort (); + + if (ROR (ll, shift1) != ROR (LL_VALUE, SHIFT1)) + abort (); + + if (ROR (ll, SHIFT1) != ROR (LL_VALUE, SHIFT1)) + abort (); + + if (ROR (ll, shift2) != ROR (LL_VALUE, SHIFT2)) + abort (); + + if (ROR (ll, SHIFT2) != ROR (LL_VALUE, SHIFT2)) + abort (); + + if (ROL (c, shift1) != ROL (CHAR_VALUE, SHIFT1)) + abort (); + + if (ROL (c, SHIFT1) != ROL (CHAR_VALUE, SHIFT1)) + abort (); + + if (ROL (s, shift1) != ROL (SHORT_VALUE, SHIFT1)) + abort (); + + if (ROL (s, SHIFT1) != ROL (SHORT_VALUE, SHIFT1)) + abort (); + + if (ROL (i, shift1) != ROL (INT_VALUE, SHIFT1)) + abort (); + + if (ROL (i, SHIFT1) != ROL (INT_VALUE, SHIFT1)) + abort (); + + if (ROL (l, shift1) != ROL (LONG_VALUE, SHIFT1)) + abort (); + + if (ROL (l, SHIFT1) != ROL (LONG_VALUE, SHIFT1)) + abort (); + + if (ROL (ll, shift1) != ROL (LL_VALUE, SHIFT1)) + abort (); + + if (ROL (ll, SHIFT1) != ROL (LL_VALUE, SHIFT1)) + abort (); + + if (ROL (ll, shift2) != ROL (LL_VALUE, SHIFT2)) + abort (); + + if (ROL (ll, SHIFT2) != ROL (LL_VALUE, SHIFT2)) + abort (); + + exit (0); +} Index: testsuite/gcc.c-torture/execute/pr40386.x =================================================================== --- testsuite/gcc.c-torture/execute/pr40386.x (revision 0) +++ testsuite/gcc.c-torture/execute/pr40386.x (revision 0) @@ -0,0 +1,2 @@ +set additional_flags "-fno-ira-share-spill-slots" +return 0