[RISCV] Allow crypto features to imply dependents

[workingjubilee]

This relationship is a logical dependency.

Note Zvbc and Zvknhb. They are explicitly called out in the spec as requiring 64 bits:

• https://github.com/riscv/riscv-crypto/blob/56ed7952d13eb5bdff92e2b522404668952f416d/doc/vector/riscv-crypto-spec-vector.adoc

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Jubilee (workingjubilee)

Changes

This relationship is a logical dependency.

Note Zvbc and Zvknhb. They are explicitly called out in the spec as requiring 64 bits:

• https://github.com/riscv/riscv-crypto/blob/56ed7952d13eb5bdff92e2b522404668952f416d/doc/vector/riscv-crypto-spec-vector.adoc

---

Full diff: https://github.com/llvm/llvm-project/pull/112659.diff

1 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVFeatures.td (+8)

diff --git a/llvm/lib/Target/RISCV/RISCVFeatures.td b/llvm/lib/Target/RISCV/RISCVFeatures.td
index 3d0e1dae801d39..aa6fd701997cba 100644
--- a/llvm/lib/Target/RISCV/RISCVFeatures.td
+++ b/llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -734,6 +734,7 @@ def HasStdExtZfhOrZvfh
 def FeatureStdExtZvkb
     : RISCVExtension<"zvkb", 1, 0,
                      "'Zvkb' (Vector Bit-manipulation used in Cryptography)">,
+                     [FeatureStdExtZve32x]>,
       RISCVExtensionBitmask<0, 52>;
 def HasStdExtZvkb : Predicate<"Subtarget->hasStdExtZvkb()">,
                     AssemblerPredicate<(all_of FeatureStdExtZvkb),
@@ -751,6 +752,7 @@ def HasStdExtZvbb : Predicate<"Subtarget->hasStdExtZvbb()">,
 def FeatureStdExtZvbc
     : RISCVExtension<"zvbc", 1, 0,
                      "'Zvbc' (Vector Carryless Multiplication)">,
+                     [FeatureStdExtZve64x]>,
       RISCVExtensionBitmask<0, 49>;
 def HasStdExtZvbc : Predicate<"Subtarget->hasStdExtZvbc()">,
                     AssemblerPredicate<(all_of FeatureStdExtZvbc),
@@ -767,6 +769,7 @@ def HasStdExtZvbcOrZvbc32e : Predicate<"Subtarget->hasStdExtZvbc() || Subtarget-
 def FeatureStdExtZvkg
     : RISCVExtension<"zvkg", 1, 0,
                      "'Zvkg' (Vector GCM instructions for Cryptography)">,
+                     [FeatureStdExtZve32x]>,
       RISCVExtensionBitmask<0, 53>;
 def HasStdExtZvkg : Predicate<"Subtarget->hasStdExtZvkg()">,
                     AssemblerPredicate<(all_of FeatureStdExtZvkg),
@@ -783,6 +786,7 @@ def HasStdExtZvkgs : Predicate<"Subtarget->hasStdExtZvkgs()">,
 def FeatureStdExtZvkned
     : RISCVExtension<"zvkned", 1, 0,
                      "'Zvkned' (Vector AES Encryption & Decryption (Single Round))">,
+                     [FeatureStdExtZve32x]>,
       RISCVExtensionBitmask<0, 54>;
 def HasStdExtZvkned : Predicate<"Subtarget->hasStdExtZvkned()">,
                       AssemblerPredicate<(all_of FeatureStdExtZvkned),
@@ -791,6 +795,7 @@ def HasStdExtZvkned : Predicate<"Subtarget->hasStdExtZvkned()">,
 def FeatureStdExtZvknha
     : RISCVExtension<"zvknha", 1, 0,
                      "'Zvknha' (Vector SHA-2 (SHA-256 only))">,
+                     [FeatureStdExtZve32x]>,
       RISCVExtensionBitmask<0, 55>;
 def HasStdExtZvknha : Predicate<"Subtarget->hasStdExtZvknha()">,
                       AssemblerPredicate<(all_of FeatureStdExtZvknha),
