[PATCH] D117640: [RISCV][RFC] add inst support of zbkb

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZb.td:613
+let Predicates = [HasStdExtZbkb] in 
+def BREV8 : RVBUnary<0b0110100, 0b00111, 0b101, OPC_OP_IMM, "brev8">;
+
----------------
Interesting. GREVI called this rev.b rather than brev8. The similar single encoding of gorci in Zbb is orc.b. That seems inconsistent was that discussed during the review process.

I think we should change this predicate to HasStdExtZbpOrZbkb and make the rev.b InstAlias point to the BREV8 instruction instead of GREVI, but the rev.b alias should only be enabled with Zbp.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZb.td:615
+
+let Predicates = [HasStdExtZbkb, IsRV32] in {
+def ZIP   : RVBUnary<0b0000100, 0b01111, 0b001, OPC_OP_IMM, "zip">;
----------------
This should be HasStdExtZbpOrZbkb. The zip and unzip aliases need to be removed from the [HasStdExtZbp, IsRV32] section later in the file.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZb.td:616
+let Predicates = [HasStdExtZbkb, IsRV32] in {
+def ZIP   : RVBUnary<0b0000100, 0b01111, 0b001, OPC_OP_IMM, "zip">;
+def UNZIP : RVBUnary<0b0000100, 0b01111, 0b101, OPC_OP_IMM, "unzip">;
----------------
ZIP->ZIP_RV32
UNZIP->UNZIP_RV32

If we ever need to add RV64 versions, they will have different encodings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117640