Fix outputs -Fre, -Fsh, -Frs, and -Fc with -Fh (microsoft#2716)

- Fix output naming for StringRef - Support /Fc when /Fh is provided - Fix root sig strip flag - Create root sig stream output - Validate -Frs root sig output for signing with DXIL.dll - Move outputs under valHR success branch - Add test for -Fre, -Fsh, -Frs, and -Fc with -Fh - Rework hcttestcmds.cmd completely: checks a lot more stuff and is way more robust, while being much easier to read, add to, and maintain, I hope.
pow2clk · Feb 25, 2020 · 367dd44 · 367dd44
1 parent eb33030
commit 367dd44
Show file tree

Hide file tree

Showing 5 changed files with 554 additions and 783 deletions.
diff --git a/include/dxc/Support/dxcapi.impl.h b/include/dxc/Support/dxcapi.impl.h
@@ -189,7 +189,7 @@ struct DxcOutputObject {
   }
   HRESULT SetName(_In_opt_z_ llvm::StringRef Name) {
     DXASSERT_NOMSG(!name);
-    if (!Name.empty())
+    if (Name.empty())
       return S_OK;
     CComPtr<IDxcBlobEncoding> pBlobEncoding;
     IFR(TranslateUtf8StringForOutput(Name.data(), Name.size(), DXC_CP_UTF16, &pBlobEncoding));

diff --git a/tools/clang/tools/dxclib/dxc.cpp b/tools/clang/tools/dxclib/dxc.cpp
@@ -302,15 +302,22 @@ int DxcContext::ActOnBlob(IDxcBlob *pBlob, IDxcBlob *pDebugBlob, LPCWSTR pDebugB
 #endif // ENABLE_SPIRV_CODEGEN
   // SPIRV Change Ends
 
+  bool disassemblyWritten = false;
   if (!m_Opts.OutputHeader.empty()) {
     llvm::Twine varName = m_Opts.VariableName.empty()
                               ? llvm::Twine("g_", m_Opts.EntryPoint)
                               : m_Opts.VariableName;
     WriteHeader(pDisassembleResult, pBlob, varName,
                 StringRefUtf16(m_Opts.OutputHeader));
-  } else if (!m_Opts.AssemblyCode.empty()) {
+    disassemblyWritten = true;
+  }
+
+  if (!m_Opts.AssemblyCode.empty()) {
     WriteBlobToFile(pDisassembleResult, m_Opts.AssemblyCode, m_Opts.DefaultTextCodePage);
-  } else {
+    disassemblyWritten = true;
+  }
+
+  if (!disassemblyWritten) {
     WriteBlobToConsole(pDisassembleResult);
   }
   return retVal;

diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp
@@ -787,11 +787,9 @@ class DxcCompiler : public IDxcCompiler3,
         if (!opts.StripReflection) {
           SerializeFlags |= SerializeDxilFlags::IncludeReflectionPart;
         }
-        // StripRootSignature disabled on Xbox for now since
-        // API wrappers rely on embedded root signature instead of separate output.
-        //if (opts.StripRootSignature) {
-        //  SerializeFlags |= SerializeDxilFlags::StripRootSignature;
-        //}
+        if (opts.StripRootSignature) {
+          SerializeFlags |= SerializeDxilFlags::StripRootSignature;
+        }
 
         // Don't do work to put in a container if an error has occurred
         // Do not create a container when there is only a a high-level representation in the module.
@@ -800,6 +798,7 @@ class DxcCompiler : public IDxcCompiler3,
           CComPtr<AbstractMemoryStream> pReflectionStream;
           CComPtr<AbstractMemoryStream> pRootSigStream;
           IFT(CreateMemoryStream(DxcGetThreadMallocNoRef(), &pReflectionStream));
+          IFT(CreateMemoryStream(DxcGetThreadMallocNoRef(), &pRootSigStream));
 
           dxcutil::AssembleInputs inputs(
                 action.takeModule(), pOutputBlob, m_pMalloc, SerializeFlags,
@@ -811,16 +810,6 @@ class DxcCompiler : public IDxcCompiler3,
           } else {
             dxcutil::AssembleToContainer(inputs);
           }
-          if (pReflectionStream && pReflectionStream->GetPtrSize()) {
-            CComPtr<IDxcBlob> pReflection;
-            IFT(pReflectionStream->QueryInterface(&pReflection));
-            IFT(pResult->SetOutputObject(DXC_OUT_REFLECTION, pReflection));
-          }
-          if (pRootSigStream && pRootSigStream->GetPtrSize()) {
-            CComPtr<IDxcBlob> pRootSignature;
-            IFT(pRootSigStream->QueryInterface(&pRootSignature));
-            IFT(pResult->SetOutputObject(DXC_OUT_ROOT_SIGNATURE, pRootSignature));
-          }
 
           // Callback after valid DXIL is produced
           if (SUCCEEDED(valHR)) {
@@ -843,13 +832,27 @@ class DxcCompiler : public IDxcCompiler3,
                 }
               }
             }
-          }
 
-          // Always make hash blob for output
-          CComPtr<IDxcBlob> pHashBlob;
-          IFT(hlsl::DxcCreateBlobOnHeapCopy(&ShaderHashContent, (UINT32)sizeof(ShaderHashContent), &pHashBlob));
-          IFT(pResult->SetOutputObject(DXC_OUT_SHADER_HASH, pHashBlob));
-        }
+            if (pReflectionStream && pReflectionStream->GetPtrSize()) {
+              CComPtr<IDxcBlob> pReflection;
+              IFT(pReflectionStream->QueryInterface(&pReflection));
+              IFT(pResult->SetOutputObject(DXC_OUT_REFLECTION, pReflection));
+            }
+            if (pRootSigStream && pRootSigStream->GetPtrSize()) {
+              CComPtr<IDxcBlob> pRootSignature;
+              IFT(pRootSigStream->QueryInterface(&pRootSignature));
+              if (needsValidation) {
+                CComPtr<IDxcBlobEncoding> pValErrors;
+                // Validation failure communicated through diagnostic error
+                dxcutil::ValidateRootSignatureInContainer(pRootSignature, &compiler.getDiagnostics());
+              }
+              IFT(pResult->SetOutputObject(DXC_OUT_ROOT_SIGNATURE, pRootSignature));
+            }
+            CComPtr<IDxcBlob> pHashBlob;
+            IFT(hlsl::DxcCreateBlobOnHeapCopy(&ShaderHashContent, (UINT32)sizeof(ShaderHashContent), &pHashBlob));
+            IFT(pResult->SetOutputObject(DXC_OUT_SHADER_HASH, pHashBlob));
+          } // SUCCEEDED(valHR)
+        } // compileOK && !opts.CodeGenHighLevel
       }
 
       // Add std err to warnings.

diff --git a/utils/hct/cmdtestfiles/smoke.hlsl b/utils/hct/cmdtestfiles/smoke.hlsl
@@ -1,4 +1,6 @@
 int g;
+static int g_unused;
+
 #ifndef semantic
 #define semantic SV_Target
 #endif