[RISCV][MC]Add support for Binary MCExpr

[Yunzezhu]

Summary

There is an issue: https://github.com/llvm/llvm-project/issues/64612
This issue happens because in RISCVMCCodeEmitter::getImmOpValue it only handles MCExpr kind Target and SymbolRef.
When code with format like .+ comes in, it comes with MCExpr kind Binary, the fixupkind remains fixup_riscv_invalid and reports error.

This patch make MCExpr kind Binary handled with the same way as MCExpr kind SymbolRef, so code with binary expression can get correct fixupkind and be used to generate more complex relocation.

[asb]

Thanks for catching this. It looks like if you do e.g. .LBB0-16 it will just underflow - do other targets do anything smarter here?

[wangpc]

I tried to fix this issue yesterday and I used the same way as this patch. Its problem is that it can't complile case in https://github.com/llvm/llvm-project/issues/64623:

<unknown>:0: error: unsupported relocation type

Because there is no corresponding relocation type for fixup type fixup_riscv_12_i.

[Yunzezhu]

In D157694#4584915, @asb wrote:

Thanks for catching this. It looks like if you do e.g. .LBB0-16 it will just underflow - do other targets do anything smarter here?

It seems PowerPC handles expr with binary expression opcode add only by adding an assert, so no underflow happens without sub kind binary expr. Other targets support binary expr like Hexagon and Lanai get fixup kind separately from LHS and RHS or from LHS only, and ignore such possible underflow.
I tried gcc and gcc supports sub kind binary exprs like .LBB0-16 too. I'm not sure if such sub expr will cause underflow since local symbol handles relocation only and global symbol looks have little chance of underflow.

[Yunzezhu]

fixed a problem cause warning during compiling.

[jobnoorman]

In D157694#4587199, @wangpc wrote:

I tried to fix this issue yesterday and I used the same way as this patch. Its problem is that it can't complile case in https://github.com/llvm/llvm-project/issues/64623:

<unknown>:0: error: unsupported relocation type

Because there is no corresponding relocation type for fixup type fixup_riscv_12_i.

I think this is fine as this error is only generated when relaxation is enabled in which case it safer to not allow the symbol difference expression in LI. The error message could be improved though.

[Yunzezhu]

ping

[Yunzezhu]

gentle ping

[asb]

As this lacks coverage for li which was the source of the original bug report, could you please add in the tests from D158830. I think it would probably be worth capturing the current underflow behaviour too (perhaps with a FIXME? note that it would likely be better to error here). Thanks.

[Yunzezhu]

Add test cases from https://reviews.llvm.org/D158830 and FIXME note.

[Yunzezhu]

gentle ping

[Yunzezhu]

gentle ping

[yroux]

Hi,

Just a comment to point that this patch will help building the RISCV Architecture test-suite with Clang
where test macros for JAL and branch contain instructions like inst reg1, reg2, label+offset.

I was about to submit the same patch, so it LGTM ;) but I let others accept it.

[asb]

LGTM, thanks!

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

case RISCVMCExpr::VK_RISCV_CALL:
       FixupKind = RISCV::fixup_riscv_call;
       RelaxCandidate = true;
       break;
     case RISCVMCExpr::VK_RISCV_CALL_PLT:
       FixupKind = RISCV::fixup_riscv_call_plt;
       RelaxCandidate = true;
       break;
     }
