On Mon, Aug 19, 2019 at 12:15 PM Andrew Burgess <andrew.burg...@embecosm.com> wrote: > The solution presented in this patch offers a partial solution to this > problem. By using the TARGET_MACHINE_DEPENDENT_REORG pass to > implement a very limited pattern matching we identify functions that > call _riscv_save_0 and _riscv_restore_0, and which could be converted > to make use of a tail call. These functions are then converted to the > non save/restore tail call form.
Looking at this patch, I noticed a typo "sone" -> "some". You are using the British spellings of optimise, optimising, and optimisation. You have t0/x5 in SIBCALL_REG_P which I'd prefer to avoid because it is the alternative return reg. SIBCALL_REG_P looks like a potential maintenance problem, as it is duplicating info already available in REG_CLASS_CONTENTS and riscv_regno_to_class. Maybe you can just use riscv_regno_to_class to compute the info, that avoids adding another copy of the info. Similarly callee_saved_reg_p is duplicating info available elsewhere. You can probably just check call_used_regs and check for a 0 value. You have two loops to look for NOTE_INSN_PROLOGUE_END, but these will be small functions so not necessarily a problem. I'm wondering how well this works with debug info, since deleting the save_0, restore_0, and call will be deleting REG_NOTES that have call frame info, and you aren't adding anything back. But it looks like this works out OK as you aren't trying to change the frame, so the info looks like it should still be right with those REG_NOTES deleted. I didn't see anything obviously wrong here. Otherwise, the basic logic looks fine. It isn't obvious why it is failing. You will have to debug some of the failing testcases and refine the patch to work for them. I do see about a 1.5% size decrease for libc.a and a 2.5% size decrease for libstdc++.a for a 32-bit newlib toolchain build with -msave-restore hardwired on, with versus without your patch. So it is reducing code size. Jim
