[PATCH] D36731: [ARM][AArch64] Cortex-A75 and Cortex-A55 support

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

"They also implement the RCpc AArch64 extension from ARMv8.3-A."

Perhaps you need to explain why a v8.2 core implements a v8.3 extension?



================
Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:92
+                                     std::vector<StringRef> &Features) {
+  if (CPU != "generic") {
+    llvm::ARM::ArchKind ArchKind = llvm::ARM::parseCPUArch(CPU);
----------------
Nit: just for readability I would prefer an early exit:

if (CPU == "generic")
   return false;


================
Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:102
+  return false;
+}
+
----------------
Nit: you're not checking the return value (so you could simplify this function, but I don't have a strong opinion on this). 


================
Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:348
             Args.MakeArgString((F.second ? "+" : "-") + F.first()));
+  } else if (!CPUName.empty() && CPUName != "generic") {
+    DecodeARMFeaturesFromCPU(D, CPUName, Features);
----------------
rengolin wrote:
> Isn't this conditional redundant with what the function does?
CPUName != "generic" is also checked in function DecodeARMFeaturesFromCPU.


================
Comment at: test/Driver/arm-cortex-cpus.c:261
 // RUN: %clang -target arm -mlittle-endian -march=armv8.2-a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V82A %s
-// CHECK-V82A: "-cc1"{{.*}} "-triple" "armv8.2{{.*}}" "-target-cpu" "generic"
+// CHECK-V82A: "-cc1"{{.*}} "-triple" "armv8.2{{.*}}" "-target-cpu" "cortex-a55"
 
----------------
Just checking: why has the default cpu changed from generic to cortex-a55, and secondly, is that what we want?


https://reviews.llvm.org/D36731