diff --git a/include/llvm/Option/OptTable.h b/include/llvm/Option/OptTable.h index 051cc294c..1304b7008 100644 --- a/include/llvm/Option/OptTable.h +++ b/include/llvm/Option/OptTable.h @@ -133,6 +133,8 @@ public: unsigned FlagsToInclude = 0, unsigned FlagsToExclude = 0) const; + Option findOption(const char *normalizedName, unsigned FlagsToInclude = 0, unsigned FlagsToExclude = 0) const; // HLSL Change + /// \brief Parse an list of arguments into an InputArgList. /// /// The resulting InputArgList will reference the strings in [\p ArgBegin, diff --git a/lib/Option/OptTable.cpp b/lib/Option/OptTable.cpp index 5c72c50e1..2c25b676c 100644 --- a/lib/Option/OptTable.cpp +++ b/lib/Option/OptTable.cpp @@ -188,6 +188,35 @@ static unsigned matchOption(const OptTable::Info *I, StringRef Str, return 0; } +// HLSL Change - begin +Option OptTable::findOption(const char *normalizedName, unsigned FlagsToInclude, unsigned FlagsToExclude) const { + const Info *Start = OptionInfos + FirstSearchableIndex; + const Info *End = OptionInfos + getNumOptions(); + + StringRef Str(normalizedName); + + for (; Start != End; ++Start) { + // Scan for first option which is a proper prefix. + for (; Start != End; ++Start) + if (Str.startswith(Start->Name)) + break; + if (Start == End) + break; + + Option Opt(Start, this); + + if (FlagsToInclude && !Opt.hasFlag(FlagsToInclude)) + continue; + if (Opt.hasFlag(FlagsToExclude)) + continue; + + return Opt; + } + + return Option(nullptr, this); +} +// HLSL Change - end + Arg *OptTable::ParseOneArg(const ArgList &Args, unsigned &Index, unsigned FlagsToInclude, unsigned FlagsToExclude) const { diff --git a/tools/clang/tools/dxcompiler/dxcpdbutils.cpp b/tools/clang/tools/dxcompiler/dxcpdbutils.cpp index 966d501d1..c383797fc 100644 --- a/tools/clang/tools/dxcompiler/dxcpdbutils.cpp +++ b/tools/clang/tools/dxcompiler/dxcpdbutils.cpp @@ -543,6 +543,29 @@ private: } void AddArgPair(ArgPair &&newPair) { + const llvm::opt::OptTable *optTable = hlsl::options::getHlslOptTable(); + + if (newPair.Name.size() && newPair.Value.size()) { + // Handling case where old positional arguments used to have + // written as the option name. + if (newPair.Name == L"") { + newPair.Name.clear(); + } + // Check if the option and its value must be merged. Newer compiler + // pre-merge them before writing them to the PDB, but older PDBs might + // have them separated. + else { + std::string NameUtf8 = ToUtf8String(newPair.Name); + llvm::opt::Option opt = optTable->findOption(NameUtf8.c_str()); + if (opt.isValid()) { + if (opt.getKind() == llvm::opt::Option::JoinedClass) { + newPair.Name += newPair.Value; + newPair.Value.clear(); + } + } + } + } + bool excludeFromFlags = false; if (newPair.Name == L"E") { m_EntryPoint = newPair.Value; diff --git a/tools/clang/tools/dxcompiler/dxcshadersourceinfo.cpp b/tools/clang/tools/dxcompiler/dxcshadersourceinfo.cpp index 397c83547..7990d59b4 100644 --- a/tools/clang/tools/dxcompiler/dxcshadersourceinfo.cpp +++ b/tools/clang/tools/dxcompiler/dxcshadersourceinfo.cpp @@ -426,6 +426,7 @@ void SourceInfoWriter::Write(llvm::StringRef targetProfile, llvm::StringRef entr const llvm::opt::OptTable *optTable = hlsl::options::getHlslOptTable(); llvm::opt::InputArgList argList = optTable->ParseArgs(optPointers, missingIndex, missingCount); + llvm::SmallString<64> argumentStorage; const size_t argumentsOffset = m_Buffer.size(); for (llvm::opt::Arg *arg : argList) { llvm::StringRef name = arg->getOption().getName(); @@ -435,6 +436,23 @@ void SourceInfoWriter::Write(llvm::StringRef targetProfile, llvm::StringRef entr value = arg->getValue(); } + + // If this is a positional argument, set the name to "" + // explicitly. + if (arg->getOption().getKind() == llvm::opt::Option::InputClass) { + name = ""; + } + // If the argument must be merged (eg. -Wx, where W is the option and x is + // the value), merge them right now. + else if (arg->getOption().getKind() == llvm::opt::Option::JoinedClass) { + argumentStorage.clear(); + argumentStorage.append(name); + argumentStorage.append(value); + + name = argumentStorage; + value = nullptr; + } + // Name Append(&m_Buffer, name.data(), name.size()); Append(&m_Buffer, 0); // Null term diff --git a/tools/clang/unittests/HLSL/CompilerTest.cpp b/tools/clang/unittests/HLSL/CompilerTest.cpp index 047007a8e..a8abb40dd 100644 --- a/tools/clang/unittests/HLSL/CompilerTest.cpp +++ b/tools/clang/unittests/HLSL/CompilerTest.cpp @@ -128,6 +128,7 @@ public: TEST_METHOD(CompileWhenWorksThenAddRemovePrivate) TEST_METHOD(CompileThenAddCustomDebugName) TEST_METHOD(CompileThenTestPdbUtils) + TEST_METHOD(CompileThenTestPdbUtilsWarningOpt) TEST_METHOD(CompileThenTestPdbInPrivate) TEST_METHOD(CompileThenTestPdbUtilsStripped) TEST_METHOD(CompileThenTestPdbUtilsEmptyEntry) @@ -1632,6 +1633,113 @@ TEST_F(CompilerTest, CompileThenTestPdbUtils) { TestPdbUtils(/*bSlim*/false, /*bSourceInDebugModule*/false, /*strip*/true); // Full PDB, where source info is stored in its own part, and debug module is present } + +TEST_F(CompilerTest, CompileThenTestPdbUtilsWarningOpt) { + CComPtr pCompiler; + VERIFY_SUCCEEDED(CreateCompiler(&pCompiler)); + + std::string main_source = R"x( + cbuffer MyCbuffer : register(b1) { + float4 my_cbuf_foo; + } + + [RootSignature("CBV(b1)")] + float4 main() : SV_Target { + return my_cbuf_foo; + } + )x"; + + CComPtr pUtils; + VERIFY_SUCCEEDED(m_dllSupport.CreateInstance(CLSID_DxcUtils, &pUtils)); + + + + CComPtr pCompiler3; + VERIFY_SUCCEEDED(pCompiler.QueryInterface(&pCompiler3)); + + const WCHAR *args[] = { + L"/Zs", + L".\redundant_input", + L"-Wno-parentheses-equality", + L"hlsl.hlsl", + L"/Tps_6_0", + L"/Emain", + }; + + DxcBuffer buf = {}; + buf.Ptr = main_source.c_str(); + buf.Size = main_source.size(); + buf.Encoding = CP_UTF8; + + CComPtr pResult; + VERIFY_SUCCEEDED(pCompiler3->Compile(&buf, args, _countof(args), nullptr, IID_PPV_ARGS(&pResult))); + + CComPtr pPdb; + VERIFY_SUCCEEDED(pResult->GetOutput(DXC_OUT_PDB, IID_PPV_ARGS(&pPdb), nullptr)); + + auto TestPdb = [](IDxcPdbUtils *pPdbUtils) { + UINT32 uArgsCount = 0; + VERIFY_SUCCEEDED(pPdbUtils->GetArgCount(&uArgsCount)); + bool foundArg = false; + for (UINT32 i = 0; i < uArgsCount; i++) { + CComBSTR pArg; + VERIFY_SUCCEEDED(pPdbUtils->GetArg(i, &pArg)); + if (pArg) { + std::wstring arg(pArg); + if (arg == L"-Wno-parentheses-equality" || arg == L"/Wno-parentheses-equality") { + foundArg = true; + } + else { + // Make sure arg value "no-parentheses-equality" doesn't show up + // as its own argument token. + VERIFY_ARE_NOT_EQUAL(arg, L"no-parentheses-equality"); + + // Make sure the presence of the argument ".\redundant_input" + // doesn't cause "" to show up. + VERIFY_ARE_NOT_EQUAL(arg, L""); + } + } + } + VERIFY_IS_TRUE(foundArg); + + UINT32 uFlagsCount = 0; + VERIFY_SUCCEEDED(pPdbUtils->GetFlagCount(&uFlagsCount)); + bool foundFlag = false; + for (UINT32 i = 0; i < uFlagsCount; i++) { + CComBSTR pFlag; + VERIFY_SUCCEEDED(pPdbUtils->GetFlag(i, &pFlag)); + if (pFlag) { + std::wstring arg(pFlag); + if (arg == L"-Wno-parentheses-equality" || arg == L"/Wno-parentheses-equality") { + foundFlag = true; + } + else { + // Make sure arg value "no-parentheses-equality" doesn't show up + // as its own flag token. + VERIFY_ARE_NOT_EQUAL(arg, L"no-parentheses-equality"); + } + } + } + VERIFY_IS_TRUE(foundFlag); + + CComBSTR pMainFileName; + VERIFY_SUCCEEDED(pPdbUtils->GetMainFileName(&pMainFileName)); + std::wstring mainFileName = pMainFileName; + VERIFY_ARE_EQUAL(mainFileName, L"hlsl.hlsl"); + }; + + CComPtr pPdbUtils; + VERIFY_SUCCEEDED(m_dllSupport.CreateInstance(CLSID_DxcPdbUtils, &pPdbUtils)); + + VERIFY_SUCCEEDED(pPdbUtils->Load(pPdb)); + TestPdb(pPdbUtils); + + CComPtr pFullPdb; + VERIFY_SUCCEEDED(pPdbUtils->GetFullPDB(&pFullPdb)); + VERIFY_SUCCEEDED(pPdbUtils->Load(pFullPdb)); + TestPdb(pPdbUtils); +} + TEST_F(CompilerTest, CompileThenTestPdbInPrivate) { CComPtr pCompiler; VERIFY_SUCCEEDED(CreateCompiler(&pCompiler));