On Fri, 9 Jan 2015, Richard Biener wrote:
> On Fri, 9 Jan 2015, Jakub Jelinek wrote:
>
> > Hi!
> >
> > The following testcase is miscompiled on s390x. The problem is that there
> > is massive cross-jumping going on, and after that post_order_compute
> > decides to call tidy_fallthru_edges, including on an edge from a bb ending
> > with a table jump to a bb with now a single successor where all the
> > jump_table_data entries point to. rtl_tidy_fallthru_edge happily removes
> > the tablejump and anything in between the current bb and next bb (i.e.
> > jump_table_data + its code_label + barrier if any), but doesn't care about
> > any possible uses of the code_label (on the testcase e.g. the label
> > reference is hoisted before the loop).
> > Now, if I try some artificial testcase like:
> > int a;
> > #define A(n) case n: a++; break;
> > #define B(n) A(n##0) A(n##1) A(n##2) A(n##3) A(n##4) A(n##5) A(n##6)
> > A(n##7) A(n##8) A(n##9)
> > #define C(n) B(n##0) B(n##1) B(n##2) B(n##3) B(n##4) B(n##5) B(n##6)
> > B(n##7) B(n##8) B(n##9)
> >
> > void
> > foo (int x)
> > {
> > switch (x)
> > {
> > C(1)
> > }
> > }
> > say on x86_64, tidy_fallthru_edges isn't called at all (it would be only if
> > there are unrelated unreachable blocks in the function at the same time),
> > so I think spending time on trying to handle tablejump_p right in
> > rtl_tidy_fallthru_edge is wasteful, my preference (for both 4.9 and trunk)
> > would be as the patch below just not handle tablejump_p in that function,
> > and for trunk perhaps try to handle it elsewhere where it will be optimized
> > even for the above testcase (somewhere in cfgcleanup?).
>
> I wonder why post_order_compute calls tidy_fallthru_edges at all - won't
> that break the just computed postorder?
>
> Other than that, why doesn't can't the issue show up with non-table-jumps?
>
> What does it take to preserve (all) the labels?
That is, just remove q, not everything between q and BB_HEAD (c)?
> Richard.
>
> > Also, eventually it would be really nice if tree-ssa-tail-merge.c could
> > handle this already at the GIMPLE level.
> >
> > Thoughts on this?
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux on the trunk and on
> > {x86_64,i686,ppc64,ppc64le,s390,s390x,armv7hl}-linux on 4.9 branch.
> >
> > 2015-01-09 Jakub Jelinek <ja...@redhat.com>
> >
> > PR rtl-optimization/64536
> > * cfgrtl.c (rtl_tidy_fallthru_edge): Don't remove tablejumps.
> >
> > * gcc.dg/pr64536.c: New test.
> >
> > --- gcc/cfgrtl.c.jj 2015-01-05 13:07:12.000000000 +0100
> > +++ gcc/cfgrtl.c 2015-01-08 17:03:18.511218340 +0100
> > @@ -1782,10 +1782,14 @@ rtl_tidy_fallthru_edge (edge e)
> > if (INSN_P (q))
> > return;
> >
> > + q = BB_END (b);
> > + /* Don't remove table jumps here. */
> > + if (tablejump_p (q, NULL, NULL))
> > + return;
> > +
> > /* Remove what will soon cease being the jump insn from the source block.
> > If block B consisted only of this single jump, turn it into a deleted
> > note. */
> > - q = BB_END (b);
> > if (JUMP_P (q)
> > && onlyjump_p (q)
> > && (any_uncondjump_p (q)
> > --- gcc/testsuite/gcc.dg/pr64536.c.jj 2015-01-08 17:13:32.218929003
> > +0100
> > +++ gcc/testsuite/gcc.dg/pr64536.c 2015-01-08 17:28:56.758428958 +0100
> > @@ -0,0 +1,67 @@
> > +/* PR rtl-optimization/64536 */
> > +/* { dg-do link } */
> > +/* { dg-options "-O2" } */
> > +/* { dg-additional-options "-fPIC" { target fpic } } */
> > +
> > +struct S { long q; } *h;
> > +long a, b, g, j, k, *c, *d, *e, *f, *i;
> > +long *baz (void)
> > +{
> > + asm volatile ("" : : : "memory");
> > + return e;
> > +}
> > +
> > +void
> > +bar (int x)
> > +{
> > + int y;
> > + for (y = 0; y < x; y++)
> > + {
> > + switch (b)
> > + {
> > + case 0:
> > + case 2:
> > + a++;
> > + break;
> > + case 3:
> > + a++;
> > + break;
> > + case 1:
> > + a++;
> > + }
> > + if (d)
> > + {
> > + f = baz ();
> > + g = k++;
> > + if (&h->q)
> > + {
> > + j = *f;
> > + h->q = *f;
> > + }
> > + else
> > + i = (long *) (h->q = *f);
> > + *c++ = (long) f;
> > + e += 6;
> > + }
> > + else
> > + {
> > + f = baz ();
> > + g = k++;
> > + if (&h->q)
> > + {
> > + j = *f;
> > + h->q = *f;
> > + }
> > + else
> > + i = (long *) (h->q = *f);
> > + *c++ = (long) f;
> > + e += 6;
> > + }
> > + }
> > +}
> > +
> > +int
> > +main ()
> > +{
> > + return 0;
> > +}
> >
> > Jakub
> >
> >
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)
