[clang-cl] Add a /diasdkdir flag and make /winsysroot imply it

[thakis]

Summary

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.

[hans]

The /winsysroot part makes sense to me, but what's the case for the new /diasdkdir flag?

clang/lib/Driver/ToolChains/MSVC.cpp

66

Maybe move the arch functions up to before visualstudio::Linker::ConstructJob instead?

The file is already kind of organized in that way, with lots of static helper functions on top, and then the implementations of the member functions.

[thakis]

move arch functions

[thakis]

In D109828#3002114, @hans wrote:

The /winsysroot part makes sense to me, but what's the case for the new /diasdkdir flag?

• If you do have a vcvars shell, you don't need the full sysroot path and it's kind of useful (see example in commit message)
• it seems nice to be able to explain /winsysroot as combination of other flags in the help text
• it makes writing the test a bit easier
• it makes diasdkdir more like the other flags controlled by /winsysroot

clang/lib/Driver/ToolChains/MSVC.cpp

66

Sure, done. The file has a bunch of other static functions all over the place too though :)

[hans]

In D109828#3002495, @thakis wrote:

In D109828#3002114, @hans wrote:

The /winsysroot part makes sense to me, but what's the case for the new /diasdkdir flag?

• If you do have a vcvars shell, you don't need the full sysroot path and it's kind of useful (see example in commit message)
• it seems nice to be able to explain /winsysroot as combination of other flags in the help text
• it makes writing the test a bit easier
• it makes diasdkdir more like the other flags controlled by /winsysroot

Okay, sounds good to me.

clang/include/clang/Driver/Options.td

 def _SLASH_permissive_ : CLFlag<"permissive-">,
   HelpText<"Disable non conforming code from compiling (default)">;
 def _SLASH_Tc : CLCompileJoinedOrSeparate<"Tc">,
   HelpText<"Treat <file> as C source file">, MetaVarName<"<file>">;
 def _SLASH_TC : CLCompileFlag<"TC">, HelpText<"Treat all source files as C">;
 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">,
   HelpText<"For use with /winsysroot, defaults to newest found">;
 def _SLASH_winsdkdir : CLJoinedOrSeparate<"winsdkdir">,
   HelpText<"Path to the Windows SDK">, MetaVarName<"<dir>">;
 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]>,
   HelpText<"Volatile loads and stores have standard semantics">;
 def _SLASH_vmb : CLFlag<"vmb">,
   HelpText<"Use a best-case representation method for member pointers">;
 def _SLASH_vmg : CLFlag<"vmg">,
   HelpText<"Use a most-general representation for member pointers">;

clang/lib/Driver/ToolChains/MSVC.cpp

 #endif
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
 using namespace clang::driver::tools;
 using namespace clang;
 using namespace llvm::opt;
 
+// Windows SDKs and VC Toolchains group their contents into subdirectories based
+// on the target architecture. This function converts an llvm::Triple::ArchType
+// to the corresponding subdirectory name.
+static const char *llvmArchToWindowsSDKArch(llvm::Triple::ArchType Arch) {
+  using ArchType = llvm::Triple::ArchType;
+  switch (Arch) {
+  case ArchType::x86:
+    return "x86";
+  case ArchType::x86_64:
+    return "x64";
+  case ArchType::arm:
+    return "arm";
+  case ArchType::aarch64:
+    return "arm64";
+  default:
+    return "";
+  }
+}
+
+// Similar to the above function, but for Visual Studios before VS2017.
+static const char *llvmArchToLegacyVCArch(llvm::Triple::ArchType Arch) {
+  using ArchType = llvm::Triple::ArchType;
+  switch (Arch) {
+  case ArchType::x86:
+    // x86 is default in legacy VC toolchains.
+    // e.g. x86 libs are directly in /lib as opposed to /lib/x86.
+    return "";
+  case ArchType::x86_64:
+    return "amd64";
+  case ArchType::arm:
+    return "arm";
+  case ArchType::aarch64:
+    return "arm64";
+  default:
+    return "";
+  }
+}
+
+// Similar to the above function, but for DevDiv internal builds.
+static const char *llvmArchToDevDivInternalArch(llvm::Triple::ArchType Arch) {
+  using ArchType = llvm::Triple::ArchType;
+  switch (Arch) {
+  case ArchType::x86:
+    return "i386";
+  case ArchType::x86_64:
+    return "amd64";
+  case ArchType::arm:
+    return "arm";
+  case ArchType::aarch64:
+    return "arm64";
+  default:
+    return "";
+  }
+}
+
 static bool canExecute(llvm::vfs::FileSystem &VFS, StringRef Path) {
   auto Status = VFS.status(Path);
   if (!Status)
     return false;
   return (Status->getPermissions() & llvm::sys::fs::perms::all_exe) != 0;
 }
 
 // Defined below.
