On Wed, 22 Nov 2017, Jakub Jelinek wrote:
> Hi!
>
> Seems at least the scheduler treats UNSPEC_VOLATILE and perhaps volatile asm
> in DEBUG_INSNs as having side-effects and schedules differently depending on
> that. While we could tweak the scheduler not to do that, these will be
> actually never useful in DEBUG_INSNs anyway, we don't know what it means so
> we can't represent it in debug info.
>
> So, the following patch just makes sure we don't add them into debug_insns,
> instead reset those.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Do we have some RTL IL verifier where we could verify those don't appear?
Maybe ICE in dwarf2out.c?
Thanks,
Richard.
> 2017-11-22 Jakub Jelinek <ja...@redhat.com>
>
> PR debug/83084
> * valtrack.c (propagate_for_debug_subst, propagate_for_debug): Reset
> debug insns if they would contain UNSPEC_VOLATILE or volatile asm.
> (dead_debug_insert_temp): Likewise, but also ignore even non-volatile
> asm.
>
> * g++.dg/opt/pr83084.C: New test.
>
> --- gcc/valtrack.c.jj 2017-09-12 21:58:01.000000000 +0200
> +++ gcc/valtrack.c 2017-11-21 18:26:10.188009398 +0100
> @@ -171,10 +171,13 @@ propagate_for_debug_subst (rtx from, con
> if (REG_P (*iter) && ++cnt > 1)
> {
> rtx dval = make_debug_expr_from_rtl (old_rtx);
> + rtx to = pair->to;
> + if (volatile_insn_p (to))
> + to = gen_rtx_UNKNOWN_VAR_LOC ();
> /* Emit a debug bind insn. */
> rtx bind
> = gen_rtx_VAR_LOCATION (GET_MODE (old_rtx),
> - DEBUG_EXPR_TREE_DECL (dval), pair->to,
> + DEBUG_EXPR_TREE_DECL (dval), to,
> VAR_INIT_STATUS_INITIALIZED);
> rtx_insn *bind_insn = emit_debug_insn_before (bind, pair->insn);
> df_insn_rescan (bind_insn);
> @@ -217,6 +220,8 @@ propagate_for_debug (rtx_insn *insn, rtx
> dest, propagate_for_debug_subst, &p);
> if (loc == INSN_VAR_LOCATION_LOC (insn))
> continue;
> + if (volatile_insn_p (loc))
> + loc = gen_rtx_UNKNOWN_VAR_LOC ();
> INSN_VAR_LOCATION_LOC (insn) = loc;
> df_insn_rescan (insn);
> }
> @@ -660,6 +665,12 @@ dead_debug_insert_temp (struct dead_debu
> }
> return 0;
> }
> + /* Asm in DEBUG_INSN is never useful, we can't emit debug info for
> + that. And for volatile_insn_p, it is actually harmful
> + - DEBUG_INSNs shouldn't have any side-effects. */
> + else if (GET_CODE (src) == ASM_OPERANDS
> + || volatile_insn_p (src))
> + set = NULL_RTX;
> }
>
> /* ??? Should we try to extract it from a PARALLEL? */
> --- gcc/testsuite/g++.dg/opt/pr83084.C.jj 2017-11-21 18:30:22.252935128
> +0100
> +++ gcc/testsuite/g++.dg/opt/pr83084.C 2017-11-21 18:29:18.000000000
> +0100
> @@ -0,0 +1,16 @@
> +// PR debug/83084
> +// { dg-do compile }
> +// { dg-options "-O2 -fcompare-debug -Wno-return-type" }
> +
> +enum E { F };
> +template <E = F> struct A {
> + bool foo ();
> + int b;
> +};
> +template <> bool A<>::foo () {
> + int a;
> + do
> + if (a)
> + return false;
> + while (__atomic_compare_exchange_n (&b, &a, 0, 1, 4, 0));
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
