On Wed, 14 Dec 2016, Jakub Jelinek wrote:
> On Tue, Dec 13, 2016 at 10:05:58AM +0100, Richard Biener wrote:
> > Hmm, so the "easier" fix would be to always create a debug insn
> > right after INSN setting a new debug reg to SRC and then doing the
> > replacement with the new debug reg? With the "optimization"
> > to not do that for some single use and/or "simple" SRC cases
> > (I think "simple" SRC would be sth with a single/zero register reference).
> >
> > You are fixing it at the wrong end IMHO, by throwing away the
> > thing after propagating (and doing all the work and doing the
> > check on each individual propagation result).
>
> Here is updated patch that does that. I had to change the combiner
> in a couple of places, because combiner is very unhappy if new instructions
> (with uids above the highest) are inserted into the insn stream,
> e.g. FOR_EACH_LOG_LINK (link, temp_insn) for DEBUG_INSN temp_insn
> will then crash. But given that DEBUG_INSNs never have non-NULL LOG_LINKS,
> I think we can just avoid using LOG_LINKS or INSN_COST on DEBUG_INSNs
> and allow new DEBUG_INSNs be inserted into the stream (but not regular
> insns - normally combine just reuses INSN_UID of the insns it is replacing).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
LGTM, please leave others some time to comment though.
Thanks,
Richard.
> 2016-12-14 Jakub Jelinek <ja...@redhat.com>
>
> PR debug/77844
> * valtrack.c: Include rtl-iter.h.
> (struct rtx_subst_pair): Add insn field.
> (propagate_for_debug_subst): If pair->to contains at least 2
> regs, create a DEBUG_INSN with a debug temp before pair->insn
> and replace from with the debug temp instead of pair->to.
> (propagate_for_debug): Initialize p.insn.
> * combine.c (find_single_use, combine_instructions,
> cant_combine_insn_p, try_combine): Use NONDEBUG_INSN_P instead of
> INSN_P.
>
> * g++.dg/opt/pr77844.C: New test.
>
> --- gcc/valtrack.c.jj 2016-12-12 22:51:13.590952439 +0100
> +++ gcc/valtrack.c 2016-12-13 12:11:16.436894296 +0100
> @@ -29,6 +29,7 @@ along with GCC; see the file COPYING3.
> #include "regs.h"
> #include "memmodel.h"
> #include "emit-rtl.h"
> +#include "rtl-iter.h"
>
> /* gen_lowpart_no_emit hook implementation for DEBUG_INSNs. In DEBUG_INSNs,
> all lowpart SUBREGs are valid, despite what the machine requires for
> @@ -145,6 +146,7 @@ struct rtx_subst_pair
> {
> rtx to;
> bool adjusted;
> + rtx_insn *insn;
> };
>
> /* DATA points to an rtx_subst_pair. Return the value that should be
> @@ -162,6 +164,23 @@ propagate_for_debug_subst (rtx from, con
> pair->adjusted = true;
> pair->to = cleanup_auto_inc_dec (pair->to, VOIDmode);
> pair->to = make_compound_operation (pair->to, SET);
> + /* Avoid propagation from growing DEBUG_INSN expressions too much. */
> + int cnt = 0;
> + subrtx_iterator::array_type array;
> + FOR_EACH_SUBRTX (iter, array, pair->to, ALL)
> + if (REG_P (*iter) && ++cnt > 1)
> + {
> + rtx dval = make_debug_expr_from_rtl (old_rtx);
> + /* Emit a debug bind insn. */
> + rtx bind
> + = gen_rtx_VAR_LOCATION (GET_MODE (old_rtx),
> + DEBUG_EXPR_TREE_DECL (dval), pair->to,
> + VAR_INIT_STATUS_INITIALIZED);
> + rtx_insn *bind_insn = emit_debug_insn_before (bind, pair->insn);
> + df_insn_rescan (bind_insn);
> + pair->to = dval;
> + break;
> + }
> return pair->to;
> }
> return copy_rtx (pair->to);
> @@ -182,6 +201,7 @@ propagate_for_debug (rtx_insn *insn, rtx
> struct rtx_subst_pair p;
> p.to = src;
> p.adjusted = false;
> + p.insn = NEXT_INSN (insn);
>
> next = NEXT_INSN (insn);
> last = NEXT_INSN (last);
> --- gcc/combine.c.jj 2016-12-13 18:07:44.716123373 +0100
> +++ gcc/combine.c 2016-12-13 20:34:07.645287571 +0100
> @@ -676,7 +676,7 @@ find_single_use (rtx dest, rtx_insn *ins
> for (next = NEXT_INSN (insn);
> next && BLOCK_FOR_INSN (next) == bb;
> next = NEXT_INSN (next))
> - if (INSN_P (next) && dead_or_set_p (next, dest))
> + if (NONDEBUG_INSN_P (next) && dead_or_set_p (next, dest))
> {
> FOR_EACH_LOG_LINK (link, next)
> if (link->insn == insn && link->regno == REGNO (dest))
> @@ -1127,7 +1127,7 @@ combine_instructions (rtx_insn *f, unsig
>
> int new_direct_jump_p = 0;
>
> - for (first = f; first && !INSN_P (first); )
> + for (first = f; first && !NONDEBUG_INSN_P (first); )
> first = NEXT_INSN (first);
> if (!first)
> return 0;
> @@ -2275,7 +2275,7 @@ cant_combine_insn_p (rtx_insn *insn)
> /* If this isn't really an insn, we can't do anything.
> This can occur when flow deletes an insn that it has merged into an
> auto-increment address. */
> - if (! INSN_P (insn))
> + if (! NONDEBUG_INSN_P (insn))
> return 1;
>
> /* Never combine loads and stores involving hard regs that are likely
> @@ -4178,7 +4178,8 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
> || insn != BB_HEAD (this_basic_block->next_bb));
> insn = NEXT_INSN (insn))
> {
> - if (INSN_P (insn) && reg_referenced_p (ni2dest, PATTERN (insn)))
> + if (NONDEBUG_INSN_P (insn)
> + && reg_referenced_p (ni2dest, PATTERN (insn)))
> {
> FOR_EACH_LOG_LINK (link, insn)
> if (link->insn == i3)
> @@ -4319,9 +4320,9 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
> for (temp_insn = NEXT_INSN (i2);
> temp_insn
> && (this_basic_block->next_bb == EXIT_BLOCK_PTR_FOR_FN (cfun)
> - || BB_HEAD (this_basic_block) != temp_insn);
> + || BB_HEAD (this_basic_block) != temp_insn);
> temp_insn = NEXT_INSN (temp_insn))
> - if (temp_insn != i3 && INSN_P (temp_insn))
> + if (temp_insn != i3 && NONDEBUG_INSN_P (temp_insn))
> FOR_EACH_LOG_LINK (link, temp_insn)
> if (link->insn == i2)
> link->insn = i3;
> --- gcc/testsuite/g++.dg/opt/pr77844.C.jj 2016-12-13 11:52:12.355311153
> +0100
> +++ gcc/testsuite/g++.dg/opt/pr77844.C 2016-12-13 11:52:12.355311153
> +0100
> @@ -0,0 +1,32 @@
> +// PR debug/77844
> +// { dg-do compile }
> +// { dg-options "-O3 -g" }
> +
> +#include <vector>
> +
> +void
> +foo (std::vector<bool>& v, int i, int j)
> +{
> + for (int k = 0; k < 5; ++k)
> + v[5 * i + k] = v[5 * i + k] | v[5 * j + k];
> +}
> +
> +void
> +bar (std::vector<bool>& v, int i, int j)
> +{
> + for (int k = 0; k < 5; ++k)
> + v[5 * i + k] = v[5 * i + k] ^ v[5 * j + k];
> +}
> +
> +void
> +baz (std::vector<bool>& v)
> +{
> + foo (v, 4, 1);
> + foo (v, 4, 2);
> + foo (v, 4, 3);
> + foo (v, 4, 4);
> + bar (v, 4, 1);
> + bar (v, 4, 2);
> + bar (v, 4, 3);
> + bar (v, 4, 4);
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
