[PATCH] D131141: [RISCV] Add MC support of RISCV Zcb Extension

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCV.td:368
+def HasStdExtZcb : Predicate<"Subtarget->hasStdExtZcb()">,
+                           AssemblerPredicate<(all_of FeatureExtZcb),
+                           "'Zcb' (Shortened format for basic bit manipulation instructions)">;
----------------
The `A` should be lined up with the `"` on the line above.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:170
+                  (C_MUL GPRC:$rs1, GPRC:$rs2)>;
+def : CompressPat<(MUL GPRC:$rs1, GPRC:$rs2, GPRC:$rs1),
+                  (C_MUL GPRC:$rs1, GPRC:$rs2)>;
----------------
The second CompressPat needs `let isCompressOnly = true in`. See the CompressPats for C_ADD.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:222
+let Predicates = [HasStdExtZcb] in {
+def : InstAlias<"c.lbu $rd, (${rs1})",(C_LW GPRC:$rd, GPRC:$rs1, 0)>;
+def : InstAlias<"c.lhu $rd, (${rs1})",(C_LW GPRC:$rd, GPRC:$rs1, 0)>;
----------------
I think C_LW the wrong instruction?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:222
+let Predicates = [HasStdExtZcb] in {
+def : InstAlias<"c.lbu $rd, (${rs1})",(C_LW GPRC:$rd, GPRC:$rs1, 0)>;
+def : InstAlias<"c.lhu $rd, (${rs1})",(C_LW GPRC:$rd, GPRC:$rs1, 0)>;
----------------
craig.topper wrote:
> I think C_LW the wrong instruction?
Do you have tests for these patterns?


================
Comment at: llvm/test/MC/RISCV/rv32zcb-valid.s:21
+# CHECK-ASM: encoding: [0x61,0x9c]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcb' (Shortened format for basic bit manipulation instructions)
+c.zext.b s0
----------------
Needs `{{$}}` at the end of the line.


================
Comment at: llvm/test/MC/RISCV/rv32zcb-valid.s:96
+# CHECK-ASM: encoding: [0x61,0x9c]
+ANDI s0, s0, 255
+
----------------
ANDI -> andi


================
Comment at: llvm/test/MC/RISCV/rv32zcb-valid.s:144
+# CHECK-ASM: encoding: [0x13,0x04,0x04,0x00]
+ADDI s0, s0, 0
+
----------------
ADDI -> addi

But what is this testing?


================
Comment at: llvm/test/MC/RISCV/rv32zcb-valid.s:148
+# CHECK-ASM: encoding: [0x13,0x44,0x14,0x00]
+xori s0, s0, 1
----------------
What is this testing?


================
Comment at: llvm/test/MC/RISCV/rv64zcb-valid.s:16
+# CHECK-ASM: encoding: [0x71,0x9c]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zba' (Address Generation Instructions), 'Zcb' (Shortened format for basic bit manipulation instructions) 
+# CHECK-NO-RV64: error: instruction requires the following: RV64I Base Instruction Set
----------------
Need `{{$}}` at the end of the line.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131141/new/

https://reviews.llvm.org/D131141