[PATCH] D141672: [RISCV] Support vector crypto extension ISA string and assembly

Mon Mar 13 15:53:50 PDT 2023

craig.topper added inline comments.
Herald added a subscriber: jobnoorman.

================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:820
+        errc::invalid_argument,
+        "zvk* requires v or zve* extension to also be specified");
+
----------------
Put single quotes around extension names in the error messages. I fixed the other error messages last week

================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:495
+                       "'Zvknhb' (Vector SHA-2. (SHA-256 and SHA-512))",
+                       [FeatureStdExtZvknha]>;
+def HasStdExtZvknhaOrZvknhb  : Predicate<"Subtarget->hasStdExtZvknha() || Subtarget->hasStdExtZvknhb()">,
----------------
Zvknhb implies Zvknha here it needs to imply it in RISCVISAInfo.cpp too.

================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:496
+                       [FeatureStdExtZvknha]>;
+def HasStdExtZvknhaOrZvknhb  : Predicate<"Subtarget->hasStdExtZvknha() || Subtarget->hasStdExtZvknhb()">,
+                                AssemblerPredicate<(any_of FeatureStdExtZvknha, FeatureStdExtZvknhb),
----------------
Line 495 has Zvknhb implying Zvknha which would make it sufficient to only check HasStdExtZknha. I'm not sure if this code should be changed or the implies at line 495.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141672