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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:10
+/// This file describes the RISC-V instructions from the 'Zc' Code-size 
+/// reduction extension, version 0.24.
+/// This version is still experimental as the 'Zc' extension hasn't been
----------------
Is this the right version?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:53
+class RVZcArith_r<bits<2> funct2, bits<3> opcode, string opcodestr>
+    : RVInst16<(outs GPRC:$rs_wb), (ins GPRC:$rs), opcodestr, "$rs", [], InstFormatCB> {
+  bits<3> rs;
----------------
80 columns


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:67
+class RVZcArith_rr<bits<6> funct6,bits<2> funct2, bits<2> opcode, string opcodestr>
+    : RVInst16<(outs GPRC:$rs1_wb), (ins GPRC:$rs1, GPRC:$rs2), opcodestr, "$rs1, $rs2", [], InstFormatCB> {
+  bits<3> rs1;
----------------
80 columns


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:98
+let Predicates = [HasStdExtZcb, IsRV64] in {
+def C_ZEXT_W : RVZcArith_r<0b11, 0b100 , "c.zext.w">, Sched<[]>;
+}
----------------
Please fill out the scheduling information to match the non-compressed forms.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:102
+let Predicates = [HasStdExtZcb] in {
+def C_ZEXT_B  : RVZcArith_r<0b11, 0b000 , "c.zext.b">, Sched<[]>;
+def C_ZEXT_H  : RVZcArith_r<0b11, 0b010 , "c.zext.h">, Sched<[]>;
----------------
Does these require Zbb as well or does Zcb imply Zbb?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:108
+
+def C_MUL     : RVZcArith_rr<0b100111, 0b10, 0b01, "c.mul">, Sched<[]>;
+
----------------
Does this require M or Zmmul or does Zcb imply M/Zmmul?


================
Comment at: llvm/test/MC/RISCV/rv32zcb-Invalid.s:1
+# RUN: not llvm-mc -triple=riscv32 -mattr=experimental-zcb -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN:     | FileCheck -check-prefixes=CHECK-ERROR %s
----------------
MaskRay wrote:
> `< %s` => `%s`
Don't capitalize "Invalid" in the test name.


================
Comment at: llvm/test/MC/RISCV/rv64zcb-Invalid.s:1
+# RUN: not llvm-mc -triple=riscv64 -mattr=experimental-zcb -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN:     | FileCheck -check-prefixes=CHECK-ERROR %s
----------------
Don't capitalize Invalid in the test name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131141