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

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Support/Host.cpp:1320
+  int Error = sysctlbyname("hw.cpufamily", &Family, &Length, NULL, 0);
+  assert(!Error && "Fetching hw.cpufamily failed");
 
----------------
Not sure this should be an assert? I think other implementations will fall back on generic on errors (e.g. the things that read /proc/cpuinfo will use "" on error for the file contents and that gets parsed as generic by the various default cases).

Also technically you need to check Length is still sizeof(Family) and not smaller (the other direction is an error), but nobody ever bothers so meh.


================
Comment at: llvm/lib/Support/Host.cpp:1326
+  case CPUFAMILY_ARM_CYCLONE:
+    return "apple-a7";
+  case CPUFAMILY_ARM_TYPHOON:
----------------
ARM only calls it cyclone, AArch64 allows both but apple-a7 is the canonical name (the comment above the cyclone alias states it's for old bitcode compatibility)


================
Comment at: llvm/lib/Support/Host.cpp:1340
+  case CPUFAMILY_ARM_FIRESTORM_ICESTORM:
+    return "apple-m1";
+  default:
----------------
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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119788