[RISCV] Match GNU tools canonical JALR and add aliases

[jrtc27]

Summary

The canonical GNU form of JALR resembles a load/store instruction rather
than placing the immediate offset as a separate argument, so match this
behaviour. Also add parser-only aliases for the three-operand form, and
add other shorter aliases also emitted by GNU tools.

[jrtc27]

test/CodeGen/RISCV/branch-relaxation.ll

35

Currently the MCOperandPredicate for simm12 doesn't match things other than literal constants and bare symbols so our aliases don't match; in future it should be modified to match the various LO12 variant kinds.

[jrtc27]

Rebased and added TODO comment.

[jrtc27]

Fixed broken test

[asb]

Thanks James, this looks good to me (needs a rebase and trivial changes for rv32e-valid.s before landing).

[plotfi]

This change has caused test failures in lld tests:

http://lab.llvm.org:8011/builders/lld-x86_64-ubuntu-fast/builds/3357

[plotfi]

Test failures in lld tests: http://lab.llvm.org:8011/builders/lld-x86_64-ubuntu-fast/builds/3357

[asb]

In D55277#1586957, @plotfi wrote:

Test failures in lld tests: http://lab.llvm.org:8011/builders/lld-x86_64-ubuntu-fast/builds/3357

Sorry about that, patch incoming.

[plotfi]

In D55277#1586958, @asb wrote:

In D55277#1586957, @plotfi wrote:

Test failures in lld tests: http://lab.llvm.org:8011/builders/lld-x86_64-ubuntu-fast/builds/3357

Sorry about that, patch incoming.

I got one coming too. Should I wait for yours or just land this one?

[plotfi]

Don't worry about it, I landed a change in r366183

[asb]

In D55277#1586959, @plotfi wrote:

In D55277#1586958, @asb wrote:

In D55277#1586957, @plotfi wrote:

Test failures in lld tests: http://lab.llvm.org:8011/builders/lld-x86_64-ubuntu-fast/builds/3357

Sorry about that, patch incoming.

I got one coming too. Should I wait for yours or just land this one?

Oh great, please land yours then. Mind is written blind as lld doesn't seem to be building after I add it to LLVM_ENABLE_PROJECTS. Will have to wipe my build dir and wait a while.

[plotfi]

In D55277#1586961, @asb wrote:

In D55277#1586959, @plotfi wrote:

In D55277#1586958, @asb wrote:

In D55277#1586957, @plotfi wrote:

Test failures in lld tests: http://lab.llvm.org:8011/builders/lld-x86_64-ubuntu-fast/builds/3357

Sorry about that, patch incoming.

I got one coming too. Should I wait for yours or just land this one?

Oh great, please land yours then. Mind is written blind as lld doesn't seem to be building after I add it to LLVM_ENABLE_PROJECTS. Will have to wipe my build dir and wait a while.

Would you be able to do a post commit look over: https://github.com/llvm/llvm-project/commit/3e10905c49ffda36d7a90e24be1ab30832fa4afc

??

Thanks

PL

[asb]

In D55277#1586965, @plotfi wrote:

In D55277#1586961, @asb wrote:

In D55277#1586959, @plotfi wrote:

In D55277#1586958, @asb wrote:

In D55277#1586957, @plotfi wrote:

Test failures in lld tests: http://lab.llvm.org:8011/builders/lld-x86_64-ubuntu-fast/builds/3357

Sorry about that, patch incoming.

I got one coming too. Should I wait for yours or just land this one?

Oh great, please land yours then. Mind is written blind as lld doesn't seem to be building after I add it to LLVM_ENABLE_PROJECTS. Will have to wipe my build dir and wait a while.

Would you be able to do a post commit look over: https://github.com/llvm/llvm-project/commit/3e10905c49ffda36d7a90e24be1ab30832fa4afc

??

Thanks

PL

LGTM. Thanks for helping clean this up. LLD is now in my standard build and pre-commit check setup.

lib/Target/RISCV/RISCVInstrInfo.td

 
 let isCall = 1 in
 def JAL : RVInstJ<OPC_JAL, (outs GPR:$rd), (ins simm21_lsb0_jal:$imm20),
                   "jal", "$rd, $imm20">;
 
 let isCall = 1 in
 def JALR : RVInstI<0b000, OPC_JALR, (outs GPR:$rd),
                    (ins GPR:$rs1, simm12:$imm12),
