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  <ja...@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

Reply via email to