Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV][NFC] Remove rdty arg of PseudoLoad and the default rdty value of PseudoFloatLoad #67014

Merged
merged 3 commits into from
Sep 22, 2023

Conversation

wangpc-pp
Copy link
Contributor

@wangpc-pp wangpc-pp commented Sep 21, 2023

rdty of PseudoLoad is always GPR and it will never be GPR for
PseudoFloatLoad.

… of PseudoFloatLoad

`rdty` of `PseudoLoad` is always `GPR` and it will never be `GPR` for
`PseudoFloatLoad`.

And `PseudoLoad` is renamed to `PseudoIntLoad` to be symmetric.
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 21, 2023

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

Changes

rdty of PseudoLoad is always GPR and it will never be GPR for
PseudoFloatLoad.

And PseudoLoad is renamed to PseudoIntLoad to be symmetric.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInstrFormats.td (+3-3)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.td (+7-7)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrFormats.td b/llvm/lib/Target/RISCV/RISCVInstrFormats.td
index f19a0b356aafbca..0cab8d4c9843b27 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrFormats.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrFormats.td
@@ -241,8 +241,8 @@ class PseudoQuietFCMP<DAGOperand Ty>
 }
 
 // Pseudo load instructions.
-class PseudoLoad<string opcodestr, RegisterClass rdty = GPR>
-    : Pseudo<(outs rdty:$rd), (ins bare_symbol:$addr), [], opcodestr, "$rd, $addr"> {
+class PseudoIntLoad<string opcodestr>
+    : Pseudo<(outs GPR:$rd), (ins bare_symbol:$addr), [], opcodestr, "$rd, $addr"> {
   let hasSideEffects = 0;
   let mayLoad = 1;
   let mayStore = 0;
@@ -250,7 +250,7 @@ class PseudoLoad<string opcodestr, RegisterClass rdty = GPR>
   let isAsmParserOnly = 1;
 }
 
-class PseudoFloatLoad<string opcodestr, RegisterClass rdty = GPR>
+class PseudoFloatLoad<string opcodestr, RegisterClass rdty>
     : Pseudo<(outs GPR:$tmp, rdty:$rd), (ins bare_symbol:$addr), [], opcodestr, "$rd, $addr, $tmp"> {
   let hasSideEffects = 0;
   let mayLoad = 1;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index abbeff78b6e2864..0bf3ca1df758d56 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -920,19 +920,19 @@ let hasSideEffects = 0, mayLoad = 0, mayStore = 0, Size = 32,
 def PseudoLI : Pseudo<(outs GPR:$rd), (ins ixlenimm_li:$imm), [],
                       "li", "$rd, $imm">;
 
-def PseudoLB  : PseudoLoad<"lb">;
-def PseudoLBU : PseudoLoad<"lbu">;
-def PseudoLH  : PseudoLoad<"lh">;
-def PseudoLHU : PseudoLoad<"lhu">;
-def PseudoLW  : PseudoLoad<"lw">;
+def PseudoLB  : PseudoIntLoad<"lb">;
+def PseudoLBU : PseudoIntLoad<"lbu">;
+def PseudoLH  : PseudoIntLoad<"lh">;
+def PseudoLHU : PseudoIntLoad<"lhu">;
+def PseudoLW  : PseudoIntLoad<"lw">;
 
 def PseudoSB  : PseudoStore<"sb">;
 def PseudoSH  : PseudoStore<"sh">;
 def PseudoSW  : PseudoStore<"sw">;
 
 let Predicates = [IsRV64] in {
-def PseudoLWU : PseudoLoad<"lwu">;
-def PseudoLD  : PseudoLoad<"ld">;
+def PseudoLWU : PseudoIntLoad<"lwu">;
+def PseudoLD  : PseudoIntLoad<"ld">;
 def PseudoSD  : PseudoStore<"sd">;
 } // Predicates = [IsRV64]
 

Copy link
Contributor

@asb asb left a comment

Choose a reason for hiding this comment

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

What do you think about leaving PseudoLoad named as it is for now? Having PseudoIntLoad but PseudoStore is inconsistent, and I think PseudoLoad+PseudoStore is fine, as that corresponds to the naming of the load/store instructions in RISC-V (referred to as just load and store, rather than integer load and store).

@wangpc-pp
Copy link
Contributor Author

What do you think about leaving PseudoLoad named as it is for now? Having PseudoIntLoad but PseudoStore is inconsistent, and I think PseudoLoad+PseudoStore is fine, as that corresponds to the naming of the load/store instructions in RISC-V (referred to as just load and store, rather than integer load and store).

Thanks! Make sense to me!

Copy link
Contributor

@asb asb left a comment

Choose a reason for hiding this comment

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

LGTM.

@wangpc-pp wangpc-pp merged commit 079b8ba into llvm:main Sep 22, 2023
1 of 2 checks passed
@wangpc-pp wangpc-pp deleted the main-pseudo-load-store branch September 22, 2023 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants