This revision was automatically updated to reflect the committed changes.
Closed by commit rG2cb43b45713d: [ARM] Preserve fpu behaviour for 
'-crypto' (authored by dnsampaio).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67608

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-features.c


Index: clang/test/Driver/arm-features.c
===================================================================
--- clang/test/Driver/arm-features.c
+++ clang/test/Driver/arm-features.c
@@ -79,3 +79,18 @@
 // RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-m23+crypto -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
 // CHECK-NOCRYPTO5: warning: ignoring extension 'crypto' because the {{.*}} 
architecture does not support it
 // CHECK-NOCRYPTO5-NOT: "-target-feature" "+crypto"{{.*}} "-target-feature" 
"+sha2" "-target-feature" "+aes"
+//
+// Check +crypto does not affect -march=armv7a -mfpu=crypto-neon-fp-armv8, but 
it does warn that +crypto has no effect
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as -march=armv7a 
-mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefixes=CHECK-WARNONLY,ALL %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as -march=armv7a+aes 
-mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefixes=CHECK-WARNONLY,ALL,CHECK-HASAES %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as 
-march=armv7a+sha2 -mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefixes=CHECK-WARNONLY,ALL,CHECK-HASSHA %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as 
-march=armv7a+sha2+aes -mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefixes=CHECK-WARNONLY,ALL,CHECK-HASSHA,CHECK-HASAES %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as -march=armv7a+aes 
-mfpu=neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefixes=ALL,CHECK-HASAES %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as 
-march=armv7a+sha2 -mfpu=neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefixes=ALL,CHECK-HASSHA %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as 
-march=armv7a+sha2+aes -mfpu=neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefixes=ALL,CHECK-HASSHA,CHECK-HASAES %s
+// CHECK-WARNONLY: warning: ignoring extension 'crypto' because the 'armv7-a' 
architecture does not support it
+// ALL:     "-target-feature"
+// CHECK-WARNONLY-NOT: "-target-feature" "-crypto"
+// CHECK-HASSHA-SAME:  "-target-feature" "+sha2"
+// CHECK-HASAES-SAME:  "-target-feature" "+aes"
+//
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -479,24 +479,33 @@
 
   // For Arch >= ARMv8.0 && A profile:  crypto = sha2 + aes
   // FIXME: this needs reimplementation after the TargetParser rewrite
