thakis created this revision.
thakis added a reviewer: hans.
Herald added subscribers: dang, pengfei.
thakis requested review of this revision.

D109708 <https://reviews.llvm.org/D109708> added "DIA SDK" to our win sysroot 
for hermetic builds
that use LLVM_ENABLE_DIA_SDK. But the build system still has to
manually pass flags pointing to it.

Since we have a /winsysroot flag, make it look at DIA SDK in
the sysroot.

With this, the following is enough to compile the DIA2Dump example:

out\gn\bin\clang-cl ^

  "sysroot\DIA SDK\Samples\DIA2Dump\DIA2Dump.cpp" ^
  "sysroot\DIA SDK\Samples\DIA2Dump\PrintSymbol.cpp" ^
  "sysroot\DIA SDK\Samples\DIA2Dump\regs.cpp" ^
  /diasdkdir "sysroot\DIA SDK" ^
  ole32.lib oleaut32.lib diaguids.lib

---

Given that the DIA SDK is part of the MSVC installation (at ` C:\Program Files 
(x86)\Microsoft Visual Studio\2017\Professional\DIA SDK` on my machine), I 
think there's a case for letting clang-cl know about it. Arguably, adding `/I 
sysroot\DIA SDK\include` to your cflags isn't super difficult, but one day 
we'll teach lld-link about /winsysroot too, and then it means you don't need to 
know about the `lib\` folder layout inside DIA SDK. Also, if you link through 
clang-cl instead of calling the linker directly, you get that simplification 
with this change already. That's nice for local one-off commands, but in 
practice most projects admittedly call link.exe directly.

So all-in-all, I think this is a good idea, but I could be convinced otherwise 
if others disagree.


https://reviews.llvm.org/D109828

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/cl-sysroot.cpp

Index: clang/test/Driver/cl-sysroot.cpp
===================================================================
--- clang/test/Driver/cl-sysroot.cpp
+++ clang/test/Driver/cl-sysroot.cpp
@@ -1,18 +1,27 @@
 // RUN: rm -rf %t
 // RUN: split-file %s %t
 
-// RUN: %clang_cl /winsysroot %t -### -- %t/foo.cpp 2>&1 | FileCheck %s
-// RUN: %clang_cl /vctoolsdir %t/VC/Tools/MSVC/27.1828.18284 \
-// RUN:           /winsdkdir "%t/Windows Kits/10" \
-// RUN:           -### -- %t/foo.cpp 2>&1 | FileCheck %s
+// RUN: %clang_cl -m64 /winsysroot %t -### -- %t/foo.cpp 2>&1 | FileCheck %s
+// RUN: %clang_cl -m64 \
+// RUN:     /diasdkdir "%t/DIA SDK" \
+// RUN:     /vctoolsdir %t/VC/Tools/MSVC/27.1828.18284 \
+// RUN:     /winsdkdir "%t/Windows Kits/10" \
+// RUN:     -### -- %t/foo.cpp 2>&1 | FileCheck %s
 
-// CHECK: "-internal-isystem" "[[ROOT:[^"]*]]{{/|\\\\}}VC{{/|\\\\}}Tools{{/|\\\\}}MSVC{{/|\\\\}}27.1828.18284{{/|\\\\}}include"
+// CHECK: "-internal-isystem" "[[ROOT:[^"]*]]{{/|\\\\}}DIA SDK{{/|\\\\}}include"
+// CHECK: "-internal-isystem" "[[ROOT]]{{/|\\\\}}VC{{/|\\\\}}Tools{{/|\\\\}}MSVC{{/|\\\\}}27.1828.18284{{/|\\\\}}include"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|\\\\}}VC{{/|\\\\}}Tools{{/|\\\\}}MSVC{{/|\\\\}}27.1828.18284{{/|\\\\}}atlmfc{{/|\\\\}}include"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|\\\\}}Windows Kits{{/|\\\\}}10{{/|\\\\}}Include{{/|\\\\}}10.0.19041.0{{/|\\\\}}ucrt"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|\\\\}}Windows Kits{{/|\\\\}}10{{/|\\\\}}Include{{/|\\\\}}10.0.19041.0{{/|\\\\}}shared"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|\\\\}}Windows Kits{{/|\\\\}}10{{/|\\\\}}Include{{/|\\\\}}10.0.19041.0{{/|\\\\}}um"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|\\\\}}Windows Kits{{/|\\\\}}10{{/|\\\\}}Include{{/|\\\\}}10.0.19041.0{{/|\\\\}}winrt"
 
+// CHECK: "-libpath:[[ROOT]]{{/|\\\\}}DIA SDK{{/|\\\\}}lib{{/|\\\\}}amd64"
+// CHECK: "-libpath:[[ROOT]]{{/|\\\\}}VC{{/|\\\\}}Tools{{/|\\\\}}MSVC{{/|\\\\}}27.1828.18284{{/|\\\\}}lib{{/|\\\\}}x64"
+// CHECK: "-libpath:[[ROOT]]{{/|\\\\}}VC{{/|\\\\}}Tools{{/|\\\\}}MSVC{{/|\\\\}}27.1828.18284{{/|\\\\}}atlmfc{{/|\\\\}}lib{{/|\\\\}}x64"
+// CHECK: "-libpath:[[ROOT]]{{/|\\\\}}Windows Kits{{/|\\\\}}10{{/|\\\\}}Lib{{/|\\\\}}10.0.19041.0{{/|\\\\}}ucrt{{/|\\\\}}x64"
+// CHECK: "-libpath:[[ROOT]]{{/|\\\\}}Windows Kits{{/|\\\\}}10{{/|\\\\}}Lib{{/|\\\\}}10.0.19041.0{{/|\\\\}}um{{/|\\\\}}x64"
+
 #--- VC/Tools/MSVC/27.1828.18284/include/string
 namespace std {
 class mystring {
@@ -24,6 +33,9 @@
 #--- Windows Kits/10/Include/10.0.19041.0/ucrt/assert.h
 #define myassert(X)
 
+#--- DIA SDK/include/cvconst.h
+#define myotherassert(X)
+
 #--- foo.cpp
 #include <assert.h>
 #include <string>
@@ -31,4 +43,5 @@
 void f() {
   std::mystring s;
   myassert(s.empty());
+  myotherassert(s.empty());
 }
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===================================================================
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -63,6 +63,8 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static const char *llvmArchToLegacyVCArch(llvm::Triple::ArchType Arch);
+
 static bool canExecute(llvm::vfs::FileSystem &VFS, StringRef Path) {
   auto Status = VFS.status(Path);
   if (!Status)
@@ -396,6 +398,20 @@
   // the environment variable is set however, assume the user knows what
   // they're doing. If the user passes /vctoolsdir or /winsdkdir, trust that
   // over env vars.
+  if (const Arg *A = Args.getLastArg(options::OPT__SLASH_diasdkdir,
+                                     options::OPT__SLASH_winsysroot)) {
+    // cl.exe doesn't find the DIA SDK automatically, so this too requires
+    // explicit flags and doesn't automatically look in "DIA SDK" relative
+    // to the path we found for VCToolChainPath.
+    llvm::SmallString<128> DIAPath(A->getValue());
+    if (A->getOption().getID() == options::OPT__SLASH_winsysroot)
+      llvm::sys::path::append(DIAPath, "DIA SDK");
+
+    // The DIA SDK always uses the legacy vc arch, even in new MSVC versions.
+    llvm::sys::path::append(DIAPath, "lib",
+                            llvmArchToLegacyVCArch(TC.getArch()));
+    CmdArgs.push_back(Args.MakeArgString(Twine("-libpath:") + DIAPath));
+  }
   if (!llvm::sys::Process::GetEnv("LIB") ||
       Args.getLastArg(options::OPT__SLASH_vctoolsdir,
                       options::OPT__SLASH_winsysroot)) {
@@ -1263,6 +1279,19 @@
     AddSystemIncludesFromEnv(Var);
   }
 
+  // Add DIA SDK include if requested.
+  if (const Arg *A = DriverArgs.getLastArg(options::OPT__SLASH_diasdkdir,
+                                           options::OPT__SLASH_winsysroot)) {
+    // cl.exe doesn't find the DIA SDK automatically, so this too requires
+    // explicit flags and doesn't automatically look in "DIA SDK" relative
+    // to the path we found for VCToolChainPath.
+    llvm::SmallString<128> DIASDKPath(A->getValue());
+    if (A->getOption().getID() == options::OPT__SLASH_winsysroot)
+      llvm::sys::path::append(DIASDKPath, "DIA SDK");
+    AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, std::string(DIASDKPath),
+                                  "include");
+  }
+
   if (DriverArgs.hasArg(options::OPT_nostdlibinc))
     return;
 
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6228,6 +6228,8 @@
 def _SLASH_Tp : CLCompileJoinedOrSeparate<"Tp">,
   HelpText<"Treat <file> as C++ source file">, MetaVarName<"<file>">;
 def _SLASH_TP : CLCompileFlag<"TP">, HelpText<"Treat all source files as C++">;
+def _SLASH_diasdkdir : CLJoinedOrSeparate<"diasdkdir">,
+  HelpText<"Path to the DIA SDK">, MetaVarName<"<dir>">;
 def _SLASH_vctoolsdir : CLJoinedOrSeparate<"vctoolsdir">,
   HelpText<"Path to the VCToolChain">, MetaVarName<"<dir>">;
 def _SLASH_vctoolsversion : CLJoinedOrSeparate<"vctoolsversion">,
@@ -6237,7 +6239,7 @@
 def _SLASH_winsdkversion : CLJoinedOrSeparate<"winsdkversion">,
   HelpText<"Full version of the Windows SDK, defaults to newest found">;
 def _SLASH_winsysroot : CLJoinedOrSeparate<"winsysroot">,
-  HelpText<"Same as /vctoolsdir <dir>/VC/Tools/MSVC/<vctoolsversion> /winsdkdir <dir>/Windows Kits/10">,
+  HelpText<"Same as \"/diasdkdir <dir>/DIA SDK\" /vctoolsdir <dir>/VC/Tools/MSVC/<vctoolsversion> \"/winsdkdir <dir>/Windows Kits/10\"">,
   MetaVarName<"<dir>">;
 def _SLASH_volatile_iso : Option<["/", "-"], "volatile:iso", KIND_FLAG>,
   Group<_SLASH_volatile_Group>, Flags<[CLOption, NoXarchOption]>,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to