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

[Renato Golin via Phabricator via cfe-commits]

rengolin added a comment.

This change seems to have nothing to do with adding core support but exposing features from CPU names.

If this was required to "support" the new cores in Clang, why wasn't it needed before?

If this is a new, more generic way, of getting support, shouldn't you remove the specialist code that already does that for all other cores?

cheers,
--renato



================
Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:90
 
+static bool DecodeARMFeaturesFromCPU(const Driver &D, StringRef CPU,
+                                     std::vector<StringRef> &Features) {
----------------
Why are you returning bool if you're not using the result?


================
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);
----------------
Isn't this conditional redundant with what the function does?


https://reviews.llvm.org/D36731