Fix #6: Produce dxil container output when validation is disabled. (#7)

* Fix #6: Produce dxil container output when validation is disabled.

Make sure that we wrap the llvm bitcode module in a dxil container
even when validation is disabled.

Note that we still produce a raw bitcode module output when only a
high- level compilation is requested by /fcgl.

When the llvm module is serialized to a dxil container we strip the
debug info. When using the internal validator we need to make a copy of
the llvm module before serializing so that debug info is present for
validation error messages.

Added a test to check that validation error messages have valid debug
info.

tools/clang/test/HLSL/val-wave-failures-ps.hlsl

@@ -1,8 +1,8 @@
 // RUN: %dxc -E main -T ps_6_0 %s /Zi | FileCheck %s
 
-// CHECK: Gradient operations are not affected by wave-sensitive data or control flow.
-// CHECK: Gradient operations are not affected by wave-sensitive data or control flow.
-// CHECK: Gradient operations are not affected by wave-sensitive data or control flow.
+// CHECK: val-wave-failures-ps.hlsl:10:9  Gradient operations are not affected by wave-sensitive data or control flow.
+// CHECK: val-wave-failures-ps.hlsl:30:12 Gradient operations are not affected by wave-sensitive data or control flow.
+// CHECK: val-wave-failures-ps.hlsl:40:12 Gradient operations are not affected by wave-sensitive data or control flow.
 
 float4 main(float4 p: SV_Position) : SV_Target {
   // cannot feed into ddx

tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -1784,6 +1784,36 @@ static void PrintPipelineStateValidationRuntimeInfo(const char *pBuffer, DXIL::S
   OS << comment << "\n";
 }
 
+// Class to manage lifetime of llvm module and provide some utility
+// functions used for generating compiler output.
+class DxilCompilerLLVMModuleOutput {
+public:
+  DxilCompilerLLVMModuleOutput(std::unique_ptr<llvm::Module> module)
+    : m_llvmModule(std::move(module))
+  { }
+
+  void CloneForDebugInfo() {
+      m_llvmModuleWithDebugInfo.reset(llvm::CloneModule(m_llvmModule.get()));
+  }
+
+ void WrapModuleInDxilContainer(IMalloc *pMalloc,  AbstractMemoryStream *pModuleBitcode, CComPtr<IDxcBlob> &pDxilContainerBlob) {
+    CComPtr<AbstractMemoryStream> pContainerStream;
+    IFT(CreateMemoryStream(pMalloc, &pContainerStream));
+    SerializeDxilContainerForModule(m_llvmModule.get(), pModuleBitcode, pContainerStream);
+
+    pDxilContainerBlob.Release();
+    IFT(pContainerStream.QueryInterface(&pDxilContainerBlob));
+  }
+
+  llvm::Module *get() { return m_llvmModule.get(); }
+  llvm::Module *getWithDebugInfo() { return m_llvmModuleWithDebugInfo.get(); }
+
+private:
+  std::unique_ptr<llvm::Module> m_llvmModule;
+  std::unique_ptr<llvm::Module> m_llvmModuleWithDebugInfo;
+};
+
+
 class DxcCompiler : public IDxcCompiler, public IDxcLangExtensions, public IDxcContainerEvent {
 private:
   DXC_MICROCOM_REF_FIELD(m_dwRef)
@@ -2002,53 +2032,38 @@ public:
         action.BeginSourceFile(compiler, file);
         action.Execute();
         action.EndSourceFile();
+        outStream.flush();
 
-        // Don't do work to put in a container if an error has occurred, or if
-        // there is only a a high-level representation in the module.
-        
+        // Don't do work to put in a container if an error has occurred
         bool compileOK = !compiler.getDiagnostics().hasErrorOccurred();
         if (compileOK) {
           HRESULT valHR = S_OK;
-          if (needsValidation) {
-            outStream.flush();
-
-            CComPtr<AbstractMemoryStream> pFinalStream;
-            IFT(CreateMemoryStream(pMalloc, &pFinalStream));
-
-            llvm::Module *pDebugModule, *pOrigModule;
-            std::unique_ptr<llvm::Module> llvmModule = action.takeModule();
-            std::unique_ptr<llvm::Module> llvmClonedModule;
-            if (internalValidator && opts.DebugInfo) {
-              // If using the internal validator, we'll use the modules directly.
-              // In this case, we'll want to make a clone to avoid SerializeDxilContainerForModule
-              // destroying this.
-              llvmClonedModule.reset(llvm::CloneModule(llvmModule.get()));
-              pDebugModule = llvmClonedModule.get();
-              SerializeDxilContainerForModule(llvmModule.get(), pOutputStream, pFinalStream);
-              pOrigModule = llvmModule.get();
-            }
-            else {
-              pOrigModule = llvmModule.get();
-              pDebugModule = nullptr;
-              SerializeDxilContainerForModule(llvmModule.get(), pOutputStream, pFinalStream);
-            }
-            pOutputBlob.Release();
-            IFT(pFinalStream.QueryInterface(&pOutputBlob));
 
-            CComPtr<IDxcBlob> pFinalStreamBlob;
-            CComPtr<IDxcOperationResult> pValResult;
-            IFT(pFinalStream.QueryInterface(&pFinalStreamBlob));
+          // Take ownership of the module from the action.
+          DxilCompilerLLVMModuleOutput llvmModule(action.takeModule());
+
+          // If using the internal validator, we'll use the modules directly.
+          // In this case, we'll want to make a clone to avoid SerializeDxilContainerForModule
+          // stripping all the debug info. The debug info will be stripped from the orginal
+          // module, but preserved in the cloned module.
+          if (internalValidator && opts.DebugInfo)
+            llvmModule.CloneForDebugInfo();
 
+          // Do not create a container when there is only a a high-level representation in the module.
+          if (!opts.CodeGenHighLevel)
+            llvmModule.WrapModuleInDxilContainer(pMalloc, pOutputStream, pOutputBlob);
+
+          if (needsValidation) {
             // Important: in-place edit is required so the blob is reused and thus
             // dxil.dll can be released.
             if (internalValidator) {
               IFT(RunInternalValidator(
-                pValidator, pOrigModule, pDebugModule, pFinalStreamBlob,
+                pValidator, llvmModule.get(), llvmModule.getWithDebugInfo(), pOutputBlob,
                 DxcValidatorFlags_InPlaceEdit, &pValResult));
             }
             else {
               IFT(pValidator->Validate(
-                pFinalStreamBlob, DxcValidatorFlags_InPlaceEdit, &pValResult));
+                pOutputBlob, DxcValidatorFlags_InPlaceEdit, &pValResult));
             }
             IFT(pValResult->GetStatus(&valHR));
             if (FAILED(valHR)) {

tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -286,6 +286,7 @@ public:
 
   TEST_METHOD(CompileWhenODumpThenPassConfig)
   TEST_METHOD(CompileWhenODumpThenOptimizerMatch)
+  TEST_METHOD(CompileWhenVdThenProducesDxilContainer)
 
   TEST_METHOD(CompileWhenShaderModelMismatchAttributeThenFail)
   TEST_METHOD(CompileBadHlslThenFail)
@@ -1524,6 +1525,24 @@ TEST_F(CompilerTest, CompileWhenODumpThenPassConfig) {
   VERIFY_ARE_NOT_EQUAL(string::npos, passes.find("inline"));
 }
 
+TEST_F(CompilerTest, CompileWhenVdThenProducesDxilContainer) {
+  CComPtr<IDxcCompiler> pCompiler;
+  CComPtr<IDxcOperationResult> pResult;
+  CComPtr<IDxcBlobEncoding> pSource;
+
+  VERIFY_SUCCEEDED(CreateCompiler(&pCompiler));
+  CreateBlobFromText(EmptyCompute, &pSource);
+
+  LPCWSTR Args[] = { L"/Vd" };
+
+  VERIFY_SUCCEEDED(pCompiler->Compile(pSource, L"source.hlsl", L"main",
+    L"cs_6_0", Args, _countof(Args), nullptr, 0, nullptr, &pResult));
+  VerifyOperationSucceeded(pResult);
+  CComPtr<IDxcBlob> pResultBlob;
+  VERIFY_SUCCEEDED(pResult->GetResult(&pResultBlob));
+  VERIFY_IS_TRUE(hlsl::IsValidDxilContainer(reinterpret_cast<hlsl::DxilContainerHeader *>(pResultBlob->GetBufferPointer()), pResultBlob->GetBufferSize()));
+}
+
 TEST_F(CompilerTest, CompileWhenODumpThenOptimizerMatch) {
   LPCWSTR OptLevels[] = { L"/Od", L"/O1", L"/O2" };
   CComPtr<IDxcCompiler> pCompiler;