llvm dev - Dec 2020 - Does brind need to preserve the return address register? RISCV seems to think so

The RISCV target defines a pseudo instruction for `brind` as shown below
(RISCVInstrInfo.td):

let isCall = 1, Defs=[X1] in
let isBarrier = 1, isBranch = 1, isIndirectBranch = 1, isTerminator = 1 in
def PseudoBRIND : Pseudo<(outs), (ins GPR:$rs1, simm12:$imm12), []>,
                  PseudoInstExpansion<(JALR X0, GPR:$rs1, simm12:$imm12)>;

def : Pat<(brind GPR:$rs1), (PseudoBRIND GPR:$rs1, 0)>;
def : Pat<(brind (add GPR:$rs1, simm12:$imm12)),
          (PseudoBRIND GPR:$rs1, simm12:$imm12)>;

Note the `Defs=[X1]` (the return address register) in the definition
despite it not actually being used in the expansion. This results in code
being generated to save the link register before performing an indirect
jump (llvm/test/CodeGen/RISCV/indirectbr.ll):

define i32 @indirectbr(i8* %target) nounwind {
; RV32I-LABEL: indirectbr:
; RV32I:       # %bb.0:
; RV32I-NEXT:    addi sp, sp, -16
; RV32I-NEXT:    sw ra, 12(sp)
; RV32I-NEXT:    jr a0
; RV32I-NEXT:  .LBB0_1: # %test_label
; RV32I-NEXT:    mv a0, zero
; RV32I-NEXT:    lw ra, 12(sp)
; RV32I-NEXT:    addi sp, sp, 16
; RV32I-NEXT:    ret
  indirectbr i8* %target, [label %test_label]
test_label:
  br label %ret
ret:
  ret i32 0
}

This seems unnecessary to me, as `brind` is not a call, right? Or are the
semantics of `brind` more complicated than I understand it to be? As far as
I can tell ARM doesn't do this, but I can't follow ARMInstrInfo.td as
well,
so I'm not sure. Should I replicate this in my target? Or should I send a
patch to fix this in the RISCV target?

Would appreciate any help in the matter.

Thanks,
Cristi.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201212/b4826369/attachment.html>

You are right, x1 is not redefined by `PseudoBRIND`. There was recently a
fix for this in the RISC-V backend:
https://github.com/llvm/llvm-project/commit/9e6c09c0d995721eff6366d9af2f8ab1c203bf61

Sam

On Sat, Dec 12, 2020 at 10:53 PM Cristian Cobzarenco via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> The RISCV target defines a pseudo instruction for `brind` as shown below
> (RISCVInstrInfo.td):
>
> let isCall = 1, Defs=[X1] in
> let isBarrier = 1, isBranch = 1, isIndirectBranch = 1, isTerminator = 1 in
> def PseudoBRIND : Pseudo<(outs), (ins GPR:$rs1, simm12:$imm12), []>,
>                   PseudoInstExpansion<(JALR X0, GPR:$rs1,
simm12:$imm12)>;
>
> def : Pat<(brind GPR:$rs1), (PseudoBRIND GPR:$rs1, 0)>;
> def : Pat<(brind (add GPR:$rs1, simm12:$imm12)),
>           (PseudoBRIND GPR:$rs1, simm12:$imm12)>;
>
> Note the `Defs=[X1]` (the return address register) in the definition
> despite it not actually being used in the expansion. This results in code
> being generated to save the link register before performing an indirect
> jump (llvm/test/CodeGen/RISCV/indirectbr.ll):
>
> define i32 @indirectbr(i8* %target) nounwind {
> ; RV32I-LABEL: indirectbr:
> ; RV32I:       # %bb.0:
> ; RV32I-NEXT:    addi sp, sp, -16
> ; RV32I-NEXT:    sw ra, 12(sp)
> ; RV32I-NEXT:    jr a0
> ; RV32I-NEXT:  .LBB0_1: # %test_label
> ; RV32I-NEXT:    mv a0, zero
> ; RV32I-NEXT:    lw ra, 12(sp)
> ; RV32I-NEXT:    addi sp, sp, 16
> ; RV32I-NEXT:    ret
>   indirectbr i8* %target, [label %test_label]
> test_label:
>   br label %ret
> ret:
>   ret i32 0
> }
>
> This seems unnecessary to me, as `brind` is not a call, right? Or are the
> semantics of `brind` more complicated than I understand it to be? As far as
> I can tell ARM doesn't do this, but I can't follow ARMInstrInfo.td
as well,
> so I'm not sure. Should I replicate this in my target? Or should I send
a
> patch to fix this in the RISCV target?
>
> Would appreciate any help in the matter.
>
> Thanks,
> Cristi.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

-- 
Sam Elliott
Software Team Lead
Senior Software Developer - LLVM and OpenTitan
lowRISC CIC
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201213/e7496864/attachment.html>