[PATCH] D142548: [AArch64] Replace AEK_CRYPTO with relevant features in cpu definitions

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In D142548#4082255 <https://reviews.llvm.org/D142548#4082255>, @lenary wrote:

> Thanks, I had been thinking this was the right way to go.
>
> I was partly wondering what we should do about `FeatureCrypto` in AArch64.td - the two things that came to mind were either to split it into `FeatureCrypto84` and `FeatureCrypto82` (names tbd), or do this full split and make FeatureCrypto do absolutely nothing (with a big comment as to why). Thoughts?

I think the current comment for FeatureCrypto match what I had thought it should do closely enough:

  // Crypto has been split up and any combination is now valid (see the
  // crypto definitions above). Also, crypto is now context sensitive:
  // it has a different meaning for e.g. Armv8.4 than it has for Armv8.2.
  // Therefore, we rely on Clang, the user interacing tool, to pass on the
  // appropriate crypto options. But here in the backend, crypto has very little
  // meaning anymore. We kept the Crypto definition here for backward
  // compatibility, and now imply features SHA2 and AES, which was the
  // "traditional" meaning of Crypto.
  def FeatureCrypto : SubtargetFeature<"crypto", "HasCrypto", "true",
    "Enable cryptographic instructions", [FeatureNEON, FeatureSHA2, FeatureAES]>;

i.e. It is there for backwards compatibility but best not to use directly. I think this works OK as-is, especially if clang stops setting it so much.

(I'm not sure that +crypto should really mean different things for 8.4, as in reality a number of cpus have come out that don't follow it, and GCC didn't implement the same thing. I don't have a very strong opinion on it though)


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

https://reviews.llvm.org/D142548