| Message ID | 4F2648FA.8040702@gnu.org |
|---|---|
| State | New |
| Headers | show |
On 30.01.2012 11:38, Paolo Bonzini wrote: > On 01/29/2012 04:09 PM, Eric Botcazou wrote: >>>> As discussed in Bugzilla, this is the patch implementing Paolo's >>>> suggestion of killing REG_EQUAL/REG_EQUIV notes from df_kill_notes. The >>>> code assumes there is at most one such note per insn. >>> >>> That's wrong though and wreaks havoc during reload, e.g.: >>> >>> (insn 169 60 62 4 (set (reg:TF 158) >>> (mem/c:TF (plus:SI (reg/f:SI 101 %sfp) >>> (const_int -16 [0xfffffffffffffff0])) [3 S16 A64])) >>> 960513-1.c:13 97 {*movtf_insn_sp32} >>> (expr_list:REG_EQUIV (mem/c:TF (plus:SI (reg/f:SI 101 %sfp) >>> (const_int -16 [0xfffffffffffffff0])) [3 S16 A64]) >>> (expr_list:REG_EQUAL (mult:TF (reg/v:TF 110 [ d ]) >>> (reg:TF 154)) >>> (nil)))) >>> >>> because the REG_EQUIV note disappears behind reload's back and it isn't >>> prepared for that. This is the cause of the following regression on SPARC: >>> >>> FAIL: gcc.c-torture/execute/960513-1.c execution, -Os >> >> As well as: >> >> FAIL: gcc.c-torture/execute/stdarg-2.c execution, -O2 >> FAIL: gcc.c-torture/execute/stdarg-2.c execution, -O3 -fomit-frame-pointer >> FAIL: gcc.c-torture/execute/stdarg-2.c execution, -O3 -g >> FAIL: gcc.c-torture/execute/stdarg-2.c execution, -Os >> FAIL: gcc.c-torture/execute/stdarg-2.c >> execution, -O2 -flto -flto-partition=none >> FAIL: gcc.c-torture/execute/stdarg-2.c execution, -O2 -flto >> >> for the exact same reason. > > Does this help? That would fix the problem of multiple notes per insn (as we wanted to do initially), but I didn't understand whether this is the real problem or the problem is the reload not happy with disappearing notes. Also I can't reproduce it with a cross to sparc64-linux -- when I put a breakpoint on the code removing the notes, I find only similarly looking insn 148 which gets removed from the df_analyze call at the start of IRA. Though I see the fail from SPARC test results on the ML, so I guess I'm missing something... Andrey > > Paolo >
> Does this help?
Yep, this eliminates all the regressions, thanks!
> That would fix the problem of multiple notes per insn (as we wanted to do > initially), but I didn't understand whether this is the real problem or the > problem is the reload not happy with disappearing notes. reload maintains a mapping between its internal structures and the RTL stream, here between reg_equiv_* fields and REG_EQUIV notes. It simply isn't prepared to deal with the case where this mapping is broken behind its back.
On 01/30/2012 01:22 PM, Eric Botcazou wrote: >> Does this help? > Yep, this eliminates all the regressions, thanks! Committed as r183719. Thanks for testing. Paolo
On 01/30/2012 09:44 AM, Andrey Belevantsev wrote: >> >> Does this help? > > That would fix the problem of multiple notes per insn (as we wanted to > do initially), but I didn't understand whether this is the real problem > or the problem is the reload not happy with disappearing notes. Also I > can't reproduce it with a cross to sparc64-linux -- when I put a > breakpoint on the code removing the notes, I find only similarly looking > insn 148 which gets removed from the df_analyze call at the start of > IRA. Though I see the fail from SPARC test results on the ML, so I > guess I'm missing something... The REG_EQUAL note can go, but the REG_EQUIV note should not (by definition: they are valid throughout the entire function). In fact, we could just as well apply the loop to REG_EQUAL notes only but that would have been a bit too clever and more risky. Paolo
On 30.01.2012 17:47, Paolo Bonzini wrote: > On 01/30/2012 09:44 AM, Andrey Belevantsev wrote: >>> >>> Does this help? >> >> That would fix the problem of multiple notes per insn (as we wanted to >> do initially), but I didn't understand whether this is the real problem >> or the problem is the reload not happy with disappearing notes. Also I >> can't reproduce it with a cross to sparc64-linux -- when I put a >> breakpoint on the code removing the notes, I find only similarly looking >> insn 148 which gets removed from the df_analyze call at the start of >> IRA. Though I see the fail from SPARC test results on the ML, so I >> guess I'm missing something... > > The REG_EQUAL note can go, but the REG_EQUIV note should not (by > definition: they are valid throughout the entire function). In fact, we > could just as well apply the loop to REG_EQUAL notes only but that would > have been a bit too clever and more risky. Eric, Paolo, thanks for the explanations! Andrey
2012-01-30 Paolo Bonzini <bonzini@gnu.org> * df-problems.c (df_kill_notes): Check that the use refers to the note under examination. Index: df-problems.c =================================================================== --- df-problems.c (revision 183693) +++ df-problems.c (working copy) @@ -2814,8 +2814,10 @@ df_kill_notes (rtx insn, bitmap live) { df_ref use = *use_rec; if (DF_REF_REGNO (use) > FIRST_PSEUDO_REGISTER + && DF_REF_LOC (use) && (DF_REF_FLAGS (use) & DF_REF_IN_NOTE) - && ! bitmap_bit_p (live, DF_REF_REGNO (use))) + && ! bitmap_bit_p (live, DF_REF_REGNO (use)) + && loc_mentioned_in_p (DF_REF_LOC (use), XEXP (link, 0))) { deleted = true; break;