[RISCV] Remove RA from reserved register to use as callee saved register

[shiva0217]

Summary

Remove RA from reserved register list, so we could use it as callee saved register

[lenary]

We discussed this in the RISC-V meeting on 19 Sep 2019.

• Pros: GCC for RISC-V and LLVM for ARM and AArch64 seem to do the same. It can help in situations with particularly bad register pressure requirements.
• Cons: It can make debugging a lot harder, though this seems not to be an issue in GDB for RISC-V.

I think we don't want to have yet another configuration flag to control this.

It would be good to see some performance comparison, but I realise you may not be able to release internal benchmarks, and we have no public benchmarking system for RISC-V.

[shiva0217]

In D67698#1675370, @lenary wrote:

We discussed this in the RISC-V meeting on 19 Sep 2019.

• Pros: GCC for RISC-V and LLVM for ARM and AArch64 seem to do the same. It can help in situations with particularly bad register pressure requirements.
• Cons: It can make debugging a lot harder, though this seems not to be an issue in GDB for RISC-V.

I think we don't want to have yet another configuration flag to control this.

It would be good to see some performance comparison, but I realise you may not be able to release internal benchmarks, and we have no public benchmarking system for RISC-V.

I can't reveal the detail of the benchmark, but the most significant case improve about 1%. It might be reasonable to free RA for register allocation since most of the backend already do so.

[luismarques]

In CoreMark-Pro, when the execution model is 1 instr == 1 cycle, with this patch the results I get is that sha-test improves by +1.43% in RV64 (GC, LP64D). For all other sub-benchmarks the performance differences round to 0.00%. There's no change to sha-test in RV32 before and after the patch.

[lenary]

We had a discussion internally at lowRISC about this. I no longer think that debuggability is a concern, as FP/CFI-info is far more important than the return address. Alex has raised that preserving RA is useful for some lightweight tracing applications, but I feel that can be solved with the implementation of -ffixed-x1 (D67185).

With that in mind, we're happy to approve this patch, pending the following:

Please can you rebase it? It applies, but tests fail. Looks like a simple update caused by a change to the materialisation of the address of var.

[shiva0217]

Rebase the test case.

Good to know -ffixed-x1 seems to be a great solution.

[lenary]

LGTM, thanks @shiva0217

lib/Target/RISCV/RISCVRegisterInfo.cpp

 }
 
 BitVector RISCVRegisterInfo::getReservedRegs(const MachineFunction &MF) const {
   const TargetFrameLowering *TFI = getFrameLowering(MF);
   BitVector Reserved(getNumRegs());
 
   // Use markSuperRegs to ensure any register aliases are also reserved
   markSuperRegs(Reserved, RISCV::X0); // zero
-  markSuperRegs(Reserved, RISCV::X1); // ra
   markSuperRegs(Reserved, RISCV::X2); // sp
   markSuperRegs(Reserved, RISCV::X3); // gp
   markSuperRegs(Reserved, RISCV::X4); // tp
   if (TFI->hasFP(MF))
     markSuperRegs(Reserved, RISCV::X8); // fp
   assert(checkAllSuperRegsMarked(Reserved));
   return Reserved;
 }