-  auto CryptoIt =
-    llvm::find_if(llvm::reverse(Features),
-                  [](const StringRef F) { return F.contains("crypto"); });
-  if (CryptoIt != Features.rend() && CryptoIt->take_front() == "+") {
-    StringRef ArchSuffix = arm::getLLVMArchSuffixForARM(
-        arm::getARMTargetCPU(CPUName, ArchName, Triple), ArchName, Triple);
-    if (llvm::ARM::parseArchVersion(ArchSuffix) >= 8 &&
-        llvm::ARM::parseArchProfile(ArchSuffix) == llvm::ARM::ProfileKind::A) {
-      if (ArchName.find_lower("+nosha2") == StringRef::npos &&
-          CPUName.find_lower("+nosha2") == StringRef::npos)
-        Features.push_back("+sha2");
-      if (ArchName.find_lower("+noaes") == StringRef::npos &&
-          CPUName.find_lower("+noaes") == StringRef::npos)
-        Features.push_back("+aes");
-    } else {
-      D.Diag(clang::diag::warn_target_unsupported_extension)
-        << "crypto" << 
llvm::ARM::getArchName(llvm::ARM::parseArch(ArchSuffix));
-      Features.push_back("-crypto");
+  auto CryptoIt = llvm::find_if(llvm::reverse(Features), [](const StringRef F) 
{
+    return F.contains("crypto");
+  });
+  if (CryptoIt != Features.rend()) {
+    if (CryptoIt->take_front() == "+") {
+      StringRef ArchSuffix = arm::getLLVMArchSuffixForARM(
+          arm::getARMTargetCPU(CPUName, ArchName, Triple), ArchName, Triple);
+      if (llvm::ARM::parseArchVersion(ArchSuffix) >= 8 &&
+          llvm::ARM::parseArchProfile(ArchSuffix) ==
+              llvm::ARM::ProfileKind::A) {
+        if (ArchName.find_lower("+nosha2") == StringRef::npos &&
+            CPUName.find_lower("+nosha2") == StringRef::npos)
+          Features.push_back("+sha2");
+        if (ArchName.find_lower("+noaes") == StringRef::npos &&
+            CPUName.find_lower("+noaes") == StringRef::npos)
+          Features.push_back("+aes");
+      } else {
+        D.Diag(clang::diag::warn_target_unsupported_extension)
+            << "crypto"
+            << llvm::ARM::getArchName(llvm::ARM::parseArch(ArchSuffix));
+        // With -fno-integrated-as -mfpu=crypto-neon-fp-armv8 some assemblers 
such as the GNU assembler
+        // will permit the use of crypto instructions as the fpu will override 
the architecture.
+        // We keep the crypto feature in this case to preserve compatibility.
+        // In all other cases we remove the crypto feature.
+        if (!Args.hasArg(options::OPT_fno_integrated_as))
+          Features.push_back("-crypto");
+      }
     }
   }
 


Index: clang/test/Driver/arm-features.c
===================================================================
--- clang/test/Driver/arm-features.c
+++ clang/test/Driver/arm-features.c
@@ -79,3 +79,18 @@
 // RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-m23+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
 // CHECK-NOCRYPTO5: warning: ignoring extension 'crypto' because the {{.*}} architecture does not support it
 // CHECK-NOCRYPTO5-NOT: "-target-feature" "+crypto"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes"
+//
+// Check +crypto does not affect -march=armv7a -mfpu=crypto-neon-fp-armv8, but it does warn that +crypto has no effect
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as -march=armv7a -mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefixes=CHECK-WARNONLY,ALL %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as -march=armv7a+aes -mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefixes=CHECK-WARNONLY,ALL,CHECK-HASAES %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as -march=armv7a+sha2 -mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefixes=CHECK-WARNONLY,ALL,CHECK-HASSHA %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as -march=armv7a+sha2+aes -mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefixes=CHECK-WARNONLY,ALL,CHECK-HASSHA,CHECK-HASAES %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as -march=armv7a+aes -mfpu=neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefixes=ALL,CHECK-HASAES %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as -march=armv7a+sha2 -mfpu=neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefixes=ALL,CHECK-HASSHA %s
+// RUN: %clang -target arm-none-none-eabi -fno-integrated-as -march=armv7a+sha2+aes -mfpu=neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefixes=ALL,CHECK-HASSHA,CHECK-HASAES %s
+// CHECK-WARNONLY: warning: ignoring extension 'crypto' because the 'armv7-a' architecture does not support it
+// ALL:     "-target-feature"
+// CHECK-WARNONLY-NOT: "-target-feature" "-crypto"
+// CHECK-HASSHA-SAME:  "-target-feature" "+sha2"
+// CHECK-HASAES-SAME:  "-target-feature" "+aes"
+//
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -479,24 +479,33 @@
 
   // For Arch >= ARMv8.0 && A profile:  crypto = sha2 + aes
   // FIXME: this needs reimplementation after the TargetParser rewrite
-  auto CryptoIt =
-    llvm::find_if(llvm::reverse(Features),
-                  [](const StringRef F) { return F.contains("crypto"); });
-  if (CryptoIt != Features.rend() && CryptoIt->take_front() == "+") {
-    StringRef ArchSuffix = arm::getLLVMArchSuffixForARM(
-        arm::getARMTargetCPU(CPUName, ArchName, Triple), ArchName, Triple);
-    if (llvm::ARM::parseArchVersion(ArchSuffix) >= 8 &&
-        llvm::ARM::parseArchProfile(ArchSuffix) == llvm::ARM::ProfileKind::A) {
-      if (ArchName.find_lower("+nosha2") == StringRef::npos &&
-          CPUName.find_lower("+nosha2") == StringRef::npos)
-        Features.push_back("+sha2");
-      if (ArchName.find_lower("+noaes") == StringRef::npos &&
-          CPUName.find_lower("+noaes") == StringRef::npos)
-        Features.push_back("+aes");
-    } else {
-      D.Diag(clang::diag::warn_target_unsupported_extension)
-        << "crypto" << llvm::ARM::getArchName(llvm::ARM::parseArch(ArchSuffix));
-      Features.push_back("-crypto");
+  auto CryptoIt = llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
+    return F.contains("crypto");
+  });
+  if (CryptoIt != Features.rend()) {
+    if (CryptoIt->take_front() == "+") {
+      StringRef ArchSuffix = arm::getLLVMArchSuffixForARM(
+          arm::getARMTargetCPU(CPUName, ArchName, Triple), ArchName, Triple);
+      if (llvm::ARM::parseArchVersion(ArchSuffix) >= 8 &&
+          llvm::ARM::parseArchProfile(ArchSuffix) ==
+              llvm::ARM::ProfileKind::A) {
+        if (ArchName.find_lower("+nosha2") == StringRef::npos &&
+            CPUName.find_lower("+nosha2") == StringRef::npos)
+          Features.push_back("+sha2");
+        if (ArchName.find_lower("+noaes") == StringRef::npos &&
+            CPUName.find_lower("+noaes") == StringRef::npos)
+          Features.push_back("+aes");
+      } else {
+        D.Diag(clang::diag::warn_target_unsupported_extension)
+            << "crypto"
+            << llvm::ARM::getArchName(llvm::ARM::parseArch(ArchSuffix));
+        // With -fno-integrated-as -mfpu=crypto-neon-fp-armv8 some assemblers such as the GNU assembler
+        // will permit the use of crypto instructions as the fpu will override the architecture.
+        // We keep the crypto feature in this case to preserve compatibility.
+        // In all other cases we remove the crypto feature.
+        if (!Args.hasArg(options::OPT_fno_integrated_as))
+          Features.push_back("-crypto");
+      }
     }
   }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to