[PATCH] D98136: [RISCV][RFC] Initially support the K-extension instructions on the LLVM MC layer

[Xinlong Wu via Phabricator via llvm-commits]

VincentWu added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoK.td:45
+        : RVInstI<funct3, OPC_OP_IMM, (outs GPR:$rd), (ins GPR:$rs1, uimm4:$rcon),
+                opcodestr, "$rd, $rs1, $rcon"> {
+        bits<4> rcon;
----------------
jrtc27 wrote:
> Isn't this the round, which is then used to derive the 8-bit round constant, not the round constant itself?
I'm a little confused about that. So do you mean number rcon is a kind of offset or something？and we need an extra base address?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoK.td:58
+    def SHA256SUM0 :    RVKUnary<0b0001000, 0b00000, 0b001, "sha256sum0">, 
+                        Sched<[]>;
+    def SHA256SUM1 :    RVKUnary<0b0001000, 0b00001, 0b001, "sha256sum1">, 
----------------
jrtc27 wrote:
> If you don't have scheduling defined yet just don't add `Sched<[]>` at all?
yes, but there will be something wrong whing building if I just remove the `Sched<[]>`. and we found that the RISCVInstrInfoB.td which is already merged also put an empty `Sched<[]>` there. So we put it there too.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoK.td:9
+///
+/// This file describes the RISC-K instructions from the standard 'K' 
+/// scalar cryptography extension, version 0.8.1. 
----------------
craig.topper wrote:
> Is this supposed to say RISC-V instead of RISC-K?
yes, it is a typo, my fault


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98136