Re: [PATCH 2/2] gcc/riscv: Add a mechanism to remove some calls to _riscv_save_0

[Jim Wilson <jimw@sifive.com>]

On Mon, Aug 19, 2019 at 12:15 PM Andrew Burgess
<andrew.burgess@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