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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to