[PATCH] D152793: [RISCV] Add MC layer support for Zicfiss.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: clang/test/Preprocessor/riscv-target-features.c:75
 // CHECK-NOT: __riscv_zvfbfwma {{.*$}}
+// CHECK-NOT: __riscv_zicfisslp {{.*$}}
 
----------------
This needs to be renamed to remove lp


================
Comment at: clang/test/Preprocessor/riscv-target-features.c:725
+// RUN: %clang -target riscv32 -menable-experimental-extensions \
+// RUN: -march=rv32izicfisslp0p1 -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-ZICFISSLP-EXT %s
----------------
This needs to be renamed to remove lp


================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:84
+    : SubtargetFeature<"experimental-zicfiss", "HasStdExtZicfiss", "true",
+                       "'zicfiss' (Shadow stack)">;
+def HasStdExtZicfiss : Predicate<"Subtarget->hasStdExtZicfiss()">,
----------------
Zicfiss


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:1921
 include "RISCVInstrInfoZicond.td"
+include "RISCVInstrInfoZicfiss.td"
 
----------------
This needs to be rebased, the files here were moved into groups.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZicfiss.td:1
+//===-- RISCVInstrInfoZ.td - RISC-V CFG -*- tablegen -*-===//
+//
----------------
Filename doesn't match


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZicfiss.td:35
+class RV_SSPop<bits<5> rd, bits<5> rs1, string opcodestr, string argstr> :
+  RVInst<(outs GPR:$rd), (ins GPR:$rs1), opcodestr, argstr, [], InstFormatI> {
+  let Inst{31-20} = 0b100000011100;
----------------
Can we use RVInstI by adding `let imm12 = 0b100000011100` to the body?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZicfiss.td:66
+let Uses = [SSP], hasSideEffects = 0, mayLoad = 0, mayStore = 0 in {
+def SSPINC : RVInst<(outs), (ins uimm5nonzero:$imm), "sspinc", "$imm", [], InstFormatI> {
+  bits<5> imm;
----------------
Can this use RVInstI with lets for imm12, rs1, and rd?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZicfiss.td:75
+
+def SSPRR : RVInst<(outs GPRNoX0:$rd), (ins), "ssprr", "$rd", [], InstFormatR> {
+  bits<5> rd;
----------------
Can this use RVInstR?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZicfiss.td:87
+let Uses = [SSP], Defs = [SSP], hasSideEffects = 0, mayLoad = 0, mayStore = 1 in {
+def SSPUSH : RVInst<(outs), (ins GPRRA:$rs2), "sspush", "$rs2", [], InstFormatR> {
+  bits<5> rs2;
----------------
Can this use RVInstR?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZicfiss.td:98
+def SSAMOSWAP :
+  RVInst<(outs GPRNoX0:$dest), (ins GPR:$addr, GPR:$src),
+         "ssamoswap", "$dest, $src, (${addr})", [], InstFormatR> {
----------------
RVInstR


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152793