| Message ID | 3d1c2c877df1da0be9eca7cb1016e8b7a7d287c8.1470015604.git.segher@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
On 07/31/2016 07:42 PM, Segher Boessenkool wrote: > A restore is supposed to restore some certain register. Restoring it > into some other register will not work. Don't. > > 2016-06-07 Segher Boessenkool <segher@kernel.crashing.org> > > * regrename.c (build_def_use): Invalidate chains that have a > REG_CFA_RESTORE on some instruction. Again, how is this different from a normal epilogue that restores registers which regrename seems to not muck with. Jeff
On Thu, Sep 08, 2016 at 11:51:53AM -0600, Jeff Law wrote: > On 07/31/2016 07:42 PM, Segher Boessenkool wrote: > >A restore is supposed to restore some certain register. Restoring it > >into some other register will not work. Don't. > > > >2016-06-07 Segher Boessenkool <segher@kernel.crashing.org> > > > > * regrename.c (build_def_use): Invalidate chains that have a > > REG_CFA_RESTORE on some instruction. > Again, how is this different from a normal epilogue that restores > registers which regrename seems to not muck with. Good question. Either way, it is always wrong to rename a register we restore from stack. Segher
On 09/09/2016 02:59 PM, Segher Boessenkool wrote: > On Thu, Sep 08, 2016 at 11:51:53AM -0600, Jeff Law wrote: >> On 07/31/2016 07:42 PM, Segher Boessenkool wrote: >>> A restore is supposed to restore some certain register. Restoring it >>> into some other register will not work. Don't. >>> >>> 2016-06-07 Segher Boessenkool <segher@kernel.crashing.org> >>> >>> * regrename.c (build_def_use): Invalidate chains that have a >>> REG_CFA_RESTORE on some instruction. >> Again, how is this different from a normal epilogue that restores >> registers which regrename seems to not muck with. > > Good question. Either way, it is always wrong to rename a register we > restore from stack. Agreed. Somehow register renaming does the right thing for a normal epilogue. I don't see anything in regrename that obviously treats the epilogue specially, there's check_new_reg_p, but I don't see that it inherently handles this case. regrename seems to use the DF infrastructure, so I wouldn't be surprised if this is a symptom of incorrect DF information for the epilogues. One way to potentially find out would be to tweak the DF code to mark all the callee saved regs as live at the return insns and see how that affects regrename's decision making. jeff
diff --git a/gcc/regrename.c b/gcc/regrename.c index 54c7768..00a5d02 100644 --- a/gcc/regrename.c +++ b/gcc/regrename.c @@ -1867,6 +1867,12 @@ build_def_use (basic_block bb) scan_rtx (insn, &XEXP (note, 0), NO_REGS, terminate_dead, OP_IN); } + + /* Step 8: Kill the chains involving register restores. Those + should restore _that_ register. */ + for (note = REG_NOTES (insn); note; note = XEXP (note, 1)) + if (REG_NOTE_KIND (note) == REG_CFA_RESTORE) + scan_rtx (insn, &XEXP (note, 0), NO_REGS, mark_all_read, OP_IN); } else if (DEBUG_INSN_P (insn) && !VAR_LOC_UNKNOWN_P (INSN_VAR_LOCATION_LOC (insn)))