* Jim Wilson <j...@sifive.com> [2019-10-17 14:55:34 -0700]: > On Thu, Oct 17, 2019 at 7:09 AM Andrew Burgess > <andrew.burg...@embecosm.com> wrote: > > I'm still working on part 2, I'm hoping to have a revised patch posted > > by Monday next week. > > I started looking at the part 2 patch also. I noticed a problem where > the NOTE_INSN_EPILOGUE_BEGIN can occur before the > NOTE_INSN_PROLOGUE_END due to branch and basic block optimizations, I > get an ICE in this case due to deref of a null pointer. This is > pretty easy to fix, I just added a check to > riscv_remove_unneeded_save_restore_calls in the loop that searches for > NOTE_INSN_PROLOGUE_END, and if I find NOTE_INSN_EPILOGUE_BEGIN first I > exit without doing anything. A more complex fix would be to try to > follow the CFG instead of scanning forward from the prologue end note > to find the epilogue begin note. I also noticed a case where the > __riscv_save_0 call was optimized away but the __riscv_restore_0 call > was not. In this case the function has two epilogues, and only one of > the two epilogues was optimized. We could just check for more than > one epilogue and return without doing any work as the simple fix. Or > a more complex fix is to try to handle more than one epilogue. > > Given the kinds of problems I'm seeing, I think there should be an > option to control this optimization, so people can turn it off is > necessary. I think it is OK if this is on by default if it passes the > gcc testsuite. > > Since you are looking at this, I can look at something else for a few > days. I was just trying to get this off of my backlog.
It's entirely up to you. I believe I have a version testing now that addresses all of the issues you mentioned above. I haven't added a switch for it, but can do if you want. My expectation was that I would post a version on Monday that had zero regressions when run with -msave-restore forced on (as you described in a previous mail). I'll continue to work on this and post on Monday unless you drop a revision earlier. Thanks, Andrew