@@ -799,6 +804,7 @@ def HasStdExtZvknha : Predicate<"Subtarget->hasStdExtZvknha()">,
 def FeatureStdExtZvknhb
     : RISCVExtension<"zvknhb", 1, 0,
                      "'Zvknhb' (Vector SHA-2 (SHA-256 and SHA-512))">,
+                     [FeatureStdExtZve64x]>,
       RISCVExtensionBitmask<0, 56>;
 def HasStdExtZvknhb : Predicate<"Subtarget->hasStdExtZvknhb()">,
                       AssemblerPredicate<(all_of FeatureStdExtZvknhb),
@@ -811,6 +817,7 @@ def HasStdExtZvknhaOrZvknhb : Predicate<"Subtarget->hasStdExtZvknha() || Subtarg
 def FeatureStdExtZvksed
     : RISCVExtension<"zvksed", 1, 0,
                      "'Zvksed' (SM4 Block Cipher Instructions)">,
+                     [FeatureStdExtZve32x]>,
       RISCVExtensionBitmask<0, 57>;
 def HasStdExtZvksed : Predicate<"Subtarget->hasStdExtZvksed()">,
                       AssemblerPredicate<(all_of FeatureStdExtZvksed),
@@ -819,6 +826,7 @@ def HasStdExtZvksed : Predicate<"Subtarget->hasStdExtZvksed()">,
 def FeatureStdExtZvksh
     : RISCVExtension<"zvksh", 1, 0,
                      "'Zvksh' (SM3 Hash Function Instructions)">,
+                     [FeatureStdExtZve32x]>,
       RISCVExtensionBitmask<0, 58>;
 def HasStdExtZvksh : Predicate<"Subtarget->hasStdExtZvksh()">,
                      AssemblerPredicate<(all_of FeatureStdExtZvksh),

[workingjubilee]

This was brought to attention by rust-lang/rust#131800

One could make some argument this is the job of the frontend, I think, and instead the frontend should satisfy this dependency! ...but it would still be preferable to encode this relationship in the .td, so the dependency check can be generated.

[topperc]

instead the frontend should satisfy this dependency!

The .td file also generates code included by RISCVISAInfo.cpp used by clang to make -march imply the dependencies. This is needed to set preprocessor defines correctly.

[wangpc-pp]

Do we need to change RISCVISAInfo::checkDependency()?

[lukel97]

Does zvkt depend on zve32x?

[topperc]

Does zvkt depend on zve32x?

It doesn't in gcc.

[workingjubilee]

According to the ISA spec so far, at least as far as I understand it, Zvkt doesn't imply any instructions. It thus doesn't imply that any register state must be addressable, either. It just specifies that other instructions should be modified in behavior if they are executed. Thus, perversely, Zvkt could be "implemented" (in the sense of being detectable) on a processor that implements zero of the instructions that Zvkt modifies.

[topperc]

LGTM

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[workingjubilee]

Not sure why it didn't whine when I ran it locally...?

[workingjubilee]

Do we need to change RISCVISAInfo::checkDependency()?

...Probably? But because LLVM believes "negative features" are a sensible notion, I know there are some absolutely wacky edge-cases in feature resolution which prevent me from feeling very confident in removing any code from that. The fact that in normal usage the errors are simply naturally silenced by correctly resolving the features makes the 5 LOC that would be clawed back seem basically inconsequential.

Believe me, the thought of deleting more code greatly tempts me.

[topperc]

Do we need to change RISCVISAInfo::checkDependency()?

...Probably? But because LLVM believes "negative features" are a sensible notion, I know there are some absolutely wacky edge-cases in feature resolution which prevent me from feeling very confident in removing any code from that. The fact that in normal usage the errors are simply naturally silenced by correctly resolving the features makes the 5 LOC that would be clawed back seem basically inconsequential.

Believe me, the thought of deleting more code greatly tempts me.

Let's go ahead and land this. I'll consider whether we can simplify RISCVISAInfo::checkDependency() separately.