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