| Message ID | 20131126202540.GN892@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 26, 2013 at 9:25 PM, Jakub Jelinek wrote: \> PR rtl-optimization/59166 > * ira.c (find_moveable_pseudos): Use DF_REF_REAL_LOC instead of > DF_REF_LOC in validate_change call. > (split_live_ranges_for_shrink_wrap): Likewise. > > * gcc.dg/torture/pr59166.c: New test. If this falls under "RTL optimizers", then OK by me. Ciao! Steven
On Tue, Nov 26, 2013 at 9:34 PM, Steven Bosscher wrote: > On Tue, Nov 26, 2013 at 9:25 PM, Jakub Jelinek wrote: > \> PR rtl-optimization/59166 >> * ira.c (find_moveable_pseudos): Use DF_REF_REAL_LOC instead of >> DF_REF_LOC in validate_change call. >> (split_live_ranges_for_shrink_wrap): Likewise. >> >> * gcc.dg/torture/pr59166.c: New test. > > > If this falls under "RTL optimizers", then OK by me. BTW brownie points for cleaning up uid_luid, I see no reason why DF_INSN_LUID can't be used instead. Ciao! Steven
On 11/26/13 13:34, Steven Bosscher wrote: > On Tue, Nov 26, 2013 at 9:25 PM, Jakub Jelinek wrote: > \> PR rtl-optimization/59166 >> * ira.c (find_moveable_pseudos): Use DF_REF_REAL_LOC instead of >> DF_REF_LOC in validate_change call. >> (split_live_ranges_for_shrink_wrap): Likewise. >> >> * gcc.dg/torture/pr59166.c: New test. > > > If this falls under "RTL optimizers", then OK by me. IMHO, it does :-) jeff
On 11/26/13 13:25, Jakub Jelinek wrote: > Hi! > > The problem on this testcase is that we have > (debug_insn 30 29 31 7 (var_location:HI D#1 (subreg:HI (reg/v:SI 93 [ p ]) 0)) pr59166.c:20 -1 > (nil)) > and split_live_ranges_for_shrink_wrap decides to replace > SImode pseudo 93 with some other SImode pseudo. But it > uses DF_REF_LOC, which is address of the HImode subreg around the pseudo, > validate_change succeeds on it, because there is no validation inside of > debug_insns and then we crash during var-tracking because of a mode > mismatch. > > The following patch uses DF_REF_REAL_LOC instead, so that it looks through > the subreg and adjusts what it should. Code inspection showed the same > issue in find_moveable_pseudos, don't have a testcase for that though. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2013-11-26 Jakub Jelinek <jakub@redhat.com> > > PR rtl-optimization/59166 > * ira.c (find_moveable_pseudos): Use DF_REF_REAL_LOC instead of > DF_REF_LOC in validate_change call. > (split_live_ranges_for_shrink_wrap): Likewise. > > * gcc.dg/torture/pr59166.c: New test. OK. Jeff
On Tue, Nov 26, 2013 at 09:25:40PM +0100, Jakub Jelinek wrote: > Hi! > > The problem on this testcase is that we have > (debug_insn 30 29 31 7 (var_location:HI D#1 (subreg:HI (reg/v:SI 93 [ p ]) 0)) pr59166.c:20 -1 > (nil)) > and split_live_ranges_for_shrink_wrap decides to replace > SImode pseudo 93 with some other SImode pseudo. But it > uses DF_REF_LOC, which is address of the HImode subreg around the pseudo, > validate_change succeeds on it, because there is no validation inside of > debug_insns and then we crash during var-tracking because of a mode > mismatch. thanks a lot for fixing it, I had no idea that DF_REF_REAL_LOC even existed. This change also apparently makes it possible to throw away the SUBREG check in the same function. I'll enqueue a patch removing it for the next stage1. > > The following patch uses DF_REF_REAL_LOC instead, so that it looks through > the subreg and adjusts what it should. Code inspection showed the same > issue in find_moveable_pseudos, don't have a testcase for that though. That is where I copied the substitution from. Martin > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2013-11-26 Jakub Jelinek <jakub@redhat.com> > > PR rtl-optimization/59166 > * ira.c (find_moveable_pseudos): Use DF_REF_REAL_LOC instead of > DF_REF_LOC in validate_change call. > (split_live_ranges_for_shrink_wrap): Likewise. > > * gcc.dg/torture/pr59166.c: New test.
--- gcc/ira.c.jj 2013-11-25 10:20:12.000000000 +0100 +++ gcc/ira.c 2013-11-26 12:45:27.443495633 +0100 @@ -4812,7 +4812,7 @@ find_moveable_pseudos (void) { rtx def_reg = DF_REF_REG (def); rtx newreg = ira_create_new_reg (def_reg); - if (validate_change (def_insn, DF_REF_LOC (def), newreg, 0)) + if (validate_change (def_insn, DF_REF_REAL_LOC (def), newreg, 0)) { unsigned nregno = REGNO (newreg); emit_insn_before (gen_move_insn (def_reg, newreg), use_insn); @@ -5034,7 +5034,7 @@ split_live_ranges_for_shrink_wrap (void) rtx newreg = NULL_RTX; df_ref use, next; - for (use = DF_REG_USE_CHAIN (REGNO(dest)); use; use = next) + for (use = DF_REG_USE_CHAIN (REGNO (dest)); use; use = next) { rtx uin = DF_REF_INSN (use); next = DF_REF_NEXT_REG (use); @@ -5045,7 +5045,7 @@ split_live_ranges_for_shrink_wrap (void) { if (!newreg) newreg = ira_create_new_reg (dest); - validate_change (uin, DF_REF_LOC (use), newreg, true); + validate_change (uin, DF_REF_REAL_LOC (use), newreg, true); } } --- gcc/testsuite/gcc.dg/torture/pr59166.c.jj 2013-11-26 12:49:33.551233602 +0100 +++ gcc/testsuite/gcc.dg/torture/pr59166.c 2013-11-26 12:50:15.589019973 +0100 @@ -0,0 +1,37 @@ +/* PR rtl-optimization/59166 */ +/* { dg-additional-options "-fcompare-debug" } */ + +int a, b, c, f, g; + +void +foo () +{ + for (; b; b++) + for (; f; f = g) + for (; a;) + ; +} + +static int +bar (int p) +{ + short d; + if (c) + { + for (; f; f = g); + foo (); + d = p; + char e = d; + if (p) + return 1; + } + return p; +} + +int +main () +{ + bar (0); + bar (g); + return 0; +}
Hi! The problem on this testcase is that we have (debug_insn 30 29 31 7 (var_location:HI D#1 (subreg:HI (reg/v:SI 93 [ p ]) 0)) pr59166.c:20 -1 (nil)) and split_live_ranges_for_shrink_wrap decides to replace SImode pseudo 93 with some other SImode pseudo. But it uses DF_REF_LOC, which is address of the HImode subreg around the pseudo, validate_change succeeds on it, because there is no validation inside of debug_insns and then we crash during var-tracking because of a mode mismatch. The following patch uses DF_REF_REAL_LOC instead, so that it looks through the subreg and adjusts what it should. Code inspection showed the same issue in find_moveable_pseudos, don't have a testcase for that though. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2013-11-26 Jakub Jelinek <jakub@redhat.com> PR rtl-optimization/59166 * ira.c (find_moveable_pseudos): Use DF_REF_REAL_LOC instead of DF_REF_LOC in validate_change call. (split_live_ranges_for_shrink_wrap): Likewise. * gcc.dg/torture/pr59166.c: New test. Jakub