[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2019-01-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

> I would be ok with reusing that option, as long as it's documented that there 
> is a difference in terms of how it can be used.

The patch is in https://reviews.llvm.org/D57487
It does not look like we're formally documenting CC1 options anywhere. I've 
added some comments next to SDKVersion in TargetOptions.h. PTAL.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673



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


[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2019-01-30 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

In D55673#1377404 , @tra wrote:

> Would that be OK to use target_sdk_version to pass *CUDA* SDK version to the 
> CC1 compilations?
>  I have upcoming changes that need to know the version to generate correct 
> glue IR for CUDA. The driver currently figures out detected CUDA version in 
> lib/Driver/ToolChains/Cuda.cpp and I could use -target-sdk-version to pass it 
> on to CC1 instances.
>
> On one hand CUDA is a target SDK and the option appears to be accessible 
> exactly where I need it. On the other hand, my use case is not *exactly* the 
> case `-target-sdk-version` was intended for (i.e. it's not darwin and it has 
> nothing to do with module metadata, though it *may* be eventually useful 
> there even for CUDA).
>
> If using the option for CUDA sounds like too much of a hack, I guess I can 
> add a separate `-cuda-sdk-version=`, though it would effectively replicate 
> some of this patch.
>
> Opinions?


I would be ok with reusing that option, as long as it's documented that there 
is a difference in terms of how it can be used.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673



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


[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2019-01-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

Would that be OK to use target_sdk_version to pass *CUDA* SDK version to the 
CC1 compilations?
I have upcoming changes that need to know the version to generate correct glue 
IR for CUDA. The driver currently figures out detected CUDA version in 
lib/Driver/ToolChains/Cuda.cpp and I could use -target-sdk-version to pass it 
on to CC1 instances.

On one hand CUDA is a target SDK and the option appears to be accessible 
exactly where I need it. On the other hand, my use case is not *exactly* the 
case `-target-sdk-version` was intended for (i.e. it's not darwin and it has 
nothing to do with module metadata, though it *may* be eventually useful there 
even for CUDA).

If using the option for CUDA sounds like too much of a hack, I guess I can add 
a separate `-cuda-sdk-version=`, though it would effectively replicate some of 
this patch.

Opinions?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673



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


[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-17 Thread Alex Lorenz via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC349380: [darwin] parse the SDK settings from 
SDKSettings.json if it exists and (authored by arphaman, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55673?vs=178309=178501#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55673

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/TargetInfo.h
  include/clang/Basic/TargetOptions.h
  include/clang/Driver/CC1Options.td
  include/clang/Driver/DarwinSDKInfo.h
  lib/CodeGen/ModuleBuilder.cpp
  lib/Driver/CMakeLists.txt
  lib/Driver/DarwinSDKInfo.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/Darwin.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/darwin-sdk-version.c
  test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
  test/Driver/darwin-sdk-version.c
  test/Frontend/ast-main.c
  test/Frontend/ast-main.cpp

Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -3193,6 +3193,14 @@
   Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128);
   Opts.NVPTXUseShortPointers = Args.hasFlag(
   options::OPT_fcuda_short_ptr, options::OPT_fno_cuda_short_ptr, false);
+  if (Arg *A = Args.getLastArg(options::OPT_target_sdk_version_EQ)) {
+llvm::VersionTuple Version;
+if (Version.tryParse(A->getValue()))
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+else
+  Opts.SDKVersion = Version;
+  }
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation ,
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -132,6 +132,9 @@
 
   M->setTargetTriple(Ctx->getTargetInfo().getTriple().getTriple());
   M->setDataLayout(Ctx->getTargetInfo().getDataLayout());
+  const auto  = Ctx->getTargetInfo().getSDKVersion();
+  if (!SDKVersion.empty())
+M->setSDKVersion(SDKVersion);
   Builder.reset(new CodeGen::CodeGenModule(Context, HeaderSearchOpts,
PreprocessorOpts, CodeGenOpts,
*M, Diags, CoverageInfo));
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1287,6 +1287,18 @@
 return DarwinPlatform(InferredFromArch, getPlatformFromOS(OS), Value);
   }
 
+  /// Constructs an inferred SDKInfo value based on the version inferred from
+  /// the SDK path itself. Only works for values that were created by inferring
+  /// the platform from the SDKPath.
+  DarwinSDKInfo inferSDKInfo() {
+assert(Kind == InferredFromSDK && "can infer SDK info only");
+llvm::VersionTuple Version;
+bool IsValid = !Version.tryParse(OSVersion);
+(void)IsValid;
+assert(IsValid && "invalid SDK version");
+return DarwinSDKInfo(Version);
+  }
+
 private:
   DarwinPlatform(SourceKind Kind, DarwinPlatformKind Platform, Arg *Argument)
   : Kind(Kind), Platform(Platform), Argument(Argument) {}
@@ -1420,8 +1432,11 @@
 }
 
 /// Tries to infer the deployment target from the SDK specified by -isysroot
-/// (or SDKROOT).
-Optional inferDeploymentTargetFromSDK(DerivedArgList ) {
+/// (or SDKROOT). Uses the version specified in the SDKSettings.json file if
+/// it's available.
+Optional
+inferDeploymentTargetFromSDK(DerivedArgList ,
+ const Optional ) {
   const Arg *A = Args.getLastArg(options::OPT_isysroot);
   if (!A)
 return None;
@@ -1429,28 +1444,37 @@
   StringRef SDK = Darwin::getSDKName(isysroot);
   if (!SDK.size())
 return None;
-  // Slice the version number out.
-  // Version number is between the first and the last number.
-  size_t StartVer = SDK.find_first_of("0123456789");
-  size_t EndVer = SDK.find_last_of("0123456789");
-  if (StartVer != StringRef::npos && EndVer > StartVer) {
-StringRef Version = SDK.slice(StartVer, EndVer + 1);
-if (SDK.startswith("iPhoneOS") || SDK.startswith("iPhoneSimulator"))
-  return DarwinPlatform::createFromSDK(
-  Darwin::IPhoneOS, Version,
-  /*IsSimulator=*/SDK.startswith("iPhoneSimulator"));
-else if (SDK.startswith("MacOSX"))
-  return DarwinPlatform::createFromSDK(Darwin::MacOS,
-   getSystemOrSDKMacOSVersion(Version));
-else if (SDK.startswith("WatchOS") || SDK.startswith("WatchSimulator"))
-  return DarwinPlatform::createFromSDK(
-  Darwin::WatchOS, Version,
-  /*IsSimulator=*/SDK.startswith("WatchSimulator"));
-else if (SDK.startswith("AppleTVOS") || 

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-17 Thread Steven Wu via Phabricator via cfe-commits
steven_wu added inline comments.



Comment at: include/clang/Driver/DarwinSDKInfo.h:36
+/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise.
+Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem 
,
+ StringRef SDKRootPath);

arphaman wrote:
> steven_wu wrote:
> > arphaman wrote:
> > > steven_wu wrote:
> > > > Isn't parseSDKSettings enough? And it can just return 
> > > > Optional?
> > > We will support other fields besides `VersionTuple` in the SDKSettings, 
> > > so that's why we have a structure.
> > I feel like for this usage, it is better to return Expected 
> > with all the fields being Optional?
> Hmm, we want to assume that version exists for future uses. I feel like the 
> current type captures the intention better.
I think it is fine for current use. We can always change in the future.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673



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


[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-17 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman marked an inline comment as done.
arphaman added inline comments.



Comment at: include/clang/Driver/DarwinSDKInfo.h:36
+/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise.
+Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem 
,
+ StringRef SDKRootPath);

steven_wu wrote:
> arphaman wrote:
> > steven_wu wrote:
> > > Isn't parseSDKSettings enough? And it can just return 
> > > Optional?
> > We will support other fields besides `VersionTuple` in the SDKSettings, so 
> > that's why we have a structure.
> I feel like for this usage, it is better to return Expected 
> with all the fields being Optional?
Hmm, we want to assume that version exists for future uses. I feel like the 
current type captures the intention better.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673



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


[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-17 Thread Steven Wu via Phabricator via cfe-commits
steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

Other than a small design choice commented inline, LGTM.




Comment at: include/clang/Driver/DarwinSDKInfo.h:36
+/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise.
+Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem 
,
+ StringRef SDKRootPath);

arphaman wrote:
> steven_wu wrote:
> > Isn't parseSDKSettings enough? And it can just return 
> > Optional?
> We will support other fields besides `VersionTuple` in the SDKSettings, so 
> that's why we have a structure.
I feel like for this usage, it is better to return Expected with 
all the fields being Optional?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673



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


[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-14 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 178309.
arphaman marked an inline comment as done.
arphaman added a comment.

Ensure test will pass on non-darwin.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/TargetInfo.h
  include/clang/Basic/TargetOptions.h
  include/clang/Driver/CC1Options.td
  include/clang/Driver/DarwinSDKInfo.h
  lib/CodeGen/ModuleBuilder.cpp
  lib/Driver/CMakeLists.txt
  lib/Driver/DarwinSDKInfo.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/Darwin.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/darwin-sdk-version.c
  test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
  test/Driver/darwin-sdk-version.c

Index: test/Driver/darwin-sdk-version.c
===
--- /dev/null
+++ test/Driver/darwin-sdk-version.c
@@ -0,0 +1,37 @@
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %S/Inputs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck %s
+// RUN: env SDKROOT=%S/Inputs/MacOSX10.14.sdk %clang -target x86_64-apple-macosx10.13 -c -### %s 2>&1 \
+// RUN:   | FileCheck %s
+//
+// RUN: rm -rf %t/SDKs/MacOSX10.10.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.10.sdk
+// RUN: %clang -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=INFER_SDK_VERSION %s
+// RUN: cp %S/Inputs/MacOSX10.14.sdk/SDKSettings.json %t/SDKs/MacOSX10.10.sdk
+// RUN: %clang -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=INFER_DEPLOYMENT_TARGET_VERSION %s
+// REQUIRES: system-darwin && native
+//
+// RUN: rm -rf %t/SDKs/MacOSX10.14.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.14.sdk
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %t/SDKs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NO_VERSION %s
+//
+// RUN: rm -rf %t/SDKs/MacOSX10.14.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.14.sdk
+// RUN: echo '{broken json' > %t/SDKs/MacOSX10.14.sdk/SDKSettings.json
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %t/SDKs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefixes=NO_VERSION,ERROR %s
+//
+// RUN: rm -rf %t/SDKs/MacOSX10.14.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.14.sdk
+// RUN: echo '{"Version":1}' > %t/SDKs/MacOSX10.14.sdk/SDKSettings.json
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %t/SDKs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefixes=NO_VERSION,ERROR %s
+
+// CHECK: -target-sdk-version=10.14
+// INFER_SDK_VERSION: "-triple" "x86_64-apple-macosx10.10.0"
+// INFER_SDK_VERSION-SAME: -target-sdk-version=10.10
+// INFER_DEPLOYMENT_TARGET_VERSION: "-triple" "x86_64-apple-macosx10.14.0"
+// NO_VERSION-NOT: target-sdk-version
+// ERROR: warning: SDK settings were ignored as 'SDKSettings.json' could not be parsed
Index: test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
===
--- /dev/null
+++ test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
@@ -0,0 +1 @@
+{"Version":"10.14"}
Index: test/CodeGen/darwin-sdk-version.c
===
--- /dev/null
+++ test/CodeGen/darwin-sdk-version.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14 -target-sdk-version=10.14.1 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: !llvm.module.flags = !{!0
+// CHECK: !0 = !{i32 2, !"SDK Version", [3 x i32] [i32 10, i32 14, i32 1]}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -3192,6 +3192,14 @@
   Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128);
   Opts.NVPTXUseShortPointers = Args.hasFlag(
   options::OPT_fcuda_short_ptr, options::OPT_fno_cuda_short_ptr, false);
+  if (Arg *A = Args.getLastArg(options::OPT_target_sdk_version_EQ)) {
+llvm::VersionTuple Version;
+if (Version.tryParse(A->getValue()))
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+else
+  Opts.SDKVersion = Version;
+  }
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation ,
Index: lib/Driver/ToolChains/Darwin.h
===
--- lib/Driver/ToolChains/Darwin.h
+++ lib/Driver/ToolChains/Darwin.h
@@ -11,9 +11,10 @@
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_DARWIN_H
 
 #include "Cuda.h"
-#include "clang/Driver/XRayArgs.h"
+#include "clang/Driver/DarwinSDKInfo.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
+#include "clang/Driver/XRayArgs.h"
 
 namespace clang {
 namespace driver {
@@ -288,6 +289,9 @@
   /// The OS version we are targeting.
   mutable VersionTuple TargetVersion;
 
+  /// The information about the darwin SDK that 

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-14 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman marked 2 inline comments as done.
arphaman added inline comments.



Comment at: lib/Driver/ToolChains/Darwin.cpp:2053
+return None;
+  }
+  return *SDKInfoOrErr;

steven_wu wrote:
> We also has this InferredFromSDK when we infer deployment target, which can 
> be used as a fallback method.
> 
> The result of parsing JSON should be available to InferredFromSDK in 
> deployment target setting.
> 
> Bonus point is to set "-sdk_version" flag passing to ld64.
Done.
We're not planning to pass it down to the linker.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673



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


[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-14 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 178307.
arphaman added a comment.

Updated to infer deployment target version from SDK versions specified in the 
JSON file and to allow inferring the SDK version from the SDK path.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/TargetInfo.h
  include/clang/Basic/TargetOptions.h
  include/clang/Driver/CC1Options.td
  include/clang/Driver/DarwinSDKInfo.h
  lib/CodeGen/ModuleBuilder.cpp
  lib/Driver/CMakeLists.txt
  lib/Driver/DarwinSDKInfo.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/Darwin.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/darwin-sdk-version.c
  test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
  test/Driver/darwin-sdk-version.c

Index: test/Driver/darwin-sdk-version.c
===
--- /dev/null
+++ test/Driver/darwin-sdk-version.c
@@ -0,0 +1,36 @@
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %S/Inputs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck %s
+// RUN: env SDKROOT=%S/Inputs/MacOSX10.14.sdk %clang -target x86_64-apple-macosx10.13 -c -### %s 2>&1 \
+// RUN:   | FileCheck %s
+//
+// RUN: rm -rf %t/SDKs/MacOSX10.10.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.10.sdk
+// RUN: %clang -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=INFER_SDK_VERSION %s
+// RUN: cp %S/Inputs/MacOSX10.14.sdk/SDKSettings.json %t/SDKs/MacOSX10.10.sdk
+// RUN: %clang -isysroot %t/SDKs/MacOSX10.10.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=INFER_DEPLOYMENT_TARGET_VERSION %s
+//
+// RUN: rm -rf %t/SDKs/MacOSX10.14.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.14.sdk
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %t/SDKs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NO_VERSION %s
+//
+// RUN: rm -rf %t/SDKs/MacOSX10.14.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.14.sdk
+// RUN: echo '{broken json' > %t/SDKs/MacOSX10.14.sdk/SDKSettings.json
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %t/SDKs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefixes=NO_VERSION,ERROR %s
+//
+// RUN: rm -rf %t/SDKs/MacOSX10.14.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.14.sdk
+// RUN: echo '{"Version":1}' > %t/SDKs/MacOSX10.14.sdk/SDKSettings.json
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %t/SDKs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefixes=NO_VERSION,ERROR %s
+
+// CHECK: -target-sdk-version=10.14
+// INFER_SDK_VERSION: "-triple" "x86_64-apple-macosx10.10.0"
+// INFER_SDK_VERSION-SAME: -target-sdk-version=10.10
+// INFER_DEPLOYMENT_TARGET_VERSION: "-triple" "x86_64-apple-macosx10.14.0"
+// NO_VERSION-NOT: target-sdk-version
+// ERROR: warning: SDK settings were ignored as 'SDKSettings.json' could not be parsed
Index: test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
===
--- /dev/null
+++ test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
@@ -0,0 +1 @@
+{"Version":"10.14"}
Index: test/CodeGen/darwin-sdk-version.c
===
--- /dev/null
+++ test/CodeGen/darwin-sdk-version.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14 -target-sdk-version=10.14.1 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: !llvm.module.flags = !{!0
+// CHECK: !0 = !{i32 2, !"SDK Version", [3 x i32] [i32 10, i32 14, i32 1]}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -3192,6 +3192,14 @@
   Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128);
   Opts.NVPTXUseShortPointers = Args.hasFlag(
   options::OPT_fcuda_short_ptr, options::OPT_fno_cuda_short_ptr, false);
+  if (Arg *A = Args.getLastArg(options::OPT_target_sdk_version_EQ)) {
+llvm::VersionTuple Version;
+if (Version.tryParse(A->getValue()))
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+else
+  Opts.SDKVersion = Version;
+  }
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation ,
Index: lib/Driver/ToolChains/Darwin.h
===
--- lib/Driver/ToolChains/Darwin.h
+++ lib/Driver/ToolChains/Darwin.h
@@ -11,9 +11,10 @@
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_DARWIN_H
 
 #include "Cuda.h"
-#include "clang/Driver/XRayArgs.h"
+#include "clang/Driver/DarwinSDKInfo.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
+#include "clang/Driver/XRayArgs.h"
 
 namespace clang {
 namespace driver {
@@ -288,6 +289,9 @@
   /// The OS version we are targeting.
   mutable VersionTuple TargetVersion;
 
+  /// The information 

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-13 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman marked an inline comment as done.
arphaman added inline comments.



Comment at: include/clang/Driver/DarwinSDKInfo.h:36
+/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise.
+Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem 
,
+ StringRef SDKRootPath);

steven_wu wrote:
> Isn't parseSDKSettings enough? And it can just return Optional?
We will support other fields besides `VersionTuple` in the SDKSettings, so 
that's why we have a structure.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673



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


[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-13 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman marked an inline comment as done.
arphaman added inline comments.



Comment at: include/clang/Driver/DarwinSDKInfo.h:1
+//===--- DarwinSDKInfo.h - SDK Information parser for darwin *- C++ 
-*-===//
+//

steven_wu wrote:
> Can this just be in Toolchains/Darwin.h?
No, we need to expose it to clients like Swift.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673



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


[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-13 Thread Steven Wu via Phabricator via cfe-commits
steven_wu added a comment.

See comments inline.




Comment at: include/clang/Driver/DarwinSDKInfo.h:1
+//===--- DarwinSDKInfo.h - SDK Information parser for darwin *- C++ 
-*-===//
+//

Can this just be in Toolchains/Darwin.h?



Comment at: include/clang/Driver/DarwinSDKInfo.h:36
+/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise.
+Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem 
,
+ StringRef SDKRootPath);

Isn't parseSDKSettings enough? And it can just return Optional?



Comment at: lib/Driver/ToolChains/Darwin.cpp:2053
+return None;
+  }
+  return *SDKInfoOrErr;

We also has this InferredFromSDK when we infer deployment target, which can be 
used as a fallback method.

The result of parsing JSON should be available to InferredFromSDK in deployment 
target setting.

Bonus point is to set "-sdk_version" flag passing to ld64.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55673



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


[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-13 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman created this revision.
arphaman added reviewers: steven_wu, ab, dexonsmith.
Herald added subscribers: jkorous, mgorny.

This patch is a follow-up to the LLVM SDK Version metadata support: 
https://reviews.llvm.org/D55612.

This patch adds support for reading the `SDKSettings.json` file in the Darwin 
driver. This file is used by the driver to determine the SDK's version, and it 
uses that information to pass it down to the compiler using the new 
`-target-sdk-version=`. This option is then used to set the appropriate `SDK 
Version` module metadata introduced in https://reviews.llvm.org/D55612.


Repository:
  rC Clang

https://reviews.llvm.org/D55673

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/TargetInfo.h
  include/clang/Basic/TargetOptions.h
  include/clang/Driver/CC1Options.td
  include/clang/Driver/DarwinSDKInfo.h
  lib/CodeGen/ModuleBuilder.cpp
  lib/Driver/CMakeLists.txt
  lib/Driver/DarwinSDKInfo.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/darwin-sdk-version.c
  test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
  test/Driver/darwin-sdk-version.c

Index: test/Driver/darwin-sdk-version.c
===
--- /dev/null
+++ test/Driver/darwin-sdk-version.c
@@ -0,0 +1,25 @@
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %S/Inputs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck %s
+// RUN: env SDKROOT=%S/Inputs/MacOSX10.14.sdk %clang -target x86_64-apple-macosx10.13 -c -### %s 2>&1 \
+// RUN:   | FileCheck %s
+//
+// RUN: rm -rf %t/SDKs/MacOSX10.14.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.14.sdk
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %t/SDKs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NO_VERSION %s
+//
+// RUN: rm -rf %t/SDKs/MacOSX10.14.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.14.sdk
+// RUN: echo '{broken json' > %t/SDKs/MacOSX10.14.sdk/SDKSettings.json
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %t/SDKs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefixes=NO_VERSION,ERROR %s
+//
+// RUN: rm -rf %t/SDKs/MacOSX10.14.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX10.14.sdk
+// RUN: echo '{"Version":1}' > %t/SDKs/MacOSX10.14.sdk/SDKSettings.json
+// RUN: %clang -target x86_64-apple-macosx10.13 -isysroot %t/SDKs/MacOSX10.14.sdk -c -### %s 2>&1 \
+// RUN:   | FileCheck --check-prefixes=NO_VERSION,ERROR %s
+
+// CHECK: -target-sdk-version=10.14
+// NO_VERSION-NOT: target-sdk-version
+// ERROR: warning: SDK settings were ignored as 'SDKSettings.json' could not be parsed
Index: test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
===
--- /dev/null
+++ test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
@@ -0,0 +1 @@
+{"Version":"10.14"}
Index: test/CodeGen/darwin-sdk-version.c
===
--- /dev/null
+++ test/CodeGen/darwin-sdk-version.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14 -target-sdk-version=10.14.1 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: !llvm.module.flags = !{!0
+// CHECK: !0 = !{i32 2, !"SDK Version", [3 x i32] [i32 10, i32 14, i32 1]}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -3188,6 +3188,14 @@
   Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128);
   Opts.NVPTXUseShortPointers = Args.hasFlag(
   options::OPT_fcuda_short_ptr, options::OPT_fno_cuda_short_ptr, false);
+  if (Arg *A = Args.getLastArg(options::OPT_target_sdk_version_EQ)) {
+llvm::VersionTuple Version;
+if (Version.tryParse(A->getValue()))
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+else
+  Opts.SDKVersion = Version;
+  }
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation ,
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -13,6 +13,7 @@
 #include "clang/Basic/AlignedAllocation.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Driver/Compilation.h"
+#include "clang/Driver/DarwinSDKInfo.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
@@ -2037,6 +2038,22 @@
   return TargetVersion < alignedAllocMinVersion(OS);
 }
 
+static Optional parseSDKSettings(llvm::vfs::FileSystem ,
+const ArgList ,
+const Driver ) {
+  const Arg *A = Args.getLastArg(options::OPT_isysroot);
+  if (!A)
+return None;
+  StringRef isysroot = A->getValue();
+  auto SDKInfoOrErr = driver::parseDarwinSDKInfo(VFS, isysroot);
+  if