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. Jim
