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