| Message ID | 20131115151048.GL10643@virgil.suse |
|---|---|
| State | New |
| Headers | show |
*Ping* This is one of many bugs blocking bootstrap on ARM. It would be helpful if the fix could be reviewed and, if correct, for it to go in. http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01820.html Thanks, James On Fri, Nov 15, 2013 at 03:10:49PM +0000, Martin Jambor wrote: > Perfect, thanks a lot. The patch has also passed bootstrap and > testing on x86_64-linux (all languages and Ada) and passed bootstrap > on ppc64 (all languages) and ia64 (C, C++ and Fortran), I do not yet > have results from testsuite runs from the latter two platforms. Just > fro the record, I've also just started an i686 bootstrap. > > Vlad, the patch below is the exactly the same one + a testcase. It > basically does what you suggested in your first email, moves the > transformation to before all the analyses and undoes some code > movement from find_moveable_pseudos() to ira(). Is it OK for trunk? > > Thanks, > > Martin > > > 2013-11-15 Martin Jambor <mjambor@suse.cz> > > * ira.c (find_moveable_pseudos): Put back various analyses from ira() > here. > (ira): Move init_reg_equiv and call to > split_live_ranges_for_shrink_wrap up, remove analyses around call > to find_moveable_pseudos. > > testsuite/ > * gcc.target/i386/pr59099.c: New test. > > diff --git a/gcc/ira.c b/gcc/ira.c > index 2ef69cb..a171761 100644 > --- a/gcc/ira.c > +++ b/gcc/ira.c > @@ -4515,6 +4515,9 @@ find_moveable_pseudos (void) > pseudo_replaced_reg.release (); > pseudo_replaced_reg.safe_grow_cleared (max_regs); > > + df_analyze (); > + calculate_dominance_info (CDI_DOMINATORS); > + > i = 0; > bitmap_initialize (&live, 0); > bitmap_initialize (&used, 0); > @@ -4827,6 +4830,14 @@ find_moveable_pseudos (void) > free (bb_moveable_reg_sets); > > last_moveable_pseudo = max_reg_num (); > + > + fix_reg_equiv_init (); > + expand_reg_info (); > + regstat_free_n_sets_and_refs (); > + regstat_free_ri (); > + regstat_init_n_sets_and_refs (); > + regstat_compute_ri (); > + free_dominance_info (CDI_DOMINATORS); > } > > > @@ -5187,7 +5198,19 @@ ira (FILE *f) > #endif > df_analyze (); > > + init_reg_equiv (); > + if (ira_conflicts_p) > + { > + calculate_dominance_info (CDI_DOMINATORS); > + > + if (split_live_ranges_for_shrink_wrap ()) > + df_analyze (); > + > + free_dominance_info (CDI_DOMINATORS); > + } > + > df_clear_flags (DF_NO_INSN_RESCAN); > + > regstat_init_n_sets_and_refs (); > regstat_compute_ri (); > > @@ -5205,7 +5228,6 @@ ira (FILE *f) > if (resize_reg_info () && flag_ira_loop_pressure) > ira_set_pseudo_classes (true, ira_dump_file); > > - init_reg_equiv (); > rebuild_p = update_equiv_regs (); > setup_reg_equiv (); > setup_reg_equiv_init (); > @@ -5228,22 +5250,7 @@ ira (FILE *f) > allocation because of -O0 usage or because the function is too > big. */ > if (ira_conflicts_p) > - { > - df_analyze (); > - calculate_dominance_info (CDI_DOMINATORS); > - > - find_moveable_pseudos (); > - if (split_live_ranges_for_shrink_wrap ()) > - df_analyze (); > - > - fix_reg_equiv_init (); > - expand_reg_info (); > - regstat_free_n_sets_and_refs (); > - regstat_free_ri (); > - regstat_init_n_sets_and_refs (); > - regstat_compute_ri (); > - free_dominance_info (CDI_DOMINATORS); > - } > + find_moveable_pseudos (); > > max_regno_before_ira = max_reg_num (); > ira_setup_eliminable_regset (true); > diff --git a/gcc/testsuite/gcc.target/i386/pr59099.c b/gcc/testsuite/gcc.target/i386/pr59099.c > new file mode 100644 > index 0000000..7dc12ff > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr59099.c > @@ -0,0 +1,76 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2 -fPIC -m32" } */ > + > +void (*pfn)(void); > + > +struct s > +{ > + void** q; > + int h; > + int t; > + int s; > +}; > + > + > +void* f (struct s *, struct s *) __attribute__ ((noinline, regparm(1))); > + > +void* > +__attribute__ ((regparm(1))) > +f (struct s *p, struct s *p2) > +{ > + void *gp, *gp1; > + int t, h, s, t2, h2, c, i; > + > + if (p2->h == p2->t) > + return 0; > + > + (*pfn) (); > + > + h = p->h; > + t = p->t; > + s = p->s; > + > + h2 = p2->h; > + t2 = p2->t; > + > + gp = p2->q[h2++]; > + > + c = (t2 - h2) / 2; > + for (i = 0; i != c; i++) > + { > + if (t == h || (h == 0 && t == s - 1)) > + break; > + gp1 = p2->q[h2++]; > + p->q[t++] = gp1; > + if (t == s) > + t = 0; > + } > + > + p2->h = h2; > + return gp; > +} > + > +static void gn () { } > + > +int > +main() > +{ > + struct s s1, s2; > + void *q[10]; > + > + pfn = gn; > + > + s1.q = q; > + s1.h = 0; > + s1.t = 2; > + s1.s = 4; > + > + s2.q = q; > + s2.h = 0; > + s2.t = 4; > + s2.s = 2; > + > + f (&s1, &s2); > + > + return 0; > +} >
On 11/15/2013, 10:10 AM, Martin Jambor wrote: > Hi, > > On Fri, Nov 15, 2013 at 09:22:01AM +0000, Matthew Leach wrote: >> Martin Jambor <mjambor@suse.cz> writes: >> >>> Hi, >>> >>> On Thu, Nov 14, 2013 at 12:18:24PM +0000, Matthew Leach wrote: >>>> Martin Jambor <mjambor@suse.cz> writes: >>>> >>>>> Hi, >>>> Hi Martin, >>>> >>>> [...] >>>> >>>>> 2013-11-04 Martin Jambor <mjambor@suse.cz> >>>>> >>>>> PR rtl-optimization/10474 >>>>> * ira.c (interesting_dest_for_shprep): New function. >>>>> (split_live_ranges_for_shrink_wrap): Likewise. >>>>> (find_moveable_pseudos): Move calculation of dominance info, >>>>> df_analysios and the final anlyses to... >>>>> (ira): ...here, call split_live_ranges_for_shrink_wrap. >>>>> >>>>> testsuite/ >>>>> * gcc.dg/pr10474.c: New testcase. >>>>> * gcc.dg/ira-shrinkwrap-prep-1.c: Likewise. >>>>> * gcc.dg/ira-shrinkwrap-prep-2.c: Likewise. >>>> This patch seems to breaks stage-3 bootstrap for >>>> armv7l-unknown-linux-gnueabihf. genmddeps and genhooks seem to be in >>>> an infinite loop (I've left them running for approx 1h 30m now). >>>> >>>> My configure flags are: >>>> >>>> ../gcc/configure --with-cpu=cortex-a15 --with-tune=cortex-a15 --disable-nla --enable-shared --with-float=hard --with-fpu=neon-vfpv4 >>>> >>> I hope all the issues are just different manisfestations of PR 59099. >>> I hope to have fixed that by the patch below but I've just only >>> started bootstrapping it (and I'd like to do that on multiple >>> platforms before proposing it). But of course everybody can test if >>> it helps with their issues (and does not create new ones), I'd be >>> grateful if you do. >> I've put this patch through a bootstrap on an ARM chromebook and it >> seems to fix the problem I was seeing.. >> > Perfect, thanks a lot. The patch has also passed bootstrap and > testing on x86_64-linux (all languages and Ada) and passed bootstrap > on ppc64 (all languages) and ia64 (C, C++ and Fortran), I do not yet > have results from testsuite runs from the latter two platforms. Just > fro the record, I've also just started an i686 bootstrap. > > Vlad, the patch below is the exactly the same one + a testcase. It > basically does what you suggested in your first email, moves the > transformation to before all the analyses and undoes some code > movement from find_moveable_pseudos() to ira(). Is it OK for trunk? > It looks ok too me, Martin. I guess it is normal to have a few iterations here as RA(IRA/reload/LRA) is very complicated area even to me as there are too many details. I frequently ran in situation when fixing a bug for one target results in another target problems. Thanks. > 2013-11-15 Martin Jambor <mjambor@suse.cz> > > * ira.c (find_moveable_pseudos): Put back various analyses from ira() > here. > (ira): Move init_reg_equiv and call to > split_live_ranges_for_shrink_wrap up, remove analyses around call > to find_moveable_pseudos. > > testsuite/ > * gcc.target/i386/pr59099.c: New test. > >
diff --git a/gcc/ira.c b/gcc/ira.c index 2ef69cb..a171761 100644 --- a/gcc/ira.c +++ b/gcc/ira.c @@ -4515,6 +4515,9 @@ find_moveable_pseudos (void) pseudo_replaced_reg.release (); pseudo_replaced_reg.safe_grow_cleared (max_regs); + df_analyze (); + calculate_dominance_info (CDI_DOMINATORS); + i = 0; bitmap_initialize (&live, 0); bitmap_initialize (&used, 0); @@ -4827,6 +4830,14 @@ find_moveable_pseudos (void) free (bb_moveable_reg_sets); last_moveable_pseudo = max_reg_num (); + + fix_reg_equiv_init (); + expand_reg_info (); + regstat_free_n_sets_and_refs (); + regstat_free_ri (); + regstat_init_n_sets_and_refs (); + regstat_compute_ri (); + free_dominance_info (CDI_DOMINATORS); } @@ -5187,7 +5198,19 @@ ira (FILE *f) #endif df_analyze (); + init_reg_equiv (); + if (ira_conflicts_p) + { + calculate_dominance_info (CDI_DOMINATORS); + + if (split_live_ranges_for_shrink_wrap ()) + df_analyze (); + + free_dominance_info (CDI_DOMINATORS); + } + df_clear_flags (DF_NO_INSN_RESCAN); + regstat_init_n_sets_and_refs (); regstat_compute_ri (); @@ -5205,7 +5228,6 @@ ira (FILE *f) if (resize_reg_info () && flag_ira_loop_pressure) ira_set_pseudo_classes (true, ira_dump_file); - init_reg_equiv (); rebuild_p = update_equiv_regs (); setup_reg_equiv (); setup_reg_equiv_init (); @@ -5228,22 +5250,7 @@ ira (FILE *f) allocation because of -O0 usage or because the function is too big. */ if (ira_conflicts_p) - { - df_analyze (); - calculate_dominance_info (CDI_DOMINATORS); - - find_moveable_pseudos (); - if (split_live_ranges_for_shrink_wrap ()) - df_analyze (); - - fix_reg_equiv_init (); - expand_reg_info (); - regstat_free_n_sets_and_refs (); - regstat_free_ri (); - regstat_init_n_sets_and_refs (); - regstat_compute_ri (); - free_dominance_info (CDI_DOMINATORS); - } + find_moveable_pseudos (); max_regno_before_ira = max_reg_num (); ira_setup_eliminable_regset (true); diff --git a/gcc/testsuite/gcc.target/i386/pr59099.c b/gcc/testsuite/gcc.target/i386/pr59099.c new file mode 100644 index 0000000..7dc12ff --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr59099.c @@ -0,0 +1,76 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fPIC -m32" } */ + +void (*pfn)(void); + +struct s +{ + void** q; + int h; + int t; + int s; +}; + + +void* f (struct s *, struct s *) __attribute__ ((noinline, regparm(1))); + +void* +__attribute__ ((regparm(1))) +f (struct s *p, struct s *p2) +{ + void *gp, *gp1; + int t, h, s, t2, h2, c, i; + + if (p2->h == p2->t) + return 0; + + (*pfn) (); + + h = p->h; + t = p->t; + s = p->s; + + h2 = p2->h; + t2 = p2->t; + + gp = p2->q[h2++]; + + c = (t2 - h2) / 2; + for (i = 0; i != c; i++) + { + if (t == h || (h == 0 && t == s - 1)) + break; + gp1 = p2->q[h2++]; + p->q[t++] = gp1; + if (t == s) + t = 0; + } + + p2->h = h2; + return gp; +} + +static void gn () { } + +int +main() +{ + struct s s1, s2; + void *q[10]; + + pfn = gn; + + s1.q = q; + s1.h = 0; + s1.t = 2; + s1.s = 4; + + s2.q = q; + s2.h = 0; + s2.t = 4; + s2.s = 2; + + f (&s1, &s2); + + return 0; +}