[PATCH] D102494: [Clang, Driver] Default to Darwin_libsystem_m veclib on iOS based targets.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2492
+  // Darwin_libsystem_m for iOS based targets.
+  if (isTargetIOSBased() && !DriverArgs.hasArgNoClaim(options::OPT_fveclib))
+    CC1Args.push_back("-fveclib=Darwin_libsystem_m");
----------------
scanon wrote:
> fhahn wrote:
> > arphaman wrote:
> > > Is this applicable to the watchOS targets as well? The iOS based check doesn't cover it.
> > `libsystem_m`'s vector functions should be available on all Darwin platforms I think. I'd gradually opt-in additional platforms, once we verified it is clearly beneficial for each platform individually.
> > 
> > Should I add a TODO?
> Correct, available on all Darwin systems. These APIs were introduced in macOS 10.13, iOS 11.0, watchOS 4.0, tvOS 11.0, and driverkit 19.0. I think we need a check that the target is at least those versions somewhere?
Yes we should check if the target version is supported in the Driver and error out if it's used for an earlier OS.


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2492
+  // Darwin_libsystem_m for iOS based targets.
+  if (isTargetIOSBased() && !DriverArgs.hasArgNoClaim(options::OPT_fveclib))
+    CC1Args.push_back("-fveclib=Darwin_libsystem_m");
----------------
arphaman wrote:
> scanon wrote:
> > fhahn wrote:
> > > arphaman wrote:
> > > > Is this applicable to the watchOS targets as well? The iOS based check doesn't cover it.
> > > `libsystem_m`'s vector functions should be available on all Darwin platforms I think. I'd gradually opt-in additional platforms, once we verified it is clearly beneficial for each platform individually.
> > > 
> > > Should I add a TODO?
> > Correct, available on all Darwin systems. These APIs were introduced in macOS 10.13, iOS 11.0, watchOS 4.0, tvOS 11.0, and driverkit 19.0. I think we need a check that the target is at least those versions somewhere?
> Yes we should check if the target version is supported in the Driver and error out if it's used for an earlier OS.
Sure, a FIXME comment for the other platform support is fine for now.


================
Comment at: clang/test/Driver/darwin-veclib-default.c:9
+
+// RUN: %clang -target arm64-apple-darwinos -S -### %s -arch arm64 2>&1 | \
+// RUN:   FileCheck --check-prefix CHECK-IOS-DEFAULT %s
----------------
fhahn wrote:
> arphaman wrote:
> > `darwinos` isn't a valid OS type, what's your intent with this testcase here?
> Ah OK! Would it make sense to check `-darwin`? Or just `-iOS`?
Just `ios`. You're already checking `darwin` above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102494