| Message ID | b8da34c1fe62f15164194ba5613e4533c30d300a.1566241796.git.andrew.burgess@embecosm.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISCV: Reduce code size when compiling with -msave-restore | expand |
x5 is used as an alternate link register, so using it for sibcalls will confuse hardware return-address stacks and reduce performance for implementations that have one. The reason I excluded a0-a7 is that those are used to pass arguments to the sibcallee. It's of course possible that's more restrictive than necessary, but make sure to test before merging. On Mon, Aug 19, 2019 at 12:16 PM Andrew Burgess <andrew.burgess@embecosm.com> wrote: > > The current SIBCALL_REGS are x6, x7, and x28 to x31. These are all > caller saved registers, however, they are not all of the caller saved > registers. > > I don't see any reason why we couldn't add t1, and a0 to a7 into this > set, and this is what this patch does. > > gcc/ChangeLog: > > * config/riscv/riscv.h (REG_CLASS_CONTENTS): Update SIBCALL_REGS. > --- > gcc/ChangeLog | 4 ++++ > gcc/config/riscv/riscv.h | 2 +- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h > index 5fc9be8edbf2..bb8240bb849a 100644 > --- a/gcc/config/riscv/riscv.h > +++ b/gcc/config/riscv/riscv.h > @@ -400,7 +400,7 @@ enum reg_class > #define REG_CLASS_CONTENTS \ > { \ > { 0x00000000, 0x00000000, 0x00000000 }, /* NO_REGS */ \ > - { 0xf00000c0, 0x00000000, 0x00000000 }, /* SIBCALL_REGS */ \ > + { 0xf003fce0, 0x00000000, 0x00000000 }, /* SIBCALL_REGS */ \ > { 0xffffffc0, 0x00000000, 0x00000000 }, /* JALR_REGS */ \ > { 0xffffffff, 0x00000000, 0x00000000 }, /* GR_REGS */ \ > { 0x00000000, 0xffffffff, 0x00000000 }, /* FP_REGS */ \ > -- > 2.14.5 >
On Mon, Aug 19, 2019 at 12:15 PM Andrew Burgess <andrew.burgess@embecosm.com> wrote: > I don't see any reason why we couldn't add t1, and a0 to a7 into this > set, and this is what this patch does. SIBCALL_REGS already includes t1 and t2. It is t0 aka x5 that is missing. I think this is wrong. As Andrew mentioned, this will penalize any target that has a call-stack aware branch predictor. We could add a tune flag for that, but it doesn't seem worth the effort. Adding the other regs a0 to a7 is OK. They won't be used unless they are available. This is OK without the t0/x5 change. Jim
On Thu, Aug 22, 2019 at 10:30 PM Jim Wilson <jimw@sifive.com> wrote: > > On Mon, Aug 19, 2019 at 12:15 PM Andrew Burgess > <andrew.burgess@embecosm.com> wrote: > > I don't see any reason why we couldn't add t1, and a0 to a7 into this > > set, and this is what this patch does. > > SIBCALL_REGS already includes t1 and t2. It is t0 aka x5 that is > missing. I think this is wrong. As Andrew mentioned, this will > penalize any target that has a call-stack aware branch predictor. We > could add a tune flag for that, but it doesn't seem worth the effort. > Adding the other regs a0 to a7 is OK. They won't be used unless they > are available. This is OK without the t0/x5 change. While reviewing the second part of the patch set, I noticed that I missed riscv_regno_to_class. A change to SIBCALL_REGS requires a corresponding change to riscv_regno_class. SIBCALL_REGS is the smallest class, so you just need to change the entries for a0 to a7 from JALR_REGS to SIBCALL_REGS. Jim
diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h index 5fc9be8edbf2..bb8240bb849a 100644 --- a/gcc/config/riscv/riscv.h +++ b/gcc/config/riscv/riscv.h @@ -400,7 +400,7 @@ enum reg_class #define REG_CLASS_CONTENTS \ { \ { 0x00000000, 0x00000000, 0x00000000 }, /* NO_REGS */ \ - { 0xf00000c0, 0x00000000, 0x00000000 }, /* SIBCALL_REGS */ \ + { 0xf003fce0, 0x00000000, 0x00000000 }, /* SIBCALL_REGS */ \ { 0xffffffc0, 0x00000000, 0x00000000 }, /* JALR_REGS */ \ { 0xffffffff, 0x00000000, 0x00000000 }, /* GR_REGS */ \ { 0x00000000, 0xffffffff, 0x00000000 }, /* FP_REGS */ \