[PATCH] Avoid creating unbounded complexity debug insns in valtrack (PR debug/77844, take 2)
Jakub Jelinek
jakub@redhat.com
Wed Dec 14 08:36:00 GMT 2016
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?
2016-12-14 Jakub Jelinek <jakub@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
More information about the Gcc-patches
mailing list