| Message ID | 20101122220027.GR29412@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Nov 22, 2010 at 11:00 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > On the attached testcases jumpif* emits: > (jump_insn 8 7 9 (set (pc) > (label_ref 0)) pr46499-1.c:26 -1 > (nil)) > > (barrier 9 8 10) > > (jump_insn 10 9 11 (set (pc) > (label_ref 0)) pr46499-1.c:26 -1 > (nil)) > > (barrier 11 10 0) > where the label_ref in both cases is the non-fallthru label. This is > because ssa_name & ssa_name is the condition and both ssa_name's > are found using TER to be 0 (and the -fno-* options ensure it is not > optimized earlier). > maybe_cleanup_end_of_block is called to find out when the sequence ends > with an unconditional jump to the non-fallthru label, with no fallthru > code_label afterwards, which means all the sequence does (assuming no > side-effects in it but GIMPLE should ensure that) is that unconditional > jump and attempts to remove any other jumps from the sequence. > Apparently it was expecting just conditional jumps to the non-fallthru > label, for unconditional ones as in this example the following > barrier needs to be removed too, otherwise we end up with a barrier > in a middle of a bb and we either ICE on it, or miscompile it. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok. Thanks, Richard. > 2010-11-22 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/46499 > * cfgexpand.c (maybe_cleanup_end_of_block): Remove also BARRIERs > following unconditional jumps. > > * gcc.dg/pr46499-1.c: New test. > * gcc.dg/pr46499-2.c: New test. > > --- gcc/cfgexpand.c.jj 2010-11-10 13:14:53.000000000 +0100 > +++ gcc/cfgexpand.c 2010-11-22 14:45:49.000000000 +0100 > @@ -1694,7 +1694,14 @@ maybe_cleanup_end_of_block (edge e, rtx > { > insn = PREV_INSN (insn); > if (JUMP_P (NEXT_INSN (insn))) > - delete_insn (NEXT_INSN (insn)); > + { > + if (!any_condjump_p (insn)) > + { > + gcc_assert (BARRIER_P (NEXT_INSN (NEXT_INSN (insn)))); > + delete_insn (NEXT_INSN (NEXT_INSN (insn))); > + } > + delete_insn (NEXT_INSN (insn)); > + } > } > } > } > --- gcc/testsuite/gcc.dg/pr46499-1.c.jj 2010-11-22 14:59:48.000000000 +0100 > +++ gcc/testsuite/gcc.dg/pr46499-1.c 2010-11-22 14:59:11.000000000 +0100 > @@ -0,0 +1,31 @@ > +/* PR middle-end/46499 */ > +/* { dg-do run } */ > +/* { dg-options "-O -fno-omit-frame-pointer -fno-tree-ccp -fno-tree-dominator-opts -finline-small-functions" } */ > + > +extern void abort (void); > + > +int count = 0; > + > +int > +foo (void) > +{ > + count++; > + return 0; > +} > + > +int > +bar (void) > +{ > + count++; > + return 0; > +} > + > +int > +main () > +{ > + if ((foo () == 1) & (bar () == 1)) > + abort (); > + if (count != 2) > + abort (); > + return 0; > +} > --- gcc/testsuite/gcc.dg/pr46499-2.c.jj 2010-11-22 14:59:53.000000000 +0100 > +++ gcc/testsuite/gcc.dg/pr46499-2.c 2010-11-22 14:55:37.000000000 +0100 > @@ -0,0 +1,19 @@ > +/* PR middle-end/46499 */ > +/* { dg-do compile } */ > +/* { dg-options "-O -fno-tree-ccp -fno-tree-dominator-opts" } */ > + > +extern void abort (void); > + > +static inline int > +foo (void) > +{ > + return 0; > +} > + > +int > +main () > +{ > + if ((foo () == 1) & (foo () == 1)) > + abort (); > + return 0; > +} > > Jakub >
On Mon, Nov 22, 2010 at 2:00 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > On the attached testcases jumpif* emits: > (jump_insn 8 7 9 (set (pc) > (label_ref 0)) pr46499-1.c:26 -1 > (nil)) > > (barrier 9 8 10) > > (jump_insn 10 9 11 (set (pc) > (label_ref 0)) pr46499-1.c:26 -1 > (nil)) > > (barrier 11 10 0) > where the label_ref in both cases is the non-fallthru label. This is > because ssa_name & ssa_name is the condition and both ssa_name's > are found using TER to be 0 (and the -fno-* options ensure it is not > optimized earlier). > maybe_cleanup_end_of_block is called to find out when the sequence ends > with an unconditional jump to the non-fallthru label, with no fallthru > code_label afterwards, which means all the sequence does (assuming no > side-effects in it but GIMPLE should ensure that) is that unconditional > jump and attempts to remove any other jumps from the sequence. > Apparently it was expecting just conditional jumps to the non-fallthru > label, for unconditional ones as in this example the following > barrier needs to be removed too, otherwise we end up with a barrier > in a middle of a bb and we either ICE on it, or miscompile it. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2010-11-22 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/46499 > * cfgexpand.c (maybe_cleanup_end_of_block): Remove also BARRIERs > following unconditional jumps. > This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46629
On Tue, Nov 23, 2010 at 1:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Mon, Nov 22, 2010 at 2:00 PM, Jakub Jelinek <jakub@redhat.com> wrote: >> Hi! >> >> On the attached testcases jumpif* emits: >> (jump_insn 8 7 9 (set (pc) >> (label_ref 0)) pr46499-1.c:26 -1 >> (nil)) >> >> (barrier 9 8 10) >> >> (jump_insn 10 9 11 (set (pc) >> (label_ref 0)) pr46499-1.c:26 -1 >> (nil)) >> >> (barrier 11 10 0) >> where the label_ref in both cases is the non-fallthru label. This is >> because ssa_name & ssa_name is the condition and both ssa_name's >> are found using TER to be 0 (and the -fno-* options ensure it is not >> optimized earlier). >> maybe_cleanup_end_of_block is called to find out when the sequence ends >> with an unconditional jump to the non-fallthru label, with no fallthru >> code_label afterwards, which means all the sequence does (assuming no >> side-effects in it but GIMPLE should ensure that) is that unconditional >> jump and attempts to remove any other jumps from the sequence. >> Apparently it was expecting just conditional jumps to the non-fallthru >> label, for unconditional ones as in this example the following >> barrier needs to be removed too, otherwise we end up with a barrier >> in a middle of a bb and we either ICE on it, or miscompile it. >> >> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? >> >> 2010-11-22 Jakub Jelinek <jakub@redhat.com> >> >> PR middle-end/46499 >> * cfgexpand.c (maybe_cleanup_end_of_block): Remove also BARRIERs >> following unconditional jumps. >> > > This caused: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46629 > LTO seems to generate unconditional jumps without barrier. Is this an LTO bug?
On Tue, Nov 23, 2010 at 2:01 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > LTO seems to generate unconditional jumps without barrier. > Is this an LTO bug? Considering LTO does not work on the RTL level at all. I suspect you could get a C testcase that produces the same ICE. -- Pinski
On Mon, Nov 22, 2010 at 2:00 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > On the attached testcases jumpif* emits: > (jump_insn 8 7 9 (set (pc) > (label_ref 0)) pr46499-1.c:26 -1 > (nil)) > > (barrier 9 8 10) > > (jump_insn 10 9 11 (set (pc) > (label_ref 0)) pr46499-1.c:26 -1 > (nil)) > > (barrier 11 10 0) > where the label_ref in both cases is the non-fallthru label. This is > because ssa_name & ssa_name is the condition and both ssa_name's > are found using TER to be 0 (and the -fno-* options ensure it is not > optimized earlier). > maybe_cleanup_end_of_block is called to find out when the sequence ends > with an unconditional jump to the non-fallthru label, with no fallthru > code_label afterwards, which means all the sequence does (assuming no > side-effects in it but GIMPLE should ensure that) is that unconditional > jump and attempts to remove any other jumps from the sequence. > Apparently it was expecting just conditional jumps to the non-fallthru > label, for unconditional ones as in this example the following > barrier needs to be removed too, otherwise we end up with a barrier > in a middle of a bb and we either ICE on it, or miscompile it. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2010-11-22 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/46499 > * cfgexpand.c (maybe_cleanup_end_of_block): Remove also BARRIERs > following unconditional jumps. > > * gcc.dg/pr46499-1.c: New test. > * gcc.dg/pr46499-2.c: New test. > > --- gcc/cfgexpand.c.jj 2010-11-10 13:14:53.000000000 +0100 > +++ gcc/cfgexpand.c 2010-11-22 14:45:49.000000000 +0100 > @@ -1694,7 +1694,14 @@ maybe_cleanup_end_of_block (edge e, rtx > { > insn = PREV_INSN (insn); > if (JUMP_P (NEXT_INSN (insn))) > - delete_insn (NEXT_INSN (insn)); > + { > + if (!any_condjump_p (insn)) ^^^^^^ Shouldn't it be any_condjump_p (NEXT_INSN (insn)) > + { > + gcc_assert (BARRIER_P (NEXT_INSN (NEXT_INSN (insn)))); > + delete_insn (NEXT_INSN (NEXT_INSN (insn))); > + } > + delete_insn (NEXT_INSN (insn)); > + } > } > } > }
On Tue, Nov 23, 2010 at 04:51:35PM -0800, H.J. Lu wrote: > > --- gcc/cfgexpand.c.jj 2010-11-10 13:14:53.000000000 +0100 > > +++ gcc/cfgexpand.c 2010-11-22 14:45:49.000000000 +0100 > > @@ -1694,7 +1694,14 @@ maybe_cleanup_end_of_block (edge e, rtx > > { > > insn = PREV_INSN (insn); > > if (JUMP_P (NEXT_INSN (insn))) > > - delete_insn (NEXT_INSN (insn)); > > + { > > + if (!any_condjump_p (insn)) > ^^^^^^ Shouldn't it > be any_condjump_p (NEXT_INSN (insn)) Oops. No, but either if (!any_condjump_p (NEXT_INSN (insn))) or if (any_uncondjump_p (NEXT_INSN (insn))) I'll bootstrap/regtest the former now. Jakub
On Tue, Nov 23, 2010 at 5:00 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Tue, Nov 23, 2010 at 04:51:35PM -0800, H.J. Lu wrote: >> > --- gcc/cfgexpand.c.jj 2010-11-10 13:14:53.000000000 +0100 >> > +++ gcc/cfgexpand.c 2010-11-22 14:45:49.000000000 +0100 >> > @@ -1694,7 +1694,14 @@ maybe_cleanup_end_of_block (edge e, rtx >> > { >> > insn = PREV_INSN (insn); >> > if (JUMP_P (NEXT_INSN (insn))) >> > - delete_insn (NEXT_INSN (insn)); >> > + { >> > + if (!any_condjump_p (insn)) >> ^^^^^^ Shouldn't it >> be any_condjump_p (NEXT_INSN (insn)) > > Oops. > No, but either if (!any_condjump_p (NEXT_INSN (insn))) > or if (any_uncondjump_p (NEXT_INSN (insn))) > > I'll bootstrap/regtest the former now. Please add the testcase in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46629 Thanks.
--- gcc/cfgexpand.c.jj 2010-11-10 13:14:53.000000000 +0100 +++ gcc/cfgexpand.c 2010-11-22 14:45:49.000000000 +0100 @@ -1694,7 +1694,14 @@ maybe_cleanup_end_of_block (edge e, rtx { insn = PREV_INSN (insn); if (JUMP_P (NEXT_INSN (insn))) - delete_insn (NEXT_INSN (insn)); + { + if (!any_condjump_p (insn)) + { + gcc_assert (BARRIER_P (NEXT_INSN (NEXT_INSN (insn)))); + delete_insn (NEXT_INSN (NEXT_INSN (insn))); + } + delete_insn (NEXT_INSN (insn)); + } } } } --- gcc/testsuite/gcc.dg/pr46499-1.c.jj 2010-11-22 14:59:48.000000000 +0100 +++ gcc/testsuite/gcc.dg/pr46499-1.c 2010-11-22 14:59:11.000000000 +0100 @@ -0,0 +1,31 @@ +/* PR middle-end/46499 */ +/* { dg-do run } */ +/* { dg-options "-O -fno-omit-frame-pointer -fno-tree-ccp -fno-tree-dominator-opts -finline-small-functions" } */ + +extern void abort (void); + +int count = 0; + +int +foo (void) +{ + count++; + return 0; +} + +int +bar (void) +{ + count++; + return 0; +} + +int +main () +{ + if ((foo () == 1) & (bar () == 1)) + abort (); + if (count != 2) + abort (); + return 0; +} --- gcc/testsuite/gcc.dg/pr46499-2.c.jj 2010-11-22 14:59:53.000000000 +0100 +++ gcc/testsuite/gcc.dg/pr46499-2.c 2010-11-22 14:55:37.000000000 +0100 @@ -0,0 +1,19 @@ +/* PR middle-end/46499 */ +/* { dg-do compile } */ +/* { dg-options "-O -fno-tree-ccp -fno-tree-dominator-opts" } */ + +extern void abort (void); + +static inline int +foo (void) +{ + return 0; +} + +int +main () +{ + if ((foo () == 1) & (foo () == 1)) + abort (); + return 0; +}
Hi! On the attached testcases jumpif* emits: (jump_insn 8 7 9 (set (pc) (label_ref 0)) pr46499-1.c:26 -1 (nil)) (barrier 9 8 10) (jump_insn 10 9 11 (set (pc) (label_ref 0)) pr46499-1.c:26 -1 (nil)) (barrier 11 10 0) where the label_ref in both cases is the non-fallthru label. This is because ssa_name & ssa_name is the condition and both ssa_name's are found using TER to be 0 (and the -fno-* options ensure it is not optimized earlier). maybe_cleanup_end_of_block is called to find out when the sequence ends with an unconditional jump to the non-fallthru label, with no fallthru code_label afterwards, which means all the sequence does (assuming no side-effects in it but GIMPLE should ensure that) is that unconditional jump and attempts to remove any other jumps from the sequence. Apparently it was expecting just conditional jumps to the non-fallthru label, for unconditional ones as in this example the following barrier needs to be removed too, otherwise we end up with a barrier in a middle of a bb and we either ICE on it, or miscompile it. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2010-11-22 Jakub Jelinek <jakub@redhat.com> PR middle-end/46499 * cfgexpand.c (maybe_cleanup_end_of_block): Remove also BARRIERs following unconditional jumps. * gcc.dg/pr46499-1.c: New test. * gcc.dg/pr46499-2.c: New test. Jakub