Merge pull request #3684 from tex3d/refactor-exectest-setup-on-release

Merge fixes for release - refactor setup, -Zi and PDB fixes

include/llvm/IR/BasicBlock.h

@@ -245,6 +245,8 @@ public:
   inline const Instruction       &back() const { return InstList.back();  }
   inline       Instruction       &back()       { return InstList.back();  }
 
+  size_t compute_size_no_dbg() const; // HLSL Change - Get the size of the block without the debug insts
+
   /// \brief Return the underlying instruction list container.
   ///
   /// Currently you need to access the underlying instruction list container

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,

lib/IR/BasicBlock.cpp

@@ -168,6 +168,18 @@ CallInst *BasicBlock::getTerminatingMustTailCall() {
   return nullptr;
 }
 
+// HLSL Change - begin
+size_t BasicBlock::compute_size_no_dbg() const {
+  size_t ret = 0;
+  for (auto it = InstList.begin(), E = InstList.end(); it != E; it++) {
+    if (isa<DbgInfoIntrinsic>(&*it))
+      continue;
+    ret++;
+  }
+  return ret;
+}
+// HLSL Change - end
+
 Instruction* BasicBlock::getFirstNonPHI() {
   for (Instruction &I : *this)
     if (!isa<PHINode>(I))

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 {

lib/Transforms/Scalar/MergedLoadStoreMotion.cpp

@@ -359,7 +359,8 @@ bool MergedLoadStoreMotion::mergeLoads(BasicBlock *BB) {
   BasicBlock *Succ0 = BI->getSuccessor(0);
   BasicBlock *Succ1 = BI->getSuccessor(1);
   // #Instructions in Succ1 for Compile Time Control
-  int Size1 = Succ1->size();
+  // int Size1 = Succ1->size(); // HLSL Change
+  int Size1 = Succ1->compute_size_no_dbg(); // HLSL Change
   int NLoads = 0;
   for (BasicBlock::iterator BBI = Succ0->begin(), BBE = Succ0->end();
        BBI != BBE;) {
@@ -529,7 +530,8 @@ bool MergedLoadStoreMotion::mergeStores(BasicBlock *T) {
     return false; // No. More than 2 predecessors.
 
   // #Instructions in Succ1 for Compile Time Control
-  int Size1 = Pred1->size();
+  // int Size1 = Succ1->size(); // HLSL Change
+  int Size1 = Pred1->compute_size_no_dbg(); // HLSL Change
   int NStores = 0;
 
   for (BasicBlock::reverse_iterator RBI = Pred0->rbegin(), RBE = Pred0->rend();

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
+      // <input> written as the option name.
+      if (newPair.Name == L"<input>") {
+        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;

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

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<IDxcCompiler> 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<IDxcUtils> pUtils;
+  VERIFY_SUCCEEDED(m_dllSupport.CreateInstance(CLSID_DxcUtils, &pUtils));
+
+
+
+  CComPtr<IDxcCompiler3> 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<IDxcResult> pResult;
+  VERIFY_SUCCEEDED(pCompiler3->Compile(&buf, args, _countof(args), nullptr, IID_PPV_ARGS(&pResult)));
+
+  CComPtr<IDxcBlob> 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 "<input>" to show up.
+          VERIFY_ARE_NOT_EQUAL(arg, L"<input>");
+        }
+      }
+    }
+    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<IDxcPdbUtils> pPdbUtils;
+  VERIFY_SUCCEEDED(m_dllSupport.CreateInstance(CLSID_DxcPdbUtils, &pPdbUtils));
+
+  VERIFY_SUCCEEDED(pPdbUtils->Load(pPdb));
+  TestPdb(pPdbUtils);
+
+  CComPtr<IDxcBlob> pFullPdb;
+  VERIFY_SUCCEEDED(pPdbUtils->GetFullPDB(&pFullPdb));
+  VERIFY_SUCCEEDED(pPdbUtils->Load(pFullPdb));
+  TestPdb(pPdbUtils);
+}
+
 TEST_F(CompilerTest, CompileThenTestPdbInPrivate) {
   CComPtr<IDxcCompiler> pCompiler;
   VERIFY_SUCCEEDED(CreateCompiler(&pCompiler));

tools/clang/unittests/HLSL/ExecutionTest.cpp

@@ -86,24 +86,6 @@ static bool contains(InputIterator b, InputIterator e, const T &val) {
   return e != std::find(b, e, val);
 }
 
-static HRESULT EnableExperimentalShaderModels() {
-  HMODULE hRuntime = LoadLibraryW(L"d3d12.dll");
-  if (hRuntime == NULL) {
-    return HRESULT_FROM_WIN32(GetLastError());
-  }
-
-  D3D12EnableExperimentalFeaturesFn pD3D12EnableExperimentalFeatures =
-    (D3D12EnableExperimentalFeaturesFn)GetProcAddress(hRuntime, "D3D12EnableExperimentalFeatures");
-  if (pD3D12EnableExperimentalFeatures == nullptr) {
-    FreeLibrary(hRuntime);
-    return HRESULT_FROM_WIN32(GetLastError());
-  }
-
-  HRESULT hr = pD3D12EnableExperimentalFeatures(1, &D3D12ExperimentalShaderModelsID, nullptr, nullptr);
-  FreeLibrary(hRuntime);
-  return hr;
-}
-
 static HRESULT ReportLiveObjects() {
   CComPtr<IDXGIDebug1> pDebug;
   IFR(DXGIGetDebugInterface1(0, IID_PPV_ARGS(&pDebug)));
@@ -461,6 +443,29 @@ public:
 
   const float ClearColor[4] = { 0.0f, 0.2f, 0.4f, 1.0f };
 
+  bool DivergentClassSetup() {
+    HRESULT hr;
+    hr = EnableExperimentalMode();
+    if (FAILED(hr)) {
+      LogCommentFmt(L"Unable to enable shader experimental mode - 0x%08x.", hr);
+    } else if (hr == S_FALSE) {
+      LogCommentFmt(L"Experimental mode not enabled.");
+    } else {
+      LogCommentFmt(L"Experimental mode enabled.");
+    }
+
+    hr = EnableDebugLayer();
+    if (FAILED(hr)) {
+      LogCommentFmt(L"Unable to enable debug layer - 0x%08x.", hr);
+    } else if (hr == S_FALSE) {
+      LogCommentFmt(L"Debug layer not enabled.");
+    } else {
+      LogCommentFmt(L"Debug layer enabled.");
+    }
+
+    return true;
+  }
+
 // Do not remove the following line - it is used by TranslateExecutionTest.py
 // MARKER: ExecutionTest/DxilConf Shared Implementation Start
 
@@ -1280,7 +1285,26 @@ public:
     return hr;
   }
 
-#ifndef _HLK_CONF
+  static HRESULT EnableExperimentalShaderModels() {
+    HMODULE hRuntime = LoadLibraryW(L"d3d12.dll");
+    if (hRuntime == NULL) {
+      return HRESULT_FROM_WIN32(GetLastError());
+    }
+
+    D3D12EnableExperimentalFeaturesFn pD3D12EnableExperimentalFeatures =
+        (D3D12EnableExperimentalFeaturesFn)GetProcAddress(
+            hRuntime, "D3D12EnableExperimentalFeatures");
+    if (pD3D12EnableExperimentalFeatures == nullptr) {
+      FreeLibrary(hRuntime);
+      return HRESULT_FROM_WIN32(GetLastError());
+    }
+
+    HRESULT hr = pD3D12EnableExperimentalFeatures(
+        1, &D3D12ExperimentalShaderModelsID, nullptr, nullptr);
+    FreeLibrary(hRuntime);
+    return hr;
+  }
+
   HRESULT EnableExperimentalMode() {
     if (m_ExperimentalModeEnabled) {
       return S_OK;
@@ -1294,7 +1318,6 @@ public:
     }
     return hr;
   }
-#endif
 
   struct FenceObj {
     HANDLE m_fenceEvent = NULL;
@@ -1432,35 +1455,7 @@ static void SetupComputeValuePattern(std::vector<uint32_t> &values,
 }
 
 bool ExecutionTest::ExecutionTestClassSetup() {
-#ifdef _HLK_CONF
-// TODO: Enabling the D3D driver verifier. Check out the logic in the D3DConf_12_Core test.
-    VERIFY_SUCCEEDED(m_support.Initialize());
-    m_UseWarp = hlsl_test::GetTestParamUseWARP(false);
-    m_EnableDebugLayer = hlsl_test::GetTestParamBool(L"DebugLayer");
-    if (m_EnableDebugLayer) {
-        EnableDebugLayer();
-    }
-    return true;
-#else
-  HRESULT hr = EnableExperimentalMode();
-  if (FAILED(hr)) {
-    LogCommentFmt(L"Unable to enable shader experimental mode - 0x%08x.", hr);
-  }
-  else if (hr == S_FALSE) {
-    LogCommentFmt(L"Experimental mode not enabled.");
-  }
-  else {
-    LogCommentFmt(L"Experimental mode enabled.");
-  }
-  hr = EnableDebugLayer();
-  if (FAILED(hr)) {
-    LogCommentFmt(L"Unable to enable debug layer - 0x%08x.", hr);
-  }
-  else {
-    LogCommentFmt(L"Debug layer enabled.");
-  }
-  return true;
-#endif
+  return DivergentClassSetup();
 }
 
 void ExecutionTest::RunRWByteBufferComputeTest(ID3D12Device *pDevice, LPCSTR pShader, std::vector<uint32_t> &values) {
@@ -9570,7 +9565,7 @@ static void WriteReadBackDump(st::ShaderOp *pShaderOp, st::ShaderOpTest *pTest,
 // It's exclusive with the use of the DLL as a TAEF target.
 extern "C" {
   __declspec(dllexport) HRESULT WINAPI InitializeOpTests(void *pStrCtx, st::OutputStringFn pOutputStrFn) {
-    HRESULT hr = EnableExperimentalShaderModels();
+    HRESULT hr = ExecutionTest::EnableExperimentalShaderModels();
     if (FAILED(hr)) {
       pOutputStrFn(pStrCtx, L"Unable to enable experimental shader models.\r\n.");
     }