Skip to content

Conversation

lenary
Copy link
Member

@lenary lenary commented Jun 6, 2024

This broadly follows how in almost all places, we accept (<reg>) to mean 0(<reg>), but I think these are the first like this for Jumps rather than Loads/Stores. These are accepted by binutils but not by LLVM: https://godbolt.org/z/GK7MGE7q7

This broadly follows how in almost all places, we accept `(<reg>)` to
mean `0(<reg>)`, but I think these are the first like this for Jumps
rather than Loads/Stores. These are accepted by binutils but not by
LLVM: https://godbolt.org/z/GK7MGE7q7
@lenary lenary requested review from asb and apazos June 6, 2024 21:58
@llvmbot llvmbot added backend:RISC-V llvm:mc Machine (object) code labels Jun 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 6, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-risc-v

Author: Sam Elliott (lenary)

Changes

This broadly follows how in almost all places, we accept (&lt;reg&gt;) to mean 0(&lt;reg&gt;), but I think these are the first like this for Jumps rather than Loads/Stores. These are accepted by binutils but not by LLVM: https://godbolt.org/z/GK7MGE7q7


Full diff: https://github.com/llvm/llvm-project/pull/94688.diff

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.td (+3)
  • (modified) llvm/test/MC/RISCV/rvi-aliases-valid.s (+10)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index a1b078910e29c..c73fc52ee729d 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -943,6 +943,9 @@ def : InstAlias<"ret",                   (JALR      X0,      X1, 0), 4>;
 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>;
+def : InstAlias<"jr (${rs})",             (JALR      X0, GPR:$rs, 0), 0>;
+def : InstAlias<"jalr (${rs})",           (JALR      X1, GPR:$rs, 0), 0>;
+def : InstAlias<"jalr $rd, (${rs})",      (JALR GPR:$rd, GPR:$rs, 0), 0>;
 
 def : InstAlias<"fence", (FENCE 0xF, 0xF)>; // 0xF == iorw
 
diff --git a/llvm/test/MC/RISCV/rvi-aliases-valid.s b/llvm/test/MC/RISCV/rvi-aliases-valid.s
index 098d5c132c98c..613fc25cdf3af 100644
--- a/llvm/test/MC/RISCV/rvi-aliases-valid.s
+++ b/llvm/test/MC/RISCV/rvi-aliases-valid.s
@@ -190,6 +190,16 @@ jalr x25, x26, 11
 # CHECK-S-OBJ-NOALIAS: jalr zero, 0(ra)
 # CHECK-S-OBJ: ret
 ret
+# CHECK-S-OBJ-NOALIAS: jalr zero, 0(s11)
+# CHECK-S-OBJ: jr s11
+jr (x27)
+# CHECK-S-OBJ-NOALIAS: jalr ra, 0(t3)
+# CHECK-S-OBJ: jalr t3
+jalr (x28)
+# CHECK-S-OBJ-NOALIAS: jalr t4, 0(t5)
+# CHECK-S-OBJ: jalr t4, t5
+jalr x29, x30
+
 # TODO call
 # TODO tail
 

@kito-cheng
Copy link
Member

Wow, Sam, welcome back to RISC-V!

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lenary lenary merged commit bafff3e into llvm:main Jun 8, 2024
@lenary lenary deleted the aelliott/jr-aliases branch June 8, 2024 16:57
nekoshirro pushed a commit to nekoshirro/Alchemist-LLVM that referenced this pull request Jun 9, 2024
This broadly follows how in almost all places, we accept `(<reg>)` to
mean `0(<reg>)`, but I think these are the first like this for Jumps
rather than Loads/Stores. These are accepted by binutils but not by
LLVM: https://godbolt.org/z/GK7MGE7q7

Signed-off-by: Hafidz Muzakky <[email protected]>
@HerrCai0907 HerrCai0907 mentioned this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V llvm:mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants