[llvm] b8028f6 - [TargetParser][AArch64] Believe runtime feature detection (#95694)

[via llvm-commits]

Author: Jubilee
Date: 2024-10-02T08:28:57+01:00
New Revision: b8028f6b87dac837d65f18c1cd49dd4db543601a

URL: https://github.com/llvm/llvm-project/commit/b8028f6b87dac837d65f18c1cd49dd4db543601a
DIFF: https://github.com/llvm/llvm-project/commit/b8028f6b87dac837d65f18c1cd49dd4db543601a.diff

LOG: [TargetParser][AArch64] Believe runtime feature detection (#95694)

In https://github.com/llvm/llvm-project/issues/90365 it was reported
that TargetParser arrives at the wrong conclusion regarding what
features are enabled when attempting to detect "native" features on the
Raspberry Pi 4, because it (correctly) detects it as a Cortex-A72, but
LLVM (incorrectly) believes all Cortex-A72s have crypto enabled. Attempt
to help ourselves by allowing runtime information derived from the host
to contradict whatever we believe is "true" about the architecture.

Added: 
    

Modified: 
    llvm/lib/TargetParser/Host.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/TargetParser/Host.cpp b/llvm/lib/TargetParser/Host.cpp
index 616e4eda1dd29d..14567af8590e63 100644
--- a/llvm/lib/TargetParser/Host.cpp
+++ b/llvm/lib/TargetParser/Host.cpp
@@ -1898,7 +1898,8 @@ const StringMap<bool> sys::getHostCPUFeatures() {
     }
 
 #if defined(__aarch64__)
-  // Keep track of which crypto features we have seen
+  // All of these are "crypto" features, but we must sift out actual features
+  // as the former meaning of "crypto" as a single feature is no more.
   enum { CAP_AES = 0x1, CAP_PMULL = 0x2, CAP_SHA1 = 0x4, CAP_SHA2 = 0x8 };
   uint32_t crypto = 0;
 #endif
@@ -1941,9 +1942,13 @@ const StringMap<bool> sys::getHostCPUFeatures() {
   }
 
 #if defined(__aarch64__)
-  // If we have all crypto bits we can add the feature
-  if (crypto == (CAP_AES | CAP_PMULL | CAP_SHA1 | CAP_SHA2))
-    Features["crypto"] = true;
+  // LLVM has decided some AArch64 CPUs have all the instructions they _may_
+  // have, as opposed to all the instructions they _must_ have, so allow runtime
+  // information to correct us on that.
+  uint32_t Aes = CAP_AES | CAP_PMULL;
+  uint32_t Sha2 = CAP_SHA1 | CAP_SHA2;
+  Features["aes"] = (crypto & Aes) == Aes;
+  Features["sha2"] = (crypto & Sha2) == Sha2;
 #endif
 
   return Features;
@@ -1952,12 +1957,17 @@ const StringMap<bool> sys::getHostCPUFeatures() {
 const StringMap<bool> sys::getHostCPUFeatures() {
   StringMap<bool> Features;
 
-  if (IsProcessorFeaturePresent(PF_ARM_NEON_INSTRUCTIONS_AVAILABLE))
-    Features["neon"] = true;
-  if (IsProcessorFeaturePresent(PF_ARM_V8_CRC32_INSTRUCTIONS_AVAILABLE))
-    Features["crc"] = true;
-  if (IsProcessorFeaturePresent(PF_ARM_V8_CRYPTO_INSTRUCTIONS_AVAILABLE))
-    Features["crypto"] = true;
+  // If we're asking the OS at runtime, believe what the OS says
+  Features["neon"] =
+      IsProcessorFeaturePresent(PF_ARM_NEON_INSTRUCTIONS_AVAILABLE);
+  Features["crc"] =
+      IsProcessorFeaturePresent(PF_ARM_V8_CRC32_INSTRUCTIONS_AVAILABLE);
+
+  // Avoid inferring "crypto" means more than the traditional AES + SHA2
+  bool TradCrypto =
+      IsProcessorFeaturePresent(PF_ARM_V8_CRYPTO_INSTRUCTIONS_AVAILABLE);
+  Features["aes"] = TradCrypto;
+  Features["sha2"] = TradCrypto;
 
   return Features;
 }