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

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

Author: tex3d

include/llvm/IR/BasicBlock.h

@@ -245,6 +245,8 @@ class BasicBlock : public Value, // Basic blocks are data objects also
   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 @@ class OptTable {
                    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 @@ struct DxcPdbUtils : public IDxcPdbUtils, public IDxcPixDxilDebugInfoFactory
   }
 
   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 @@ class CompilerTest : public ::testing::Test {
   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));