-                   "jalr", "$rd, $rs1, $imm12">;
+                   "jalr", "$rd, ${imm12}(${rs1})">;
 } // hasSideEffects = 0, mayLoad = 0, mayStore = 0
 
 def BEQ  : BranchCC_rri<0b000, "beq">;
 def BNE  : BranchCC_rri<0b001, "bne">;
 def BLT  : BranchCC_rri<0b100, "blt">;
 def BGE  : BranchCC_rri<0b101, "bge">;
 def BLTU : BranchCC_rri<0b110, "bltu">;
 def BGEU : BranchCC_rri<0b111, "bgeu">;
def : InstAlias<"bgt $rs, $rt, $offset",
                 (BLT  GPR:$rt, GPR:$rs, simm13_lsb0:$offset), 0>;
 def : InstAlias<"ble $rs, $rt, $offset",
                 (BGE  GPR:$rt, GPR:$rs, simm13_lsb0:$offset), 0>;
 def : InstAlias<"bgtu $rs, $rt, $offset",
                 (BLTU GPR:$rt, GPR:$rs, simm13_lsb0:$offset), 0>;
 def : InstAlias<"bleu $rs, $rt, $offset",
                 (BGEU GPR:$rt, GPR:$rs, simm13_lsb0:$offset), 0>;
 
-// "ret" has more weight since "ret" and "jr" alias the same "jalr" instruction.
-def : InstAlias<"j $offset",   (JAL  X0, simm21_lsb0_jal:$offset)>;
-def : InstAlias<"jal $offset", (JAL  X1, simm21_lsb0_jal:$offset)>;
-def : InstAlias<"jr $rs",      (JALR X0, GPR:$rs, 0)>;
-def : InstAlias<"jalr $rs",    (JALR X1, GPR:$rs, 0)>;
-def : InstAlias<"ret",         (JALR X0,      X1, 0), 2>;
+def : InstAlias<"j $offset",   (JAL X0, simm21_lsb0_jal:$offset)>;
+def : InstAlias<"jal $offset", (JAL X1, simm21_lsb0_jal:$offset)>;
+
+// Non-zero offset aliases of "jalr" are the lowest weight, followed by the
+// two-register form, then the one-register forms and finally "ret".
+def : InstAlias<"jr $rs",                (JALR      X0, GPR:$rs, 0), 3>;
+def : InstAlias<"jr ${offset}(${rs})",   (JALR      X0, GPR:$rs, simm12:$offset)>;
+def : InstAlias<"jalr $rs",              (JALR      X1, GPR:$rs, 0), 3>;
+def : InstAlias<"jalr ${offset}(${rs})", (JALR      X1, GPR:$rs, simm12:$offset)>;
+def : InstAlias<"jalr $rd, $rs",         (JALR GPR:$rd, GPR:$rs, 0), 2>;
+def : InstAlias<"ret",                   (JALR      X0,      X1, 0), 4>;
+
+// Non-canonical forms for jump targets also accepted by the assembler.
+def : InstAlias<"jr $rs, $offset",        (JALR      X0, GPR:$rs, simm12:$offset), 0>;
+def : InstAlias<"jalr $rs, $offset",      (JALR      X1, GPR:$rs, simm12:$offset), 0>;
+def : InstAlias<"jalr $rd, $rs, $offset", (JALR GPR:$rd, GPR:$rs, simm12:$offset), 0>;
+
 // TODO call
 // TODO tail
 
 def : InstAlias<"fence", (FENCE 0xF, 0xF)>; // 0xF == iorw
 
 // CSR Addresses: 0xC00 == cycle,  0xC01 == time,  0xC02 == instret
 //                0xC80 == cycleh, 0xC81 == timeh, 0xC82 == instreth
 def : InstAlias<"rdinstret $rd", (CSRRS GPR:$rd, 0xC02, X0)>;

