From 7fc02979135d5654535507d40bc6cc70335d1726 Mon Sep 17 00:00:00 2001 From: David Peixotto Date: Mon, 9 Jan 2017 13:27:28 -0800 Subject: [PATCH 1/2] 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. --- .../clang/tools/dxcompiler/dxcompilerobj.cpp | 28 +++++++++---------- tools/clang/unittests/HLSL/CompilerTest.cpp | 19 +++++++++++++ 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index c5f2d7aef4..77166c0137 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -1824,6 +1824,15 @@ class DxcCompiler : public IDxcCompiler, public IDxcLangExtensions, public IDxcC DXASSERT(!opts.HLSL2015, "else ReadDxcOpts didn't fail for non-isense"); finished = false; } + + void WrapModuleInDxilContainer(IMalloc *pMalloc, llvm::Module *llvmModule, AbstractMemoryStream *pModuleBitcode, CComPtr &pDxilContainerBlob) { + CComPtr pContainerStream; + IFT(CreateMemoryStream(pMalloc, &pContainerStream)); + SerializeDxilContainerForModule(llvmModule, pModuleBitcode, pContainerStream); + + pDxilContainerBlob.Release(); + IFT(pContainerStream.QueryInterface(&pDxilContainerBlob)); + } public: DXC_MICROCOM_ADDREF_RELEASE_IMPL(m_dwRef) DXC_LANGEXTENSIONS_HELPER_IMPL(m_langExtensionsHelper) @@ -2009,14 +2018,13 @@ class DxcCompiler : public IDxcCompiler, public IDxcLangExtensions, public IDxcC bool compileOK = !compiler.getDiagnostics().hasErrorOccurred(); if (compileOK) { HRESULT valHR = S_OK; + std::unique_ptr llvmModule = action.takeModule(); + if (!opts.CodeGenHighLevel) + WrapModuleInDxilContainer(pMalloc, llvmModule.get(), pOutputStream, pOutputBlob); if (needsValidation) { outStream.flush(); - CComPtr pFinalStream; - IFT(CreateMemoryStream(pMalloc, &pFinalStream)); - llvm::Module *pDebugModule, *pOrigModule; - std::unique_ptr llvmModule = action.takeModule(); std::unique_ptr llvmClonedModule; if (internalValidator && opts.DebugInfo) { // If using the internal validator, we'll use the modules directly. @@ -2024,31 +2032,23 @@ class DxcCompiler : public IDxcCompiler, public IDxcLangExtensions, public IDxcC // 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 pFinalStreamBlob; - CComPtr pValResult; - IFT(pFinalStream.QueryInterface(&pFinalStreamBlob)); // 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, pOrigModule, pDebugModule, pOutputBlob, DxcValidatorFlags_InPlaceEdit, &pValResult)); } else { IFT(pValidator->Validate( - pFinalStreamBlob, DxcValidatorFlags_InPlaceEdit, &pValResult)); + pOutputBlob, DxcValidatorFlags_InPlaceEdit, &pValResult)); } IFT(pValResult->GetStatus(&valHR)); if (FAILED(valHR)) { diff --git a/tools/clang/unittests/HLSL/CompilerTest.cpp b/tools/clang/unittests/HLSL/CompilerTest.cpp index 6127e63507..a2a1ef8ded 100644 --- a/tools/clang/unittests/HLSL/CompilerTest.cpp +++ b/tools/clang/unittests/HLSL/CompilerTest.cpp @@ -286,6 +286,7 @@ class CompilerTest { 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 pCompiler; + CComPtr pResult; + CComPtr 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 pResultBlob; + VERIFY_SUCCEEDED(pResult->GetResult(&pResultBlob)); + VERIFY_IS_TRUE(hlsl::IsValidDxilContainer(reinterpret_cast(pResultBlob->GetBufferPointer()), pResultBlob->GetBufferSize())); +} + TEST_F(CompilerTest, CompileWhenODumpThenOptimizerMatch) { LPCWSTR OptLevels[] = { L"/Od", L"/O1", L"/O2" }; CComPtr pCompiler; From aa1303f9448639b234af9a3c825e0f04a6d7b135 Mon Sep 17 00:00:00 2001 From: David Peixotto Date: Wed, 11 Jan 2017 15:30:02 -0800 Subject: [PATCH 2/2] Clone llvm module for debug info before serializing to container 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. --- .../clang/test/HLSL/val-wave-failures-ps.hlsl | 6 +- .../clang/tools/dxcompiler/dxcompilerobj.cpp | 79 +++++++++++-------- 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/tools/clang/test/HLSL/val-wave-failures-ps.hlsl b/tools/clang/test/HLSL/val-wave-failures-ps.hlsl index 817a0bfd37..b3cf9b1764 100644 --- a/tools/clang/test/HLSL/val-wave-failures-ps.hlsl +++ b/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 diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 77166c0137..28b2fdec86 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/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 module) + : m_llvmModule(std::move(module)) + { } + + void CloneForDebugInfo() { + m_llvmModuleWithDebugInfo.reset(llvm::CloneModule(m_llvmModule.get())); + } + + void WrapModuleInDxilContainer(IMalloc *pMalloc, AbstractMemoryStream *pModuleBitcode, CComPtr &pDxilContainerBlob) { + CComPtr 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 m_llvmModule; + std::unique_ptr m_llvmModuleWithDebugInfo; +}; + + class DxcCompiler : public IDxcCompiler, public IDxcLangExtensions, public IDxcContainerEvent { private: DXC_MICROCOM_REF_FIELD(m_dwRef) @@ -1824,15 +1854,6 @@ class DxcCompiler : public IDxcCompiler, public IDxcLangExtensions, public IDxcC DXASSERT(!opts.HLSL2015, "else ReadDxcOpts didn't fail for non-isense"); finished = false; } - - void WrapModuleInDxilContainer(IMalloc *pMalloc, llvm::Module *llvmModule, AbstractMemoryStream *pModuleBitcode, CComPtr &pDxilContainerBlob) { - CComPtr pContainerStream; - IFT(CreateMemoryStream(pMalloc, &pContainerStream)); - SerializeDxilContainerForModule(llvmModule, pModuleBitcode, pContainerStream); - - pDxilContainerBlob.Release(); - IFT(pContainerStream.QueryInterface(&pDxilContainerBlob)); - } public: DXC_MICROCOM_ADDREF_RELEASE_IMPL(m_dwRef) DXC_LANGEXTENSIONS_HELPER_IMPL(m_langExtensionsHelper) @@ -2011,39 +2032,33 @@ class DxcCompiler : public IDxcCompiler, public IDxcLangExtensions, public IDxcC 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; - std::unique_ptr llvmModule = action.takeModule(); + + // 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) - WrapModuleInDxilContainer(pMalloc, llvmModule.get(), pOutputStream, pOutputBlob); - if (needsValidation) { - outStream.flush(); - - llvm::Module *pDebugModule, *pOrigModule; - std::unique_ptr 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(); - pOrigModule = llvmModule.get(); - } - else { - pOrigModule = llvmModule.get(); - pDebugModule = nullptr; - } + 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, pOutputBlob, + pValidator, llvmModule.get(), llvmModule.getWithDebugInfo(), pOutputBlob, DxcValidatorFlags_InPlaceEdit, &pValResult)); } else {