This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix VTA updating in the combiner (PR debug/48343)
On Sun, Apr 03, 2011 at 11:39:22AM +0200, Eric Botcazou wrote:
> > I think we need to update there in all cases. The reason we don't need to
> > update beyond i3 resp. undobuf.other_insn is that DF guarantees us that
> > there won't be debug insns referring to those pseudos afterwards, otherwise
> > either the pseudo must be live afterwards in real code (then it wouldn't
> > be a single use case), or debug insns would be reset, or a debug temporary
> > would be created, where the temporary is set before last place where
> > the pseudo is used in real code. Now, once we propagate_for_debug after
> > some insn, DF hasn't been run in between and thus the pseudos might be live
> > afterwards.
>
> Frankly moving down last_combined_insn to undobuf.other_insn in the UNDO_MODE
> case seems a little overengineered at this point.
You are right, in that case we only replace regs with lowpart subreg
thereof, so it shouldn't extend the lifetime of any pseudo.
> > If you just want to avoid a global variable, the code can be surely changed
> > to have a local variable from combine_instructions and pass address to that
> > to all try_combine calls, but other than that I think we should do what the
> > patch does.
>
> I'd eliminate the global variable and directly pass the insn to try_combine,
> this is good enough for now IMO.
So something like this?
Alternatively, perhaps all we care about is either i3, or NEXT_INSN of the last
debug_insn propagate_for_debug changed in an interesting way. Thus
propagate_for_debug could return the last DEBUG_INSN it changed, and caller
decide either that no updating of last_modified_debug_insn is needed (that
is the case of the pair of propagate_for_debug calls from first to last
which just make it a lowpart subreg), or it would update it.
2011-04-04 Jakub Jelinek <jakub@redhat.com>
PR debug/48343
* combine.c (combine_instructions): Add last_combined_insn,
update it if insn is after it, pass it to all try_combine
calls.
(try_combine): Add last_combined_insn parameter, pass it instead of
i3 to propagate_for_debug.
* gcc.dg/torture/pr48343.c: New test.
--- gcc/combine.c.jj 2011-04-04 08:56:08.000000000 +0200
+++ gcc/combine.c 2011-04-04 09:41:13.000000000 +0200
@@ -387,7 +387,7 @@ static int cant_combine_insn_p (rtx);
static int can_combine_p (rtx, rtx, rtx, rtx, rtx, rtx, rtx *, rtx *);
static int combinable_i3pat (rtx, rtx *, rtx, rtx, rtx, int, int, rtx *);
static int contains_muldiv (rtx);
-static rtx try_combine (rtx, rtx, rtx, rtx, int *);
+static rtx try_combine (rtx, rtx, rtx, rtx, int *, rtx);
static void undo_all (void);
static void undo_commit (void);
static rtx *find_split_point (rtx *, rtx, bool);
@@ -1159,6 +1159,7 @@ combine_instructions (rtx f, unsigned in
FOR_EACH_BB (this_basic_block)
{
+ rtx last_combined_insn = NULL_RTX;
optimize_this_for_speed_p = optimize_bb_for_speed_p (this_basic_block);
last_call_luid = 0;
mem_last_set = -1;
@@ -1177,6 +1178,10 @@ combine_instructions (rtx f, unsigned in
next = 0;
if (NONDEBUG_INSN_P (insn))
{
+ if (last_combined_insn == NULL_RTX
+ || DF_INSN_LUID (last_combined_insn) < DF_INSN_LUID (insn))
+ last_combined_insn = insn;
+
/* See if we know about function return values before this
insn based upon SUBREG flags. */
check_promoted_subreg (insn, PATTERN (insn));
@@ -1190,7 +1195,8 @@ combine_instructions (rtx f, unsigned in
for (links = LOG_LINKS (insn); links; links = XEXP (links, 1))
if ((next = try_combine (insn, XEXP (links, 0), NULL_RTX,
- NULL_RTX, &new_direct_jump_p)) != 0)
+ NULL_RTX, &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
/* Try each sequence of three linked insns ending with this one. */
@@ -1208,8 +1214,8 @@ combine_instructions (rtx f, unsigned in
nextlinks;
nextlinks = XEXP (nextlinks, 1))
if ((next = try_combine (insn, link, XEXP (nextlinks, 0),
- NULL_RTX,
- &new_direct_jump_p)) != 0)
+ NULL_RTX, &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
}
@@ -1227,14 +1233,15 @@ combine_instructions (rtx f, unsigned in
&& sets_cc0_p (PATTERN (prev)))
{
if ((next = try_combine (insn, prev, NULL_RTX, NULL_RTX,
- &new_direct_jump_p)) != 0)
+ &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
for (nextlinks = LOG_LINKS (prev); nextlinks;
nextlinks = XEXP (nextlinks, 1))
if ((next = try_combine (insn, prev, XEXP (nextlinks, 0),
- NULL_RTX,
- &new_direct_jump_p)) != 0)
+ NULL_RTX, &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
}
@@ -1247,14 +1254,15 @@ combine_instructions (rtx f, unsigned in
&& reg_mentioned_p (cc0_rtx, SET_SRC (PATTERN (insn))))
{
if ((next = try_combine (insn, prev, NULL_RTX, NULL_RTX,
- &new_direct_jump_p)) != 0)
+ &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
for (nextlinks = LOG_LINKS (prev); nextlinks;
nextlinks = XEXP (nextlinks, 1))
if ((next = try_combine (insn, prev, XEXP (nextlinks, 0),
- NULL_RTX,
- &new_direct_jump_p)) != 0)
+ NULL_RTX, &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
}
@@ -1269,8 +1277,8 @@ combine_instructions (rtx f, unsigned in
&& NONJUMP_INSN_P (prev)
&& sets_cc0_p (PATTERN (prev))
&& (next = try_combine (insn, XEXP (links, 0),
- prev, NULL_RTX,
- &new_direct_jump_p)) != 0)
+ prev, NULL_RTX, &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
#endif
@@ -1281,7 +1289,8 @@ combine_instructions (rtx f, unsigned in
nextlinks = XEXP (nextlinks, 1))
if ((next = try_combine (insn, XEXP (links, 0),
XEXP (nextlinks, 0), NULL_RTX,
- &new_direct_jump_p)) != 0)
+ &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
/* Try four-instruction combinations. */
@@ -1305,14 +1314,16 @@ combine_instructions (rtx f, unsigned in
nextlinks = XEXP (nextlinks, 1))
if ((next = try_combine (insn, link, link1,
XEXP (nextlinks, 0),
- &new_direct_jump_p)) != 0)
+ &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
/* I0, I1 -> I2, I2 -> I3. */
for (nextlinks = XEXP (next1, 1); nextlinks;
nextlinks = XEXP (nextlinks, 1))
if ((next = try_combine (insn, link, link1,
XEXP (nextlinks, 0),
- &new_direct_jump_p)) != 0)
+ &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
}
@@ -1326,14 +1337,16 @@ combine_instructions (rtx f, unsigned in
nextlinks = XEXP (nextlinks, 1))
if ((next = try_combine (insn, link, link1,
XEXP (nextlinks, 0),
- &new_direct_jump_p)) != 0)
+ &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
/* I0 -> I1; I1, I2 -> I3. */
for (nextlinks = LOG_LINKS (link1); nextlinks;
nextlinks = XEXP (nextlinks, 1))
if ((next = try_combine (insn, link, link1,
XEXP (nextlinks, 0),
- &new_direct_jump_p)) != 0)
+ &new_direct_jump_p,
+ last_combined_insn)) != 0)
goto retry;
}
}
@@ -1362,7 +1375,8 @@ combine_instructions (rtx f, unsigned in
i2mod_old_rhs = copy_rtx (orig);
i2mod_new_rhs = copy_rtx (note);
next = try_combine (insn, i2mod, NULL_RTX, NULL_RTX,
- &new_direct_jump_p);
+ &new_direct_jump_p,
+ last_combined_insn);
i2mod = NULL_RTX;
if (next)
goto retry;
@@ -2501,10 +2515,15 @@ update_cfg_for_uncondjump (rtx insn)
resume scanning.
Set NEW_DIRECT_JUMP_P to a nonzero value if try_combine creates a
- new direct jump instruction. */
+ new direct jump instruction.
+
+ LAST_COMBINED_INSN is either I3, or some insn after I3 that has
+ been I3 passed to an earlier try_combine within the same basic
+ block. */
static rtx
-try_combine (rtx i3, rtx i2, rtx i1, rtx i0, int *new_direct_jump_p)
+try_combine (rtx i3, rtx i2, rtx i1, rtx i0, int *new_direct_jump_p,
+ rtx last_combined_insn)
{
/* New patterns for I3 and I2, respectively. */
rtx newpat, newi2pat = 0;
@@ -3851,7 +3870,7 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
i2src while its original mode is temporarily
restored, and then clear i2scratch so that we don't
do it again later. */
- propagate_for_debug (i2, i3, reg, i2src);
+ propagate_for_debug (i2, last_combined_insn, reg, i2src);
i2scratch = false;
/* Put back the new mode. */
adjust_reg_mode (reg, new_mode);
@@ -3864,13 +3883,16 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
if (reg == i2dest)
{
first = i2;
- last = i3;
+ last = last_combined_insn;
}
else
{
first = i3;
last = undobuf.other_insn;
gcc_assert (last);
+ if (DF_INSN_LUID (last)
+ < DF_INSN_LUID (last_combined_insn))
+ last = last_combined_insn;
}
/* We're dealing with a reg that changed mode but not
@@ -4098,14 +4120,14 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
if (newi2pat)
{
if (MAY_HAVE_DEBUG_INSNS && i2scratch)
- propagate_for_debug (i2, i3, i2dest, i2src);
+ propagate_for_debug (i2, last_combined_insn, i2dest, i2src);
INSN_CODE (i2) = i2_code_number;
PATTERN (i2) = newi2pat;
}
else
{
if (MAY_HAVE_DEBUG_INSNS && i2src)
- propagate_for_debug (i2, i3, i2dest, i2src);
+ propagate_for_debug (i2, last_combined_insn, i2dest, i2src);
SET_INSN_DELETED (i2);
}
@@ -4114,7 +4136,7 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
LOG_LINKS (i1) = 0;
REG_NOTES (i1) = 0;
if (MAY_HAVE_DEBUG_INSNS)
- propagate_for_debug (i1, i3, i1dest, i1src);
+ propagate_for_debug (i1, last_combined_insn, i1dest, i1src);
SET_INSN_DELETED (i1);
}
@@ -4123,7 +4145,7 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
LOG_LINKS (i0) = 0;
REG_NOTES (i0) = 0;
if (MAY_HAVE_DEBUG_INSNS)
- propagate_for_debug (i0, i3, i0dest, i0src);
+ propagate_for_debug (i0, last_combined_insn, i0dest, i0src);
SET_INSN_DELETED (i0);
}
--- gcc/testsuite/gcc.dg/torture/pr48343.c.jj 2011-04-04 09:31:33.000000000 +0200
+++ gcc/testsuite/gcc.dg/torture/pr48343.c 2011-04-04 09:31:33.000000000 +0200
@@ -0,0 +1,19 @@
+/* PR debug/48343 */
+/* { dg-do compile } */
+/* { dg-options "-fcompare-debug" } */
+
+void foo (unsigned char *, unsigned char *);
+
+void
+test (unsigned int x, int y)
+{
+ unsigned int i, j = 0, k;
+ unsigned char s[256], t[64];
+ foo (s, t);
+ t[0] = y;
+ for (i = 0; i < 256; i++)
+ {
+ j = (j + s[i] + t[i % x]) & 0xff;
+ k = i; i = j; j = k;
+ }
+}
Jakub