[PATCH] D119788: [AArch64] Add support for -march=native for Apple M1 CPU

Fri Feb 18 15:04:47 PST 2022

ab added a comment.

In D119788#3331324 <https://reviews.llvm.org/D119788#3331324>, @keith wrote:

> Thanks for the tips! I left the default as generic here, but I totally take your point on defaulting to the newest and I'm happy to update if you feel strongly, lmk what you think

I agree with Tim, this should default to the newest known one.

generic doesn't really make sense for Darwin anyway, the "oldest" sane default would probably be cyclone, apple-a12, or apple-m1, depending on the slice and OS

================
Comment at: llvm/lib/Support/Host.cpp:1303
+#elif defined(__APPLE__) && (defined(__arm__) || defined(__aarch64__))
+#include <sys/param.h>
+#include <sys/sysctl.h>
----------------
Looks like we have the same includes for `computeHostNumPhysicalCores`.  Maybe move this to the top of the file, gated with just `__APPLE__`?

================
Comment at: llvm/lib/Support/Host.cpp:1340
+  case CPUFAMILY_ARM_FIRESTORM_ICESTORM:
+    return "apple-m1";
+  default:
----------------
jrtc27 wrote:
> Maybe a question for AArch64 maintainers about whether apple-a14 or apple-m1 should be used... they're the same thing from the backend's perspective just with different names. Shame the naming doesn't separate out the "generation" from the specific configuration like you see for other backends (apple-aN is almost that as it omits all the bionic etc marketing name fluff, except apple-m1 comes along and ruins that).
Seems fine to stick to the `apple-aNN` names here.  (we already have a similar situation with `apple-sN`)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119788