test/CodeGen/RISCV/branch-relaxation.ll

 
 define i32 @relax_jal(i1 %a) {
 ; CHECK-LABEL: relax_jal:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    andi a0, a0, 1
 ; CHECK-NEXT:    bnez a0, .LBB1_1
 ; CHECK-NEXT:  # %bb.3:
 ; CHECK-NEXT:    lui a0, %hi(.LBB1_2)
-; CHECK-NEXT:    jalr zero, a0, %lo(.LBB1_2)
+; CHECK-NEXT:    jalr zero, %lo(.LBB1_2)(a0)
 ; CHECK-NEXT:  .LBB1_1: # %iftrue
 ; CHECK-NEXT:    #APP
 ; CHECK-NEXT:    #NO_APP
 ; CHECK-NEXT:    #APP
 ; CHECK-NEXT:    .space 1048576
 ; CHECK-NEXT:    #NO_APP
 ; CHECK-NEXT:    addi a0, zero, 1
 ; CHECK-NEXT:    ret

test/CodeGen/RISCV/indirectbr.ll

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

test/CodeGen/RISCV/option-rvc.ll

 ; RUN: llc -mtriple=riscv32 -filetype=obj < %s\
 ; RUN: | llvm-objdump -triple=riscv32 -mattr=+c -d -riscv-no-aliases -\
 ; RUN: | FileCheck -check-prefix=CHECK %s
 
 ; This test demonstrates that .option norvc has no effect on codegen when
 ; emitting an ELF directly.
 
 define i32 @add(i32 %a, i32 %b) nounwind {
 ; CHECK-LABEL: add:
 ; CHECK:    add a0, a1, a0
-; CHECK-NEXT:    jalr zero, ra, 0
+; CHECK-NEXT:    jalr zero, 0(ra)
   tail call void asm sideeffect ".option rvc", ""()
   %add = add nsw i32 %b, %a
   ret i32 %add
 }

test/MC/RISCV/compress-rv32i.s

 # CHECK-INST: c.lwsp  ra, 252(sp)
 # CHECK: # encoding:  [0xfe,0x50]
 lw ra, 252(sp)
 
 # CHECK-BYTES: 82 80
 # CHECK-ALIAS: ret
 # CHECK-INST: c.jr ra
 # CHECK: # encoding:  [0x82,0x80]
-jalr zero, ra, 0
+jalr zero, 0(ra)
 
 # CHECK-BYTES: 92 80
 # CHECK-ALIAS: add ra, zero, tp
 # CHECK-INST: c.mv ra, tp
 # CHECK:  # encoding: [0x92,0x80]
 add ra, zero, tp
 
 # CHECK-BYTES: 92 80
 # CHECK-ALIAS: add ra, zero, tp
 # CHECK-INST: c.mv ra, tp
 # CHECK:  # encoding: [0x92,0x80]
 add ra, tp, zero
 
 # CHECK-BYTES: 02 90
 # CHECK-ALIAS: ebreak
 # CHECK-INST: c.ebreak
 # CHECK: # encoding: [0x02,0x90]
 ebreak
 
 # CHECK-BYTES: 02 94
 # CHECK-ALIAS: jalr s0
 # CHECK-INST: c.jalr s0
 # CHECK: # encoding: [0x02,0x94]
-jalr ra, s0, 0
+jalr ra, 0(s0)
 
 # CHECK-BYTES: 3e 94
 # CHECK-ALIAS: add s0, s0, a5
 # CHECK-INST: c.add s0, a5
 # CHECK: # encoding:  [0x3e,0x94]
 add s0, a5, s0
 
 # CHECK-BYTES: 3e 94

test/MC/RISCV/fixups.s

 # Testing the function call offset could resovled by assembler
 # when the function and the callsite within the same compile unit
 # and the linker relaxation is disabled.
 func:
 .fill 100
 call func
 # CHECK-FIXUP: fixup A - offset: 0, value: func, kind: fixup_riscv_call
 # CHECK-INSTR: auipc   ra, 0
-# CHECK-INSTR: jalr    ra, ra, -100
+# CHECK-INSTR: jalr    ra, -100(ra)
 
 .fill 10000
 call func
 # CHECK-FIXUP: fixup A - offset: 0, value: func, kind: fixup_riscv_call
 # CHECK-INSTR: auipc   ra, 1048574
-# CHECK-INSTR: jalr    ra, ra, -1916
+# CHECK-INSTR: jalr    ra, -1916(ra)
 
 .fill 20888
 call func
 # CHECK-FIXUP: fixup A - offset: 0, value: func, kind: fixup_riscv_call
 # CHECK-INSTR: auipc   ra, 1048568
-# CHECK-INSTR: jalr    ra, ra, 1764
+# CHECK-INSTR: jalr    ra, 1764(ra)

test/MC/RISCV/rv32i-valid.s

 # CHECK-OBJ: R_RISCV_JAL foo
 jal a0, foo
 # CHECK-ASM: jal a0, a0
 # CHECK-ASM: encoding: [0x6f,0bAAAA0101,A,A]
 # CHECK-OBJ: jal a0, 0
 # CHECK-OBJ: R_RISCV_JAL a0
 jal a0, a0
 
-# CHECK-ASM-AND-OBJ: jalr a0, a1, -2048
+# CHECK-ASM-AND-OBJ: jalr a0, -2048(a1)
 # CHECK-ASM: encoding: [0x67,0x85,0x05,0x80]
-jalr a0, a1, -2048
-# CHECK-ASM-AND-OBJ: jalr a0, a1, -2048
+jalr a0, -2048(a1)
+# CHECK-ASM-AND-OBJ: jalr a0, -2048(a1)
 # CHECK-ASM: encoding: [0x67,0x85,0x05,0x80]
-jalr a0, a1, %lo(2048)
-# CHECK-ASM-AND-OBJ: jalr t2, t1, 2047
+jalr a0, %lo(2048)(a1)
+# CHECK-ASM-AND-OBJ: jalr t2, 2047(t1)
 # CHECK-ASM: encoding: [0xe7,0x03,0xf3,0x7f]
-jalr t2, t1, 2047
-# CHECK-ASM-AND-OBJ: jalr sp, zero, 256
+jalr t2, 2047(t1)
+# CHECK-ASM-AND-OBJ: jalr sp, 256(zero)
 # CHECK-ASM: encoding: [0x67,0x01,0x00,0x10]
-jalr sp, zero, 256
+jalr sp, 256(zero)
 
 # CHECK-ASM-AND-OBJ: beq s1, s1, 102
 # CHECK-ASM: encoding: [0x63,0x83,0x94,0x06]
 beq s1, s1, 102
 # CHECK-ASM-AND-OBJ: bne a4, a5, -4096
 # CHECK-ASM: encoding: [0x63,0x10,0xf7,0x80]
 bne a4, a5, -4096
 # CHECK-ASM-AND-OBJ: blt sp, gp, 4094

test/MC/RISCV/rvi-aliases-valid.s

 # CHECK-OBJ: R_RISCV_JAL foo
 jal foo
 # CHECK-S-NOALIAS: jal ra, a0
 # CHECK-S: jal a0
 # CHECK-OBJ-NOALIAS: jal ra, 0
 # CHECK-OBJ: jal 0
 # CHECK-OBJ: R_RISCV_JAL a0
 jal a0
-# CHECK-S-OBJ-NOALIAS: jalr zero, s4, 0
+# CHECK-S-OBJ-NOALIAS: jalr zero, 0(s4)
 # CHECK-S-OBJ: jr s4
 jr x20
-# CHECK-S-OBJ-NOALIAS: jalr ra, s5, 0
-# CHECK-S-OBJ: jalr s5
-jalr x21
-# CHECK-S-OBJ-NOALIAS: jalr zero, ra, 0
+# CHECK-S-OBJ-NOALIAS: jalr zero, 6(s5)
+# CHECK-S-OBJ: jr 6(s5)
+jr 6(x21)
+# CHECK-S-OBJ-NOALIAS: jalr zero, 7(s6)
+# CHECK-S-OBJ: jr 7(s6)
+jr x22, 7
+# CHECK-S-OBJ-NOALIAS: jalr ra, 0(s4)
+# CHECK-S-OBJ: jalr s4
+jalr x20
+# CHECK-S-OBJ-NOALIAS: jalr ra, 8(s5)
+# CHECK-S-OBJ: jalr 8(s5)
+jalr 8(x21)
+# CHECK-S-OBJ-NOALIAS: jalr s6, 0(s7)
+# CHECK-S-OBJ: jalr s6, s7
+jalr x22, x23
+# CHECK-S-OBJ-NOALIAS: jalr ra, 9(s8)
+# CHECK-S-OBJ: jalr 9(s8)
+jalr x24, 9
+# CHECK-S-OBJ-NOALIAS: jalr s9, 11(s10)
+# CHECK-S-OBJ: jalr s9, 11(s10)
+jalr x25, x26, 11
+# CHECK-S-OBJ-NOALIAS: jalr zero, 0(ra)
 # CHECK-S-OBJ: ret
 ret
 # TODO call
 # TODO tail
 
 # CHECK-S-OBJ-NOALIAS: fence iorw, iorw
 # CHECK-S-OBJ: fence
 fence