test/CodeGen/RISCV/callee-saved-gprs.ll

 
 ; This function tests that RISCVRegisterInfo::getCalleeSavedRegs returns
 ; something appropriate.
 
 define void @callee() nounwind {
 ; RV32I-LABEL: callee:
 ; RV32I:       # %bb.0:
 ; RV32I-NEXT:    addi sp, sp, -80
-; RV32I-NEXT:    sw s0, 76(sp)
-; RV32I-NEXT:    sw s1, 72(sp)
-; RV32I-NEXT:    sw s2, 68(sp)
-; RV32I-NEXT:    sw s3, 64(sp)
-; RV32I-NEXT:    sw s4, 60(sp)
-; RV32I-NEXT:    sw s5, 56(sp)
-; RV32I-NEXT:    sw s6, 52(sp)
-; RV32I-NEXT:    sw s7, 48(sp)
-; RV32I-NEXT:    sw s8, 44(sp)
-; RV32I-NEXT:    sw s9, 40(sp)
-; RV32I-NEXT:    sw s10, 36(sp)
-; RV32I-NEXT:    sw s11, 32(sp)
+; RV32I-NEXT:    sw ra, 76(sp)
+; RV32I-NEXT:    sw s0, 72(sp)
+; RV32I-NEXT:    sw s1, 68(sp)
+; RV32I-NEXT:    sw s2, 64(sp)
+; RV32I-NEXT:    sw s3, 60(sp)
+; RV32I-NEXT:    sw s4, 56(sp)
+; RV32I-NEXT:    sw s5, 52(sp)
+; RV32I-NEXT:    sw s6, 48(sp)
+; RV32I-NEXT:    sw s7, 44(sp)
+; RV32I-NEXT:    sw s8, 40(sp)
+; RV32I-NEXT:    sw s9, 36(sp)
+; RV32I-NEXT:    sw s10, 32(sp)
+; RV32I-NEXT:    sw s11, 28(sp)
 ; RV32I-NEXT:    lui a0, %hi(var)
 ; RV32I-NEXT:    addi a1, a0, %lo(var)
 ;
 ; RV32I-WITH-FP-LABEL: callee:
 ; RV32I-WITH-FP:       # %bb.0:
 ; RV32I-WITH-FP-NEXT:    addi sp, sp, -80
 ; RV32I-WITH-FP-NEXT:    sw ra, 76(sp)
 ; RV32I-WITH-FP-NEXT:    sw s0, 72(sp)

 ; RV32I-WITH-FP-NEXT:    sw s11, 28(sp)
 ; RV32I-WITH-FP-NEXT:    addi s0, sp, 80
 ; RV32I-WITH-FP-NEXT:    lui a0, %hi(var)
 ; RV32I-WITH-FP-NEXT:    addi a1, a0, %lo(var)
 ;
 ; RV64I-LABEL: callee:
 ; RV64I:       # %bb.0:
 ; RV64I-NEXT:    addi sp, sp, -144
-; RV64I-NEXT:    sd s0, 136(sp)
-; RV64I-NEXT:    sd s1, 128(sp)
-; RV64I-NEXT:    sd s2, 120(sp)
-; RV64I-NEXT:    sd s3, 112(sp)
-; RV64I-NEXT:    sd s4, 104(sp)
-; RV64I-NEXT:    sd s5, 96(sp)
-; RV64I-NEXT:    sd s6, 88(sp)
-; RV64I-NEXT:    sd s7, 80(sp)
-; RV64I-NEXT:    sd s8, 72(sp)
-; RV64I-NEXT:    sd s9, 64(sp)
-; RV64I-NEXT:    sd s10, 56(sp)
-; RV64I-NEXT:    sd s11, 48(sp)
+; RV64I-NEXT:    sd ra, 136(sp)
+; RV64I-NEXT:    sd s0, 128(sp)
+; RV64I-NEXT:    sd s1, 120(sp)
+; RV64I-NEXT:    sd s2, 112(sp)
+; RV64I-NEXT:    sd s3, 104(sp)
+; RV64I-NEXT:    sd s4, 96(sp)
+; RV64I-NEXT:    sd s5, 88(sp)
+; RV64I-NEXT:    sd s6, 80(sp)
+; RV64I-NEXT:    sd s7, 72(sp)
+; RV64I-NEXT:    sd s8, 64(sp)
+; RV64I-NEXT:    sd s9, 56(sp)
+; RV64I-NEXT:    sd s10, 48(sp)
+; RV64I-NEXT:    sd s11, 40(sp)
 ; RV64I-NEXT:    lui a0, %hi(var)
 ; RV64I-NEXT:    addi a1, a0, %lo(var)
 ;
 ; RV64I-WITH-FP-LABEL: callee:
 ; RV64I-WITH-FP:       # %bb.0:
 ; RV64I-WITH-FP-NEXT:    addi sp, sp, -160
 ; RV64I-WITH-FP-NEXT:    sd ra, 152(sp)
 ; RV64I-WITH-FP-NEXT:    sd s0, 144(sp)