On Fri, 29 Jan 2021, Jakub Jelinek wrote:
> Hi!
>
> When expansion emits some control flow insns etc. inside of a former GIMPLE
> basic block, find_bb_boundaries needs to split it into multiple basic
> blocks.
> The code needs to ignore debug insns in decisions how many splits to do or
> where in between some non-debug insns the split should be done, but it can
> decide where to put debug insns if they can be kept and otherwise throws
> them away (they can't stay outside of basic blocks).
> On the following testcase, we end up in the bb from expander with
> control flow insn
> debug insns
> barrier
> some other insn
> (the some other insn is effectively dead after __builtin_unreachable and
> we'll optimize that out later).
> Without debug insns, we'd do the split when encountering some other insn
> and split after PREV_INSN (some other insn), i.e. after barrier (and the
> splitting code then moves the barrier in between basic blocks).
> But if there are debug insns, we actually split before the first debug insn
> that appeared after the control flow insn, so after control flow insn,
> and get a basic block that starts with debug insns and then has a barrier
> in the middle that nothing moves it out of the bb. This leads to ICEs and
> even if it wouldn't, different behavior from -g0.
> The reason for treating debug insns that way is a different case, e.g.
> control flow insn
> debug insns
> some other insn
> or even
> control flow insn
> barrier
> debug insns
> some other insn
> where splitting before the first such debug insn allows us to keep them
> while otherwise we would have to drop them on the floor, and in those
> situations we behave the same with -g0 and -g.
>
> So, the following patch fixes it by resetting debug_insn not just when
> splitting the blocks (it is set only after seeing a control flow insn and
> before splitting for it if needed), but also when seeing a barrier,
> which effectively means we always throw away debug insns after a control
> flow insn and before following barrier if any, but there is no way around
> that, control flow insn must be the last in the bb (BB_END) and BARRIER
> after it, debug insns aren't allowed outside of bb.
> We still handle the other cases fine (when there is no barrier or when
> debug insns appear only after the barrier).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2021-01-29 Jakub Jelinek <ja...@redhat.com>
>
> PR debug/98331
> * cfgbuild.c (find_bb_boundaries): Reset debug_insn when seeing
> a BARRIER.
>
> * gcc.dg/pr98331.c: New test.
>
> --- gcc/cfgbuild.c.jj 2021-01-04 10:25:39.000000000 +0100
> +++ gcc/cfgbuild.c 2021-01-28 14:18:24.061246540 +0100
> @@ -545,6 +545,7 @@ find_bb_boundaries (basic_block bb)
> if the barrier were preceded by a control_flow_insn_p insn. */
> if (!flow_transfer_insn)
> flow_transfer_insn = prev_nonnote_nondebug_insn_bb (insn);
> + debug_insn = NULL;
> }
>
> if (control_flow_insn_p (insn))
> --- gcc/testsuite/gcc.dg/pr98331.c.jj 2021-01-28 14:25:08.361706720 +0100
> +++ gcc/testsuite/gcc.dg/pr98331.c 2021-01-28 14:24:48.084934402 +0100
> @@ -0,0 +1,18 @@
> +/* PR debug/98331 */
> +/* { dg-do compile } */
> +/* { dg-options "-g -O2 -fcompare-debug" } */
> +/* { dg-additional-options "-march=x86-64" { target { i?86-*-* x86_64-*-* }
> } } */
> +
> +void bar (const char *);
> +unsigned long long x;
> +
> +void
> +foo (void)
> +{
> + int a = 1;
> + bar ("foo");
> + int b = 2;
> + __atomic_fetch_add (&x, 1, 0);
> + int c = 3;
> + __builtin_unreachable ();
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
