[PATCH] D50046: [RISCV] Add InstAlias definitions for add, and, xor, or, sll, srl, sra, slt and sltu with immediate
Alex Bradbury via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 1 08:36:30 PDT 2018
- Previous message: [PATCH] D50046: [RISCV] Add InstAlias definitions for add, and, xor, or, sll, srl, sra, slt and sltu with immediate
- Next message: [PATCH] D50046: [RISCV] Add InstAlias definitions for add[w], and, xor, or, sll[w], srl[w], sra[w], slt and sltu with immediate.
- Messages sorted by:
[ date ]
[ thread ]
[ subject ]
[ author ]
asb added a comment.
Thanks Kito, I've added a couple of comments. I think the main issue is the pattern for the shift amount immediate, which means invalid assembly is accepted.
================
Comment at: lib/Target/RISCV/RISCVInstrInfo.td:562-567
+def : InstAlias<"sll $rd, $rs1, $imm12",
+ (SLLI GPR:$rd, GPR:$rs1, simm12:$imm12)>;
+def : InstAlias<"srl $rd, $rs1, $imm12",
+ (SRLI GPR:$rd, GPR:$rs1, simm12:$imm12)>;
+def : InstAlias<"sra $rd, $rs1, $imm12",
+ (SRAI GPR:$rd, GPR:$rs1, simm12:$imm12)>;
----------------
These should use uimmlog2xlen for the shift amount. When fixing this, could you please also add tests that demonstrate the range checks are correct (adding to rv32i-aliases-invalid.s and rv64i-aliases-invalid.s)?
================
Comment at: test/MC/RISCV/rvi-aliases-valid.s:179
+
+# CHECK-INST: srli a2, a3, 4
+# CHECK-ALIAS: srli a2, a3, 4
----------------
kito-cheng wrote:
> sabuasal wrote:
> > This test should print the canonical version of the instruction, you are printing the Alias.
> Those alias instructions always disassemble to **i** version in GAS even -M no-aliases is given, my thought is keep the behavior with GAS.
Yes, we should match gas here. Perhaps add a comment to this test like "The following aliases are accepted as input but the canonical form of the instruction will always be printed."
https://reviews.llvm.org/D50046
- Previous message: [PATCH] D50046: [RISCV] Add InstAlias definitions for add, and, xor, or, sll, srl, sra, slt and sltu with immediate
- Next message: [PATCH] D50046: [RISCV] Add InstAlias definitions for add[w], and, xor, or, sll[w], srl[w], sra[w], slt and sltu with immediate.
- Messages sorted by:
[ date ]
[ thread ]
[ subject ]
[ author ]
More information about the llvm-commits
mailing list