if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles) &&
     CmdArgs.push_back("-defaultlib:oldnames");
   }
 
   // If the VC environment hasn't been configured (perhaps because the user
   // did not run vcvarsall), try to build a consistent link environment.  If
   // 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)) {
     CmdArgs.push_back(Args.MakeArgString(
         Twine("-libpath:") +
         TC.getSubDirectoryPath(
             toolchains::MSVCToolChain::SubDirectoryType::Lib)));
     CmdArgs.push_back(Args.MakeArgString(
void MSVCToolChain::AddHIPIncludeArgs(const ArgList &DriverArgs,
   RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
 }
 
 void MSVCToolChain::printVerboseInfo(raw_ostream &OS) const {
   CudaInstallation.print(OS);
   RocmInstallation.print(OS);
 }
 
-// Windows SDKs and VC Toolchains group their contents into subdirectories based
-// on the target architecture. This function converts an llvm::Triple::ArchType
-// to the corresponding subdirectory name.
-static const char *llvmArchToWindowsSDKArch(llvm::Triple::ArchType Arch) {
-  using ArchType = llvm::Triple::ArchType;
-  switch (Arch) {
-  case ArchType::x86:
-    return "x86";
-  case ArchType::x86_64:
-    return "x64";
-  case ArchType::arm:
-    return "arm";
-  case ArchType::aarch64:
-    return "arm64";
-  default:
-    return "";
-  }
-}
-
-// Similar to the above function, but for Visual Studios before VS2017.
-static const char *llvmArchToLegacyVCArch(llvm::Triple::ArchType Arch) {
-  using ArchType = llvm::Triple::ArchType;
-  switch (Arch) {
-  case ArchType::x86:
-    // x86 is default in legacy VC toolchains.
-    // e.g. x86 libs are directly in /lib as opposed to /lib/x86.
-    return "";
-  case ArchType::x86_64:
-    return "amd64";
-  case ArchType::arm:
-    return "arm";
-  case ArchType::aarch64:
-    return "arm64";
-  default:
-    return "";
-  }
-}
-
-// Similar to the above function, but for DevDiv internal builds.
-static const char *llvmArchToDevDivInternalArch(llvm::Triple::ArchType Arch) {
-  using ArchType = llvm::Triple::ArchType;
-  switch (Arch) {
-  case ArchType::x86:
-    return "i386";
-  case ArchType::x86_64:
-    return "amd64";
-  case ArchType::arm:
-    return "arm";
-  case ArchType::aarch64:
-    return "arm64";
-  default:
-    return "";
-  }
-}
-
 // Get the path to a specific subdirectory in the current toolchain for
 // a given target architecture.
 // VS2017 changed the VC toolchain layout, so this should be used instead
 // of hardcoding paths.
 std::string
 MSVCToolChain::getSubDirectoryPath(SubDirectoryType Type,
                                    llvm::StringRef SubdirParent,
                                    llvm::Triple::ArchType TargetArch) const {
void MSVCToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   };
 
   // Add %INCLUDE%-like dirs via /external:env: flags.
   for (const auto &Var :
        DriverArgs.getAllArgValues(options::OPT__SLASH_external_env)) {
     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;
 
   // Honor %INCLUDE% and %EXTERNAL_INCLUDE%. It should have essential search
   // paths set by vcvarsall.bat. Skip if the user expressly set a vctoolsdir.
   if (!DriverArgs.getLastArg(options::OPT__SLASH_vctoolsdir,
                              options::OPT__SLASH_winsysroot)) {
     bool Found = AddSystemIncludesFromEnv("INCLUDE");

clang/test/Driver/cl-sysroot.cpp

 // 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 {
 public:
   bool empty();
 };
 }
 
 #--- 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>
 
 void f() {
   std::mystring s;
   myassert(s.empty());
 }