-  } else if (Kind == MCExpr::SymbolRef &&
-             cast<MCSymbolRefExpr>(Expr)->getKind() == MCSymbolRefExpr::VK_None) {
+  } else if ((Kind == MCExpr::SymbolRef &&
+                 cast<MCSymbolRefExpr>(Expr)->getKind() ==
+                     MCSymbolRefExpr::VK_None) ||
+             Kind == MCExpr::Binary) {
+    // FIXME: Sub kind binary exprs have chance of underflow.
     if (MIFrm == RISCVII::InstFormatJ) {
       FixupKind = RISCV::fixup_riscv_jal;
     } else if (MIFrm == RISCVII::InstFormatB) {
       FixupKind = RISCV::fixup_riscv_branch;
     } else if (MIFrm == RISCVII::InstFormatCJ) {
       FixupKind = RISCV::fixup_riscv_rvc_jump;
     } else if (MIFrm == RISCVII::InstFormatCB) {
       FixupKind = RISCV::fixup_riscv_rvc_branch;

llvm/test/MC/RISCV/fixups-binary-expression.s

+# RUN: llvm-mc -triple riscv32 -mattr=+c -riscv-no-aliases < %s -show-encoding \
+# RUN:     | FileCheck -check-prefix=CHECK-FIXUP %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+c < %s \
+# RUN:     | llvm-objdump -M no-aliases -d - \
+# RUN:     | FileCheck -check-prefix=CHECK-INSTR %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+c,+relax < %s \
+# RUN:     | llvm-readobj -r - | FileCheck -check-prefix=CHECK-RELOC %s
+
+.LBB0:
+
+.LBB1:
+
+jal zero, .LBB0+16
+# CHECK-FIXUP: fixup A - offset: 0, value: .LBB0+16, kind: fixup_riscv_jal
+# CHECK-INSTR: jal zero, 0x10
+# CHECK-RELOC:  R_RISCV_JAL
+
+beq a0, a1, .LBB1+32
+# CHECK-FIXUP: fixup A - offset: 0, value: .LBB1+32, kind: fixup_riscv_branch
+# CHECK-INSTR: beq a0, a1, 0x20
+# CHECK-RELOC:  R_RISCV_BRANCH
+
+c.j     .+32
+# CHECK:   fixup A - offset: 0, value: .Ltmp0+32, kind: fixup_riscv_rvc_jump
+# CHECK-INSTR: c.j   0x28
+# CHECK-RELOC:  R_RISCV_RVC_JUMP
+
+c.beqz a0, .-2
+# CHECK-FIXUP: fixup A - offset: 0, value: .Ltmp1-2, kind: fixup_riscv_rvc_branch
+# CHECK-INSTR: c.beqz a0, 0x8
+# CHECK-RELOC:  R_RISCV_RVC_BRANCH

llvm/test/MC/RISCV/fixups-invalid.s

 # RUN: not llvm-mc -filetype=obj %s -triple=riscv32 -o /dev/null 2>&1 \
 # RUN:     | FileCheck %s
 # RUN: not llvm-mc -filetype=obj %s -triple=riscv64 -o /dev/null 2>&1 \
 # RUN:     | FileCheck %s
 
 .byte foo   # CHECK: [[@LINE]]:7: error: 1-byte data relocations not supported
 .2byte foo  # CHECK: [[@LINE]]:8: error: 2-byte data relocations not supported
 
 # Test that using li with a symbol difference constant rejects values that
 # cannot fit in a signed 12-bit integer.
 .Lbuf: .skip (1 << 11)
 .Lbuf_end:
 .equ CONST, .Lbuf_end - .Lbuf
 # CHECK: error: operand must be a constant 12-bit integer
 li a0, CONST
+# CHECK: error: operand must be a constant 12-bit integer
+li a0, .Lbuf_end - .Lbuf

llvm/test/MC/RISCV/rv32i-aliases-valid.s

 li a0, CONST
 
 .equ CONST, .Lbuf_end - .Lbuf
 # CHECK-ASM: li a0, CONST
 # CHECK-ASM-NOALIAS: addi a0, zero, CONST
 # CHECK-OBJ-NOALIAS: addi a0, zero, 8
 li a0, CONST
 
+# CHECK-ASM: addi a0, zero, .Lbuf_end-.Lbuf
+# CHECK-ASM-NOALIAS: addi a0, zero, .Lbuf_end-.Lbuf
+# CHECK-OBJ-NOALIAS: addi a0, zero, 8
+li a0, .Lbuf_end - .Lbuf
+
 # CHECK-INST: addi a0, zero, 0
 # CHECK-ALIAS: li a0, 0
 la x10, 0
 lla x10, 0
 # CHECK-INST: addi a0, zero, 1
 # CHECK-ALIAS: li a0, 1
 la x10, 1
 lla x10, 1

llvm/test/MC/RISCV/rv64i-aliases-valid.s

 li a0, CONST
 
 .equ CONST, .Lbuf_end - .Lbuf
 # CHECK-ASM: li a0, CONST
 # CHECK-ASM-NOALIAS: addi a0, zero, CONST
 # CHECK-OBJ-NOALIAS: addi a0, zero, 8
 li a0, CONST
 
+# CHECK-ASM: addi a0, zero, .Lbuf_end-.Lbuf
+# CHECK-ASM-NOALIAS: addi a0, zero, .Lbuf_end-.Lbuf
+# CHECK-OBJ-NOALIAS: addi a0, zero, 8
+li a0, .Lbuf_end - .Lbuf
+
 # CHECK-INST: addi a0, zero, 0
 # CHECK-ALIAS: li a0, 0
 la x10, 0
 lla x10, 0
 # CHECK-INST: addi a0, zero, 1
 # CHECK-ALIAS: li a0, 1
 la x10, 1
 lla x10, 1