This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83396#c28 on ia64 (PR bootstrap/83396, take 2)
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Jeff Law <law at redhat dot com>, Alexandre Oliva <aoliva at redhat dot com>, Andreas Schwab <schwab at suse dot de>, gcc-patches at gcc dot gnu dot org
- Date: Thu, 14 Dec 2017 11:31:40 +0100 (CET)
- Subject: Re: [PATCH] Fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83396#c28 on ia64 (PR bootstrap/83396, take 2)
- Authentication-results: sourceware.org; auth=none
- References: <20171213142507.GK2353@tucnak> <20171213222436.GO2353@tucnak>
On Wed, 13 Dec 2017, Jakub Jelinek wrote:
> On Wed, Dec 13, 2017 at 03:25:07PM +0100, Jakub Jelinek wrote:
> > I think there are 2 issues. One is that the ia64 backend emits
> > the group barrier insns before BB_HEAD label, so it isn't part of a bb,
> > but has BLOCK_FOR_INSN of the following block, that looks invalid to me
> > and the ia64.c hunk ought to fix that, except that I don't have access to
> > ia64 anymore and so can't test it. Andreas, could you try that?
> >
> > Another thing is that if we because of this end up with insns outside of
> > basic blocks, the vt_initialize asserts will fire again. Here, first of
> > all, IMNSHO we should assert that debug bind insns aren't outside of basic
> > blocks, the other patches and checking should ensure that (and if any slips
> > in, we want to fix that too rather than work-around).
> > Another is that while walking from get_first_insn to one before BB_HEAD (bb->next_bb),
> > we can encounter insns outside of bb not just before BB_HEAD (bb), but also
> > after BB_END (bb), both cases are outside of a bb and thus we can
> > expect BLOCK_FOR_INSN being NULL.
> >
> > Bootstrapped/regtested on x86_64-linux, i686-linux, powerpc64le-linux,
> > regtest on powerpc64-linux pending. Ok for trunk perhaps without the
> > ia64.c bits until that gets tested?
> >
> > Or, in the PR there is a variant patch which just doesn't do the asserts and
> > doesn't have to track outside_bb.
>
> Here is another variant, without trying to change ia64 backend which
> apparently doesn't bootstrap for other reasons.
>
> This patch instead ignores insns outside of basic blocks during var-tracking
> exactly as it has been ignoring them before, and just processes the debug
> begin stmt markers in there (and verifies no debug bind stmts appear in
> between bbs).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Richard.
> 2017-12-13 Jakub Jelinek <jakub@redhat.com>
>
> PR bootstrap/83396
> * var-tracking.c (vt_initialize): Ignore non-DEBUG_INSNs outside of
> basic blocks. Assert debug bind insns don't appear outside of bbs,
> don't reset them. Assert insns without BLOCK_FOR_INSN are outside of
> bb. Simplify.
>
> * gcc.dg/pr83396.c: New test.
>
> --- gcc/var-tracking.c.jj 2017-12-13 13:22:59.651783152 +0100
> +++ gcc/var-tracking.c 2017-12-13 19:11:13.895699735 +0100
> @@ -10157,25 +10157,31 @@ vt_initialize (void)
> insns that might be before it too. Unfortunately,
> BB_HEADER and BB_FOOTER are not set while we run this
> pass. */
> - insn = get_first_insn (bb);
> - for (rtx_insn *next;
> - insn != BB_HEAD (bb->next_bb)
> - ? next = NEXT_INSN (insn), true : false;
> + rtx_insn *next;
> + bool outside_bb = true;
> + for (insn = get_first_insn (bb); insn != BB_HEAD (bb->next_bb);
> insn = next)
> {
> + if (insn == BB_HEAD (bb))
> + outside_bb = false;
> + else if (insn == NEXT_INSN (BB_END (bb)))
> + outside_bb = true;
> + next = NEXT_INSN (insn);
> if (INSN_P (insn))
> {
> + if (outside_bb)
> + {
> + /* Ignore non-debug insns outside of basic blocks. */
> + if (!DEBUG_INSN_P (insn))
> + continue;
> + /* Debug binds shouldn't appear outside of bbs. */
> + gcc_assert (!DEBUG_BIND_INSN_P (insn));
> + }
> basic_block save_bb = BLOCK_FOR_INSN (insn);
> if (!BLOCK_FOR_INSN (insn))
> {
> + gcc_assert (outside_bb);
> BLOCK_FOR_INSN (insn) = bb;
> - gcc_assert (DEBUG_INSN_P (insn));
> - /* Reset debug insns between basic blocks.
> - Their location is not reliable, because they
> - were probably not maintained up to date. */
> - if (DEBUG_BIND_INSN_P (insn))
> - INSN_VAR_LOCATION_LOC (insn)
> - = gen_rtx_UNKNOWN_VAR_LOC ();
> }
> else
> gcc_assert (BLOCK_FOR_INSN (insn) == bb);
> --- gcc/testsuite/gcc.dg/pr83396.c.jj 2017-12-13 15:53:15.446687005 +0100
> +++ gcc/testsuite/gcc.dg/pr83396.c 2017-12-13 15:53:15.446687005 +0100
> @@ -0,0 +1,12 @@
> +/* PR bootstrap/83396 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g" } */
> +
> +int bar (int);
> +int baz (int);
> +
> +int
> +foo (int x)
> +{
> + return bar (x) || baz (x) != 0;
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)