From ffe1d013dffa75df8daedd8d29e561053c1cc7de Mon Sep 17 00:00:00 2001 From: Adam Yang Date: Wed, 17 Mar 2021 17:01:20 -0700 Subject: [PATCH 01/11] Made line number in error reports work even when without Zi --- include/llvm/IR/DebugInfo.h | 1 + lib/HLSL/HLMatrixLowerPass.cpp | 2 +- lib/IR/DebugInfo.cpp | 2 ++ lib/Transforms/Scalar/LowerTypePasses.cpp | 2 +- lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp | 2 +- tools/clang/tools/dxcompiler/dxcompilerobj.cpp | 4 ++++ 6 files changed, 10 insertions(+), 3 deletions(-) diff --git a/include/llvm/IR/DebugInfo.h b/include/llvm/IR/DebugInfo.h index 7d03673844..fdfc5f7eae 100644 --- a/include/llvm/IR/DebugInfo.h +++ b/include/llvm/IR/DebugInfo.h @@ -60,6 +60,7 @@ bool stripDebugInfo(Function &F); /// \brief Return Debug Info Metadata Version by checking module flags. unsigned getDebugMetadataVersionFromModule(const Module &M); +bool hasCompileUnits(const Module &M); // HLSL Change /// \brief Utility to find all debug info in a module. /// diff --git a/lib/HLSL/HLMatrixLowerPass.cpp b/lib/HLSL/HLMatrixLowerPass.cpp index db3649d635..beddb2b16e 100644 --- a/lib/HLSL/HLMatrixLowerPass.cpp +++ b/lib/HLSL/HLMatrixLowerPass.cpp @@ -197,7 +197,7 @@ bool HLMatrixLowerPass::runOnModule(Module &M) { m_pHLModule = &m_pModule->GetOrCreateHLModule(); // Load up debug information, to cross-reference values and the instructions // used to load them. - m_HasDbgInfo = getDebugMetadataVersionFromModule(M) != 0; + m_HasDbgInfo = hasCompileUnits(M); m_matToVecStubs = &matToVecStubs; m_vecToMatStubs = &vecToMatStubs; diff --git a/lib/IR/DebugInfo.cpp b/lib/IR/DebugInfo.cpp index 02e1383806..812385874c 100644 --- a/lib/IR/DebugInfo.cpp +++ b/lib/IR/DebugInfo.cpp @@ -379,6 +379,8 @@ unsigned llvm::getDebugMetadataVersionFromModule(const Module &M) { return 0; } +bool llvm::hasCompileUnits(const Module &M) { return nullptr != M.getNamedMetadata("llvm.dbg.cu"); } // HLSL Change + DenseMap llvm::makeSubprogramMap(const Module &M) { DenseMap R; diff --git a/lib/Transforms/Scalar/LowerTypePasses.cpp b/lib/Transforms/Scalar/LowerTypePasses.cpp index f464ef0a37..b370b9aace 100644 --- a/lib/Transforms/Scalar/LowerTypePasses.cpp +++ b/lib/Transforms/Scalar/LowerTypePasses.cpp @@ -130,7 +130,7 @@ bool LowerTypePass::runOnModule(Module &M) { initialize(M); // Load up debug information, to cross-reference values and the instructions // used to load them. - bool HasDbgInfo = getDebugMetadataVersionFromModule(M) != 0; + bool HasDbgInfo = llvm::hasCompileUnits(M) != 0; llvm::DebugInfoFinder Finder; if (HasDbgInfo) { Finder.processModule(M); diff --git a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp index ec8ae92806..473937931f 100644 --- a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp +++ b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp @@ -3905,7 +3905,7 @@ class SROA_Parameter_HLSL : public ModulePass { const DataLayout &DL = M.getDataLayout(); // Load up debug information, to cross-reference values and the instructions // used to load them. - m_HasDbgInfo = getDebugMetadataVersionFromModule(M) != 0; + m_HasDbgInfo = nullptr != M.getNamedMetadata("llvm.dbg.cu"); InjectReturnAfterNoReturnPreserveOutput(*m_pHLModule); diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 50718c0488..6f193102cb 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -1252,6 +1252,10 @@ class DxcCompiler : public IDxcCompiler3, // TODO: consider // DebugPass, DebugCompilationDir, DwarfDebugFlags, SplitDwarfFile } + else { + CodeGenOptions &CGOpts = compiler.getCodeGenOpts(); + CGOpts.setDebugInfo(CodeGenOptions::LocTrackingOnly); + } clang::PreprocessorOptions &PPOpts(compiler.getPreprocessorOpts()); for (size_t i = 0; i < defines.size(); ++i) { From 93373dae8bf844c7bf1dde44f32eba4edbaea286 Mon Sep 17 00:00:00 2001 From: Adam Yang Date: Thu, 18 Mar 2021 14:15:42 -0700 Subject: [PATCH 02/11] Replaced the hasCompileUnits function with a more easy to understand hasDebugInfo function --- include/llvm/IR/DebugInfo.h | 2 +- lib/DXIL/DxilUtil.cpp | 2 +- lib/DxilContainer/DxilContainerAssembler.cpp | 16 +++++----------- lib/HLSL/DxilCondenseResources.cpp | 2 +- lib/HLSL/DxilGenerationPass.cpp | 2 +- lib/HLSL/HLMatrixLowerPass.cpp | 2 +- lib/IR/DebugInfo.cpp | 15 ++++++++++++++- lib/Transforms/Scalar/LowerTypePasses.cpp | 2 +- lib/Transforms/Scalar/Scalarizer.cpp | 2 +- 9 files changed, 26 insertions(+), 19 deletions(-) diff --git a/include/llvm/IR/DebugInfo.h b/include/llvm/IR/DebugInfo.h index fdfc5f7eae..ca3c8a4d06 100644 --- a/include/llvm/IR/DebugInfo.h +++ b/include/llvm/IR/DebugInfo.h @@ -60,7 +60,7 @@ bool stripDebugInfo(Function &F); /// \brief Return Debug Info Metadata Version by checking module flags. unsigned getDebugMetadataVersionFromModule(const Module &M); -bool hasCompileUnits(const Module &M); // HLSL Change +bool hasDebugInfo(const Module &M); // HLSL Change - Helper function to check if there's real debug info (variables, types) /// \brief Utility to find all debug info in a module. /// diff --git a/lib/DXIL/DxilUtil.cpp b/lib/DXIL/DxilUtil.cpp index 143f219309..453231a36b 100644 --- a/lib/DXIL/DxilUtil.cpp +++ b/lib/DXIL/DxilUtil.cpp @@ -314,7 +314,7 @@ static void EmitWarningOrErrorOnGlobalVariable(llvm::LLVMContext &Ctx, GlobalVar if (GV) { Module &M = *GV->getParent(); - if (getDebugMetadataVersionFromModule(M) != 0) { + if (hasDebugInfo(M)) { DebugInfoFinder FinderObj; DebugInfoFinder &Finder = FinderObj; // Debug modules have no dxil modules. Use it if you got it. diff --git a/lib/DxilContainer/DxilContainerAssembler.cpp b/lib/DxilContainer/DxilContainerAssembler.cpp index 745d2017a2..e105a65da1 100644 --- a/lib/DxilContainer/DxilContainerAssembler.cpp +++ b/lib/DxilContainer/DxilContainerAssembler.cpp @@ -1531,15 +1531,10 @@ DxilContainerWriter *hlsl::NewDxilContainerWriter() { return new DxilContainerWriter_impl(); } -static bool HasDebugInfo(const Module &M) { - for (Module::const_named_metadata_iterator NMI = M.named_metadata_begin(), - NME = M.named_metadata_end(); - NMI != NME; ++NMI) { - if (NMI->getName().startswith("llvm.dbg.")) { - return true; - } - } - return false; +static bool HasDebugInfoOrLineNumbers(const Module &M) { + return + llvm::getDebugMetadataVersionFromModule(M) != 0 || + llvm::hasDebugInfo(M); } static void GetPaddedProgramPartSize(AbstractMemoryStream *pStream, @@ -1776,8 +1771,7 @@ void hlsl::SerializeDxilContainerForModule(DxilModule *pModule, // If we have debug information present, serialize it to a debug part, then use the stripped version as the canonical program version. CComPtr pProgramStream = pInputProgramStream; bool bModuleStripped = false; - bool bHasDebugInfo = HasDebugInfo(*pModule->GetModule()); - if (bHasDebugInfo) { + if (HasDebugInfoOrLineNumbers(*pModule->GetModule())) { uint32_t debugInUInt32, debugPaddingBytes; GetPaddedProgramPartSize(pInputProgramStream, debugInUInt32, debugPaddingBytes); if (Flags & SerializeDxilFlags::IncludeDebugInfoPart) { diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index d0a8b8916c..3b2396b323 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -560,7 +560,7 @@ class DxilLowerCreateHandleForLib : public ModulePass { // Load up debug information, to cross-reference values and the instructions // used to load them. - m_HasDbgInfo = getDebugMetadataVersionFromModule(M) != 0; + m_HasDbgInfo = hasDebugInfo(M); GenerateDxilResourceHandles(); diff --git a/lib/HLSL/DxilGenerationPass.cpp b/lib/HLSL/DxilGenerationPass.cpp index 3653b85a26..ca3b1dbfb0 100644 --- a/lib/HLSL/DxilGenerationPass.cpp +++ b/lib/HLSL/DxilGenerationPass.cpp @@ -201,7 +201,7 @@ class DxilGenerationPass : public ModulePass { // Load up debug information, to cross-reference values and the instructions // used to load them. - m_HasDbgInfo = getDebugMetadataVersionFromModule(M) != 0; + m_HasDbgInfo = hasDebugInfo(M); // EntrySig for shader functions. DxilEntryPropsMap EntryPropsMap; diff --git a/lib/HLSL/HLMatrixLowerPass.cpp b/lib/HLSL/HLMatrixLowerPass.cpp index beddb2b16e..d036c80f7a 100644 --- a/lib/HLSL/HLMatrixLowerPass.cpp +++ b/lib/HLSL/HLMatrixLowerPass.cpp @@ -197,7 +197,7 @@ bool HLMatrixLowerPass::runOnModule(Module &M) { m_pHLModule = &m_pModule->GetOrCreateHLModule(); // Load up debug information, to cross-reference values and the instructions // used to load them. - m_HasDbgInfo = hasCompileUnits(M); + m_HasDbgInfo = hasDebugInfo(M); m_matToVecStubs = &matToVecStubs; m_vecToMatStubs = &vecToMatStubs; diff --git a/lib/IR/DebugInfo.cpp b/lib/IR/DebugInfo.cpp index 812385874c..9c74aab3bd 100644 --- a/lib/IR/DebugInfo.cpp +++ b/lib/IR/DebugInfo.cpp @@ -379,7 +379,20 @@ unsigned llvm::getDebugMetadataVersionFromModule(const Module &M) { return 0; } -bool llvm::hasCompileUnits(const Module &M) { return nullptr != M.getNamedMetadata("llvm.dbg.cu"); } // HLSL Change +// HLSL Change - begin +bool llvm::hasDebugInfo(const Module &M) { + // We might just get away with checking if there's "llvm.dbg.cu", + // but this is more robust. + for (Module::const_named_metadata_iterator NMI = M.named_metadata_begin(), + NME = M.named_metadata_end(); + NMI != NME; ++NMI) { + if (NMI->getName().startswith("llvm.dbg.")) { + return true; + } + } + return false; +} +// HLSL Change - end DenseMap llvm::makeSubprogramMap(const Module &M) { diff --git a/lib/Transforms/Scalar/LowerTypePasses.cpp b/lib/Transforms/Scalar/LowerTypePasses.cpp index b370b9aace..76deb74f40 100644 --- a/lib/Transforms/Scalar/LowerTypePasses.cpp +++ b/lib/Transforms/Scalar/LowerTypePasses.cpp @@ -130,7 +130,7 @@ bool LowerTypePass::runOnModule(Module &M) { initialize(M); // Load up debug information, to cross-reference values and the instructions // used to load them. - bool HasDbgInfo = llvm::hasCompileUnits(M) != 0; + bool HasDbgInfo = llvm::hasDebugInfo(M); llvm::DebugInfoFinder Finder; if (HasDbgInfo) { Finder.processModule(M); diff --git a/lib/Transforms/Scalar/Scalarizer.cpp b/lib/Transforms/Scalar/Scalarizer.cpp index c9429d9f62..2872ad0850 100644 --- a/lib/Transforms/Scalar/Scalarizer.cpp +++ b/lib/Transforms/Scalar/Scalarizer.cpp @@ -729,7 +729,7 @@ bool Scalarizer::finish() { Module &M = *Gathered.front().first->getModule(); LLVMContext &Ctx = M.getContext(); const DataLayout &DL = M.getDataLayout(); - bool HasDbgInfo = getDebugMetadataVersionFromModule(M) != 0; + bool HasDbgInfo = hasDebugInfo(M); // Map from an extract element inst to a Value which replaced it. DenseMap EltMap; // HLSL Change Ends. From b89300d51d1f4109dc5ab5ad04dc90da3c544dff Mon Sep 17 00:00:00 2001 From: Adam Yang Date: Thu, 18 Mar 2021 14:47:14 -0700 Subject: [PATCH 03/11] Fixed the things for the validator too --- tools/clang/tools/dxcompiler/dxclinker.cpp | 2 +- tools/clang/tools/dxcompiler/dxcompilerobj.cpp | 2 +- tools/clang/tools/dxcompiler/dxcutil.cpp | 5 ++--- tools/clang/tools/dxcompiler/dxcutil.h | 2 -- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/tools/clang/tools/dxcompiler/dxclinker.cpp b/tools/clang/tools/dxcompiler/dxclinker.cpp index 1674cc3ca4..dabf8a0ab2 100644 --- a/tools/clang/tools/dxcompiler/dxclinker.cpp +++ b/tools/clang/tools/dxcompiler/dxclinker.cpp @@ -274,7 +274,7 @@ HRESULT STDMETHODCALLTYPE DxcLinker::Link( dxcutil::AssembleInputs inputs( std::move(pM), pOutputBlob, pMalloc, SerializeFlags, pOutputStream, - opts.DebugInfo, opts.DebugFile, &Diag); + opts.DebugFile, &Diag); if (needsValidation) { valHR = dxcutil::ValidateAndAssembleToContainer(inputs); } else { diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 6f193102cb..7b82da387a 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -973,7 +973,7 @@ class DxcCompiler : public IDxcCompiler3, dxcutil::AssembleInputs inputs( std::move(serializeModule), pOutputBlob, m_pMalloc, SerializeFlags, - pOutputStream, opts.GenerateFullDebugInfo(), + pOutputStream, opts.GetPDBName(), &compiler.getDiagnostics(), &ShaderHashContent, pReflectionStream, pRootSigStream); diff --git a/tools/clang/tools/dxcompiler/dxcutil.cpp b/tools/clang/tools/dxcompiler/dxcutil.cpp index 717f1f999f..735396f112 100644 --- a/tools/clang/tools/dxcompiler/dxcutil.cpp +++ b/tools/clang/tools/dxcompiler/dxcutil.cpp @@ -20,6 +20,7 @@ #include "llvm/Bitcode/ReaderWriter.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/DiagnosticPrinter.h" +#include "llvm/IR/DebugInfo.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" #include "llvm/Support/MemoryBuffer.h" @@ -70,7 +71,6 @@ AssembleInputs::AssembleInputs(std::unique_ptr &&pM, IMalloc *pMalloc, hlsl::SerializeDxilFlags SerializeFlags, CComPtr &pModuleBitcode, - bool bDebugInfo, llvm::StringRef DebugName, clang::DiagnosticsEngine *pDiag, hlsl::DxilShaderHash *pShaderHashOut, @@ -81,7 +81,6 @@ AssembleInputs::AssembleInputs(std::unique_ptr &&pM, pMalloc(pMalloc), SerializeFlags(SerializeFlags), pModuleBitcode(pModuleBitcode), - bDebugInfo(bDebugInfo), DebugName(DebugName), pDiag(pDiag), pShaderHashOut(pShaderHashOut), @@ -164,7 +163,7 @@ HRESULT ValidateAndAssembleToContainer(AssembleInputs &inputs) { // SerializeDxilContainerForModule stripping all the debug info. The debug // info will be stripped from the orginal module, but preserved in the cloned // module. - if (inputs.bDebugInfo) { + if (llvm::getDebugMetadataVersionFromModule(*inputs.pM) != 0) { llvmModuleWithDebugInfo.reset(llvm::CloneModule(inputs.pM.get())); } } diff --git a/tools/clang/tools/dxcompiler/dxcutil.h b/tools/clang/tools/dxcompiler/dxcutil.h index 5de35c4e6a..804ad4361e 100644 --- a/tools/clang/tools/dxcompiler/dxcutil.h +++ b/tools/clang/tools/dxcompiler/dxcutil.h @@ -45,7 +45,6 @@ struct AssembleInputs { IMalloc *pMalloc, hlsl::SerializeDxilFlags SerializeFlags, CComPtr &pModuleBitcode, - bool bDebugInfo = false, llvm::StringRef DebugName = llvm::StringRef(), clang::DiagnosticsEngine *pDiag = nullptr, hlsl::DxilShaderHash *pShaderHashOut = nullptr, @@ -56,7 +55,6 @@ struct AssembleInputs { IMalloc *pMalloc; hlsl::SerializeDxilFlags SerializeFlags; CComPtr &pModuleBitcode; - bool bDebugInfo; llvm::StringRef DebugName = llvm::StringRef(); clang::DiagnosticsEngine *pDiag; hlsl::DxilShaderHash *pShaderHashOut = nullptr; From 96fa84b6727bd3274056db94087b3656f4f726d3 Mon Sep 17 00:00:00 2001 From: Adam Yang Date: Fri, 19 Mar 2021 14:36:45 -0700 Subject: [PATCH 04/11] Added column info and tests --- .../mesh-val/asOversizePayload.hlsl | 2 +- .../mesh-val/msOversizePayload.hlsl | 4 ++-- .../hlsl/diagnostics/errors/AddUint64Odd.hlsl | 2 +- .../diagnostics/errors/InnerCoverage.hlsl | 4 ++-- .../errors/invalid_input_output_types.hlsl | 6 ++--- .../diagnostics/errors/local_resource2.hlsl | 2 +- .../diagnostics/errors/local_resource3.hlsl | 2 +- .../diagnostics/errors/local_resource5.hlsl | 2 +- .../errors/local_resource5_dbg.hlsl | 2 +- .../diagnostics/errors/local_resource6.hlsl | 2 +- .../errors/local_resource6_dbg.hlsl | 2 +- .../diagnostics/errors/unsupported_error.hlsl | 4 ++-- .../warnings/Wno-attribute-statement.hlsl | 3 ++- .../warnings/Wno-comma-in-init.hlsl | 2 +- .../warnings/Wno-for-redefinition.hlsl | 2 +- .../functions/entrypoints/unused_func.hlsl | 4 ++-- .../pack/sm_6_6_pack_unpack_error.hlsl | 23 ++++++++++--------- .../HLSLFileCheck/validation/UndefStore.hlsl | 4 ++-- .../clang/tools/dxcompiler/dxcompilerobj.cpp | 1 + 19 files changed, 38 insertions(+), 35 deletions(-) diff --git a/tools/clang/test/CodeGenHLSL/mesh-val/asOversizePayload.hlsl b/tools/clang/test/CodeGenHLSL/mesh-val/asOversizePayload.hlsl index d2b82ef47b..abcf36c2fc 100644 --- a/tools/clang/test/CodeGenHLSL/mesh-val/asOversizePayload.hlsl +++ b/tools/clang/test/CodeGenHLSL/mesh-val/asOversizePayload.hlsl @@ -2,7 +2,7 @@ // RUN: %dxilver 1.6 | %dxc -E main -T as_6_5 %s | FileCheck %s -check-prefix=CHK_NODB // CHK_DB: 23:5: error: For amplification shader with entry 'main', payload size 16400 is greater than maximum size of 16384 bytes. -// CHK_NODB: For amplification shader with entry 'main', payload size 16400 is greater than maximum size of 16384 bytes. +// CHK_NODB: 23:5: error: For amplification shader with entry 'main', payload size 16400 is greater than maximum size of 16384 bytes. #define NUM_THREADS 32 diff --git a/tools/clang/test/CodeGenHLSL/mesh-val/msOversizePayload.hlsl b/tools/clang/test/CodeGenHLSL/mesh-val/msOversizePayload.hlsl index 1651a589ec..27be45bc76 100644 --- a/tools/clang/test/CodeGenHLSL/mesh-val/msOversizePayload.hlsl +++ b/tools/clang/test/CodeGenHLSL/mesh-val/msOversizePayload.hlsl @@ -2,7 +2,7 @@ // RUN: %dxilver 1.6 | %dxc -E main -T ms_6_5 %s | FileCheck %s -check-prefix=CHK_NODB // CHK_DB: :29: error: For mesh shader with entry 'main', payload size 16404 is greater than maximum size of 16384 bytes. -// CHK_NODB: error: For mesh shader with entry 'main', payload size 16404 is greater than maximum size of 16384 bytes. Use /Zi for source location. +// CHK_NODB: :29: error: For mesh shader with entry 'main', payload size 16404 is greater than maximum size of 16384 bytes. #define MAX_VERT 32 #define MAX_PRIM 16 @@ -62,4 +62,4 @@ void main( prims[tig / 3] = op; } verts[tig] = ov; -} \ No newline at end of file +} diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/AddUint64Odd.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/AddUint64Odd.hlsl index c2d3972e55..6e130462c1 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/AddUint64Odd.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/AddUint64Odd.hlsl @@ -2,7 +2,7 @@ // RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s -check-prefix=CHK_NODB // CHK_DB: 8:12: error: AddUint64 can only be applied to uint2 and uint4 operands. -// CHK_NODB: error: AddUint64 can only be applied to uint2 and uint4 operands. Use /Zi for source location. +// CHK_NODB: 8:12: error: AddUint64 can only be applied to uint2 and uint4 operands. float4 main(uint4 a : A, uint4 b :B) : SV_TARGET { uint c = AddUint64(a.x, b.x); diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/InnerCoverage.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/InnerCoverage.hlsl index d4dc93edbb..f933f957c4 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/InnerCoverage.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/InnerCoverage.hlsl @@ -5,8 +5,8 @@ // CHK_DB: 11:1: error: Parameter with semantic SV_InnerCoverage has overlapping semantic index at 0. // CHK_DB: 11:1: error: Pixel shader inputs SV_Coverage and SV_InnerCoverage are mutually exclusive. -// CHK_NODB: error: Parameter with semantic SV_InnerCoverage has overlapping semantic index at 0. Use /Zi for source location. -// CHK_NODB: error: Pixel shader inputs SV_Coverage and SV_InnerCoverage are mutually exclusive. Use /Zi for source location. +// CHK_NODB: 11:1: error: Parameter with semantic SV_InnerCoverage has overlapping semantic index at 0. +// CHK_NODB: 11:1: error: Pixel shader inputs SV_Coverage and SV_InnerCoverage are mutually exclusive. void main(snorm float b : B, uint c:C, #ifndef GENLL diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/invalid_input_output_types.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/invalid_input_output_types.hlsl index 9bbf192f43..da5286a6e4 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/invalid_input_output_types.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/invalid_input_output_types.hlsl @@ -3,9 +3,9 @@ // CHK_DB: 9:1: error: i64(type for I) cannot be used as shader inputs or outputs. // CHK_DB: 9:1: error: double(type for SV_Target) cannot be used as shader inputs or outputs. -// CHK_NODB: cannot be used as shader inputs or outputs. Use /Zi for source location. -// CHK_NODB: cannot be used as shader inputs or outputs. Use /Zi for source location. +// CHK_NODB: 9:1: error: i64(type for I) cannot be used as shader inputs or outputs. +// CHK_NODB: 9:1: error: double(type for SV_Target) cannot be used as shader inputs or outputs. double main(uint64_t i:I) : SV_Target { return 1; -} \ No newline at end of file +} diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource2.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource2.hlsl index f497e54f0e..4dcd19447c 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource2.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource2.hlsl @@ -2,7 +2,7 @@ // RUN: %dxc -E main -Od -T ps_6_0 %s | FileCheck %s -check-prefix=CHK_NODB // CHK_DB: 9:10: error: local resource not guaranteed to map to unique global resource. -// CHK_NODB: error: local resource not guaranteed to map to unique global resource. Use /Zi for source location. +// CHK_NODB: 9:10: error: local resource not guaranteed to map to unique global resource. float4 Tex2D(Texture2D t, SamplerState s, float2 c) { diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource3.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource3.hlsl index 9c4c978008..1e61dcbae5 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource3.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource3.hlsl @@ -2,7 +2,7 @@ // RUN: %dxc -E main -Od -T ps_6_0 %s | FileCheck %s -check-prefix=CHK_NODB // CHK_DB: 9:10: error: local resource not guaranteed to map to unique global resource. -// CHK_NODB: error: local resource not guaranteed to map to unique global resource. Use /Zi for source location. +// CHK_NODB: 9:10: error: local resource not guaranteed to map to unique global resource. float4 Tex2D(Texture2D t, SamplerState s, float2 c) { diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource5.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource5.hlsl index bf4bbecc76..e20526c1e3 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource5.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource5.hlsl @@ -2,7 +2,7 @@ // RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s -check-prefix=CHK_NODB // CHK_DB: 17:7: error: local resource not guaranteed to map to unique global resource. -// CHK_NODB: error: local resource not guaranteed to map to unique global resource. Use /Zi for source location. +// CHK_NODB: 17:7: error: local resource not guaranteed to map to unique global resource. float4 Tex2D(Texture2D t, SamplerState s, float2 c) { diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource5_dbg.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource5_dbg.hlsl index a1d5a57b96..b70e18f814 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource5_dbg.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource5_dbg.hlsl @@ -2,7 +2,7 @@ // RUN: %dxc -E main -Od -T ps_6_0 %s | FileCheck %s -check-prefix=CHK_NODB // CHK_DB: 9:10: error: local resource not guaranteed to map to unique global resource. -// CHK_NODB: error: local resource not guaranteed to map to unique global resource. Use /Zi for source location. +// CHK_NODB: 9:10: error: local resource not guaranteed to map to unique global resource. float4 Tex2D(Texture2D t, SamplerState s, float2 c) { diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource6.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource6.hlsl index 0ea3368886..4eeaaa9835 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource6.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource6.hlsl @@ -2,7 +2,7 @@ // RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s -check-prefix=CHK_NODB // CHK_DB: 18:7: error: local resource not guaranteed to map to unique global resource. -// CHK_NODB: error: local resource not guaranteed to map to unique global resource. Use /Zi for source location. +// CHK_NODB: 18:7: error: local resource not guaranteed to map to unique global resource. float4 Tex2D(Texture2D t, SamplerState s, float2 c) { diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource6_dbg.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource6_dbg.hlsl index 79cc400b28..bd7e513ceb 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource6_dbg.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/local_resource6_dbg.hlsl @@ -2,7 +2,7 @@ // RUN: %dxc -E main -Od -T ps_6_0 %s | FileCheck %s -check-prefix=CHK_NODB // CHK_DB: 9:10: error: local resource not guaranteed to map to unique global resource. -// CHK_NODB: error: local resource not guaranteed to map to unique global resource. Use /Zi for source location. +// CHK_NODB: 9:10: error: local resource not guaranteed to map to unique global resource. float4 Tex2D(Texture2D t, SamplerState s, float2 c) { diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/unsupported_error.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/unsupported_error.hlsl index 6f01492bc9..20647b5904 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/unsupported_error.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/unsupported_error.hlsl @@ -4,7 +4,7 @@ // Regression test for a crash when lowering unsupported intrinsics // CHK_DB: 10:50: error: Unsupported intrinsic -// CHK_NODB: error: Unsupported intrinsic. Use /Zi for source location. +// CHK_NODB: 10:50: error: Unsupported intrinsic sampler TextureSampler; -float4 main(float2 uv : UV) : SV_Target { return tex2D(TextureSampler, uv); } \ No newline at end of file +float4 main(float2 uv : UV) : SV_Target { return tex2D(TextureSampler, uv); } diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-attribute-statement.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-attribute-statement.hlsl index 5e3b559722..27aeb303ac 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-attribute-statement.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-attribute-statement.hlsl @@ -2,6 +2,8 @@ // Make sure the specified warning gets turned off +// CHECK: 9:1: error: Semantic must be defined + // This function has no output semantic on purpose in order to produce an error, // otherwise, the warnings will not be captured in the output for FileCheck. float main() { @@ -14,4 +16,3 @@ float main() { return 0; } -// CHECK: error: Semantic must be defined diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-comma-in-init.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-comma-in-init.hlsl index 79e8429fd6..1cbb9ef2bc 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-comma-in-init.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-comma-in-init.hlsl @@ -7,7 +7,7 @@ float main() { // comma expression used where a constructor list may have been intended -// CHECK-NOT: comma +// CHECK-NOT: comma expression int a = 1, b = 2; int c = (a, b); diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-for-redefinition.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-for-redefinition.hlsl index ce0c3efb79..d4c1710fe6 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-for-redefinition.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/warnings/Wno-for-redefinition.hlsl @@ -7,7 +7,7 @@ float main() { // redefinition of %0 shadows declaration in the outer scope; most recent declaration will be used -// CHECK-NOT: redefinition +// CHECK-NOT: redefinition of for (int i=0; i<4; i++); for (int i=0; i<4; i++); diff --git a/tools/clang/test/HLSLFileCheck/hlsl/functions/entrypoints/unused_func.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/functions/entrypoints/unused_func.hlsl index 2a6a60144f..e646bc6648 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/functions/entrypoints/unused_func.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/functions/entrypoints/unused_func.hlsl @@ -2,8 +2,8 @@ // Make sure unused functions are not generated. -// CHECK-NOT: unused +// CHECK-NOT: unused_function_name -void unused() {} +void unused_function_name() {} void main() {} diff --git a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pack/sm_6_6_pack_unpack_error.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pack/sm_6_6_pack_unpack_error.hlsl index eef6332471..c58647acff 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pack/sm_6_6_pack_unpack_error.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pack/sm_6_6_pack_unpack_error.hlsl @@ -1,22 +1,23 @@ // RUN: %dxilver 1.6 | %dxc -T ps_6_5 -enable-16bit-types %s | FileCheck %s -// CHECK: Opcode Pack4x8 not valid in shader model ps_6_5 -// CHECK: Opcode Pack4x8 not valid in shader model ps_6_5 -// CHECK: Opcode Unpack4x8 not valid in shader model ps_6_5 -// CHECK: Opcode Unpack4x8 not valid in shader model ps_6_5 -// CHECK: Opcode Unpack4x8 not valid in shader model ps_6_5 -// CHECK: Opcode Unpack4x8 not valid in shader model ps_6_5 -// CHECK: Opcode Pack4x8 not valid in shader model ps_6_5 -// CHECK: Opcode Pack4x8 not valid in shader model ps_6_5 +// CHECK-DAG: 13:{{[0-9]+}}: error: Opcode Pack4x8 not valid in shader model ps_6_5 +// CHECK-DAG: 14:{{[0-9]+}}: error: Opcode Pack4x8 not valid in shader model ps_6_5 +// CHECK-DAG: 15:{{[0-9]+}}: error: Opcode Unpack4x8 not valid in shader model ps_6_5 +// CHECK-DAG: 16:{{[0-9]+}}: error: Opcode Unpack4x8 not valid in shader model ps_6_5 +// CHECK-DAG: 17:{{[0-9]+}}: error: Opcode Pack4x8 not valid in shader model ps_6_5 +// CHECK-DAG: 18:{{[0-9]+}}: error: Opcode Pack4x8 not valid in shader model ps_6_5 +// CHECK-DAG: 19:{{[0-9]+}}: error: Opcode Unpack4x8 not valid in shader model ps_6_5 +// CHECK-DAG: 20:{{[0-9]+}}: error: Opcode Unpack4x8 not valid in shader model ps_6_5 int16_t4 main(int4 input1 : Inputs1, int16_t4 input2 : Inputs2) : SV_Target { int8_t4_packed ps1 = pack_s8(input1); int8_t4_packed ps2 = pack_clamp_s8(input1); - int16_t4 up1_out = unpack_s8s16(ps1) + unpack_s8s16(ps2); - + int16_t4 up1_out = unpack_s8s16(ps1) + + unpack_s8s16(ps2); uint8_t4_packed pu1 = pack_u8(input2); uint8_t4_packed pu2 = pack_clamp_u8(input2); - uint16_t4 up2_out = unpack_u8u16(pu1) + unpack_u8u16(pu2); + uint16_t4 up2_out = unpack_u8u16(pu1) + + unpack_u8u16(pu2); return up1_out + up2_out; } diff --git a/tools/clang/test/HLSLFileCheck/validation/UndefStore.hlsl b/tools/clang/test/HLSLFileCheck/validation/UndefStore.hlsl index 9250102237..1f893cbdfa 100644 --- a/tools/clang/test/HLSLFileCheck/validation/UndefStore.hlsl +++ b/tools/clang/test/HLSLFileCheck/validation/UndefStore.hlsl @@ -2,7 +2,7 @@ // RUN: %dxilver 1.6 | %dxc -E main -T cs_6_0 %s | FileCheck %s -check-prefix=CHECK -check-prefix=CHK_NODB // CHK_DB: 18:17: error: Assignment of undefined values to UAV. -// CHK_NODB: Function: main: error: Assignment of undefined values to UAV. Use /Zi for source location. +// CHK_NODB: 18:17: error: Assignment of undefined values to UAV. RWBuffer output; @@ -16,4 +16,4 @@ void main(uint3 DTid : SV_DispatchThreadID) { uint sum = Add(sum, (uint)DTid.x); // Deliberate use of uninitialised variable 'sum' output[DTid.x] = sum; -} \ No newline at end of file +} diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 7b82da387a..0efefca3e2 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -1255,6 +1255,7 @@ class DxcCompiler : public IDxcCompiler3, else { CodeGenOptions &CGOpts = compiler.getCodeGenOpts(); CGOpts.setDebugInfo(CodeGenOptions::LocTrackingOnly); + CGOpts.DebugColumnInfo = 1; } clang::PreprocessorOptions &PPOpts(compiler.getPreprocessorOpts()); From a50eac68f93acbda90b24437872dc3bc05c5a36f Mon Sep 17 00:00:00 2001 From: Adam Yang Date: Fri, 19 Mar 2021 19:22:27 -0700 Subject: [PATCH 05/11] New validator interface --- include/dxc/dxcapi.h | 11 +++++ tools/clang/tools/dxcompiler/dxcutil.cpp | 40 +++++++++++++++---- tools/clang/tools/dxcompiler/dxcvalidator.cpp | 39 +++++++++++++++++- 3 files changed, 81 insertions(+), 9 deletions(-) diff --git a/include/dxc/dxcapi.h b/include/dxc/dxcapi.h index 21aafd3ec4..e1dec973b8 100644 --- a/include/dxc/dxcapi.h +++ b/include/dxc/dxcapi.h @@ -511,6 +511,17 @@ struct IDxcValidator : public IUnknown { ) = 0; }; +CROSS_PLATFORM_UUIDOF(IDxcValidator2, "458e1fd1-b1b2-4750-a6e1-9c10f03bed92") +struct IDxcValidator2 : public IDxcValidator { + // Validate a shader. + virtual HRESULT STDMETHODCALLTYPE ValidateWithDebug( + _In_ IDxcBlob *pShader, // Shader to validate. + _In_ UINT32 Flags, // Validation flags. + _In_ DxcBuffer *pDebugModule, // Debug module to provide line numbers + _COM_Outptr_ IDxcOperationResult **ppResult // Validation output status, buffer, and errors + ) = 0; +}; + CROSS_PLATFORM_UUIDOF(IDxcContainerBuilder, "334b1f50-2292-4b35-99a1-25588d8c17fe") struct IDxcContainerBuilder : public IUnknown { virtual HRESULT STDMETHODCALLTYPE Load(_In_ IDxcBlob *pDxilContainerHeader) = 0; // Loads DxilContainer to the builder diff --git a/tools/clang/tools/dxcompiler/dxcutil.cpp b/tools/clang/tools/dxcompiler/dxcutil.cpp index 735396f112..ae913db0d8 100644 --- a/tools/clang/tools/dxcompiler/dxcutil.cpp +++ b/tools/clang/tools/dxcompiler/dxcutil.cpp @@ -150,6 +150,11 @@ HRESULT ValidateAndAssembleToContainer(AssembleInputs &inputs) { bool bInternalValidator = CreateValidator(pValidator); // Warning on internal Validator + CComPtr pValidator2; + if (!bInternalValidator) { + pValidator.QueryInterface(&pValidator2); + } + if (bInternalValidator) { if (inputs.pDiag) { unsigned diagID = @@ -158,11 +163,14 @@ HRESULT ValidateAndAssembleToContainer(AssembleInputs &inputs) { "signed for use in release environments.\r\n"); inputs.pDiag->Report(diagID); } - // 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 (bInternalValidator || pValidator2) { + // If using the internal validator or external validator supports + // IDxcValidator2, 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 (llvm::getDebugMetadataVersionFromModule(*inputs.pM) != 0) { llvmModuleWithDebugInfo.reset(llvm::CloneModule(inputs.pM.get())); } @@ -198,8 +206,26 @@ HRESULT ValidateAndAssembleToContainer(AssembleInputs &inputs) { llvmModuleWithDebugInfo.get(), inputs.pOutputContainerBlob, DxcValidatorFlags_InPlaceEdit, &pValResult)); } else { - IFT(pValidator->Validate(inputs.pOutputContainerBlob, DxcValidatorFlags_InPlaceEdit, - &pValResult)); + if (pValidator2) { + + // If metadata was stripped, re-serialize the input module. + CComPtr pDebugModuleStream; + IFT(CreateMemoryStream(DxcGetThreadMallocNoRef(), &pDebugModuleStream)); + raw_stream_ostream outStream(pDebugModuleStream.p); + WriteBitcodeToFile(llvmModuleWithDebugInfo.get(), outStream, true); + outStream.flush(); + + DxcBuffer debugModule = {}; + debugModule.Ptr = pDebugModuleStream->GetPtr(); + debugModule.Size = pDebugModuleStream->GetPtrSize(); + + IFT(pValidator2->ValidateWithDebug(inputs.pOutputContainerBlob, DxcValidatorFlags_InPlaceEdit, + &debugModule, &pValResult)); + } + else { + IFT(pValidator->Validate(inputs.pOutputContainerBlob, DxcValidatorFlags_InPlaceEdit, + &pValResult)); + } } IFT(pValResult->GetStatus(&valHR)); if (inputs.pDiag) { diff --git a/tools/clang/tools/dxcompiler/dxcvalidator.cpp b/tools/clang/tools/dxcompiler/dxcvalidator.cpp index ee980ee107..c27ad24787 100644 --- a/tools/clang/tools/dxcompiler/dxcvalidator.cpp +++ b/tools/clang/tools/dxcompiler/dxcvalidator.cpp @@ -20,6 +20,7 @@ #include "dxc/Support/Global.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/MSFileSystem.h" +#include "llvm/Support/MemoryBuffer.h" #include "dxc/Support/microcom.h" #include "dxc/Support/FileIOHelper.h" #include "dxc/Support/dxcapi.impl.h" @@ -53,7 +54,7 @@ struct DiagRestore { } }; -class DxcValidator : public IDxcValidator, +class DxcValidator : public IDxcValidator2, #ifdef SUPPORT_QUERY_GIT_COMMIT_INFO public IDxcVersionInfo2 #else @@ -79,7 +80,7 @@ class DxcValidator : public IDxcValidator, DXC_MICROCOM_TM_CTOR(DxcValidator) HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, void **ppvObject) override { - return DoBasicQueryInterface(this, iid, ppvObject); + return DoBasicQueryInterface(this, iid, ppvObject); } // For internal use only. @@ -98,6 +99,14 @@ class DxcValidator : public IDxcValidator, _COM_Outptr_ IDxcOperationResult **ppResult // Validation output status, buffer, and errors ) override; + // IDxcValidator2 + HRESULT STDMETHODCALLTYPE ValidateWithDebug( + _In_ IDxcBlob *pShader, // Shader to validate. + _In_ UINT32 Flags, // Validation flags. + _In_ DxcBuffer *pDebugModule, // Debug module to provide line numbers + _COM_Outptr_ IDxcOperationResult **ppResult // Validation output status, buffer, and errors + ) override; + // IDxcVersionInfo HRESULT STDMETHODCALLTYPE GetVersion(_Out_ UINT32 *pMajor, _Out_ UINT32 *pMinor) override; HRESULT STDMETHODCALLTYPE GetFlags(_Out_ UINT32 *pFlags) override; @@ -122,6 +131,32 @@ HRESULT STDMETHODCALLTYPE DxcValidator::Validate( return ValidateWithOptModules(pShader, Flags, nullptr, nullptr, ppResult); } +HRESULT STDMETHODCALLTYPE DxcValidator::ValidateWithDebug( + _In_ IDxcBlob *pShader, // Shader to validate. + _In_ UINT32 Flags, // Validation flags. + _In_ DxcBuffer *pDebugModule, // Debug module to provide line numbers + _COM_Outptr_ IDxcOperationResult **ppResult // Validation output status, buffer, and errors +) +{ + DxcThreadMalloc TM(m_pMalloc); + if (pShader == nullptr || ppResult == nullptr || Flags & ~DxcValidatorFlags_ValidMask) + return E_INVALIDARG; + if ((Flags & DxcValidatorFlags_ModuleOnly) && (Flags & (DxcValidatorFlags_InPlaceEdit | DxcValidatorFlags_RootSignatureOnly))) + return E_INVALIDARG; + + std::unique_ptr pM; + std::unique_ptr pMemBuf; + LLVMContext ctx; + if (pDebugModule) { + pMemBuf = llvm::MemoryBuffer::getMemBuffer(StringRef((char *)pDebugModule->Ptr, pDebugModule->Size), false); + llvm::ErrorOr > ModuleOrErr = llvm::parseBitcodeFile(pMemBuf->getMemBufferRef(), ctx); + if (ModuleOrErr) { + pM = std::move(ModuleOrErr.get()); + } + } + return ValidateWithOptModules(pShader, Flags, nullptr, pM.get(), ppResult); +} + HRESULT DxcValidator::ValidateWithOptModules( _In_ IDxcBlob *pShader, // Shader to validate. _In_ UINT32 Flags, // Validation flags. From f27aeea05d0a48985757e2eb32cdd5b718bec52a Mon Sep 17 00:00:00 2001 From: Tex Riddell Date: Fri, 19 Mar 2021 22:49:20 -0700 Subject: [PATCH 06/11] Fixes for IDxcValidator2 interface and prep for external interface --- include/dxc/HLSL/DxilValidation.h | 7 +++++++ include/dxc/dxcapi.h | 2 +- lib/HLSL/DxilValidation.cpp | 14 ++++++++++++++ tools/clang/tools/dxcompiler/dxcutil.cpp | 2 +- tools/clang/tools/dxcompiler/dxcvalidator.cpp | 4 +++- 5 files changed, 26 insertions(+), 3 deletions(-) diff --git a/include/dxc/HLSL/DxilValidation.h b/include/dxc/HLSL/DxilValidation.h index d808e6d560..5b0b4b8852 100644 --- a/include/dxc/HLSL/DxilValidation.h +++ b/include/dxc/HLSL/DxilValidation.h @@ -347,6 +347,13 @@ HRESULT ValidateDxilContainer(_In_reads_bytes_(ContainerSize) const void *pConta _In_ uint32_t ContainerSize, _In_ llvm::raw_ostream &DiagStream); +// Full container validation, including ValidateDxilModule, with debug module +HRESULT ValidateDxilContainer(_In_reads_bytes_(ContainerSize) const void *pContainer, + _In_ uint32_t ContainerSize, + const void *pOptDebugBitcode, + uint32_t OptDebugBitcodeSize, + _In_ llvm::raw_ostream &DiagStream); + class PrintDiagnosticContext { private: llvm::DiagnosticPrinter &m_Printer; diff --git a/include/dxc/dxcapi.h b/include/dxc/dxcapi.h index e1dec973b8..a55920d4f5 100644 --- a/include/dxc/dxcapi.h +++ b/include/dxc/dxcapi.h @@ -517,7 +517,7 @@ struct IDxcValidator2 : public IDxcValidator { virtual HRESULT STDMETHODCALLTYPE ValidateWithDebug( _In_ IDxcBlob *pShader, // Shader to validate. _In_ UINT32 Flags, // Validation flags. - _In_ DxcBuffer *pDebugModule, // Debug module to provide line numbers + _In_opt_ DxcBuffer *pOptDebugBitcode, // Optional debug module bitcode to provide line numbers _COM_Outptr_ IDxcOperationResult **ppResult // Validation output status, buffer, and errors ) = 0; }; diff --git a/lib/HLSL/DxilValidation.cpp b/lib/HLSL/DxilValidation.cpp index 8a1bd9b690..bee44e81b9 100644 --- a/lib/HLSL/DxilValidation.cpp +++ b/lib/HLSL/DxilValidation.cpp @@ -6246,6 +6246,8 @@ _Use_decl_annotations_ HRESULT ValidateLoadModuleFromContainerLazy( _Use_decl_annotations_ HRESULT ValidateDxilContainer(const void *pContainer, uint32_t ContainerSize, + const void *pOptDebugBitcode, + uint32_t OptDebugBitcodeSize, llvm::raw_ostream &DiagStream) { LLVMContext Ctx, DbgCtx; std::unique_ptr pModule, pDebugModule; @@ -6260,6 +6262,12 @@ HRESULT ValidateDxilContainer(const void *pContainer, IFR(ValidateLoadModuleFromContainer(pContainer, ContainerSize, pModule, pDebugModule, Ctx, DbgCtx, DiagStream)); + if (!pDebugModule && pOptDebugBitcode) { + // TODO: lazy load for perf + IFR(ValidateLoadModule((const char *)pOptDebugBitcode, OptDebugBitcodeSize, + pDebugModule, DbgCtx, DiagStream, /*bLazyLoad*/false)); + } + // Validate DXIL Module IFR(ValidateDxilModule(pModule.get(), pDebugModule.get())); @@ -6271,4 +6279,10 @@ HRESULT ValidateDxilContainer(const void *pContainer, IsDxilContainerLike(pContainer, ContainerSize), ContainerSize); } +_Use_decl_annotations_ +HRESULT ValidateDxilContainer(const void *pContainer, + uint32_t ContainerSize, + llvm::raw_ostream &DiagStream) { + return ValidateDxilContainer(pContainer, ContainerSize, nullptr, 0, DiagStream); +} } // namespace hlsl diff --git a/tools/clang/tools/dxcompiler/dxcutil.cpp b/tools/clang/tools/dxcompiler/dxcutil.cpp index ae913db0d8..3622fc8368 100644 --- a/tools/clang/tools/dxcompiler/dxcutil.cpp +++ b/tools/clang/tools/dxcompiler/dxcutil.cpp @@ -206,7 +206,7 @@ HRESULT ValidateAndAssembleToContainer(AssembleInputs &inputs) { llvmModuleWithDebugInfo.get(), inputs.pOutputContainerBlob, DxcValidatorFlags_InPlaceEdit, &pValResult)); } else { - if (pValidator2) { + if (pValidator2 && llvmModuleWithDebugInfo) { // If metadata was stripped, re-serialize the input module. CComPtr pDebugModuleStream; diff --git a/tools/clang/tools/dxcompiler/dxcvalidator.cpp b/tools/clang/tools/dxcompiler/dxcvalidator.cpp index c27ad24787..e07fdbb6d2 100644 --- a/tools/clang/tools/dxcompiler/dxcvalidator.cpp +++ b/tools/clang/tools/dxcompiler/dxcvalidator.cpp @@ -143,12 +143,14 @@ HRESULT STDMETHODCALLTYPE DxcValidator::ValidateWithDebug( return E_INVALIDARG; if ((Flags & DxcValidatorFlags_ModuleOnly) && (Flags & (DxcValidatorFlags_InPlaceEdit | DxcValidatorFlags_RootSignatureOnly))) return E_INVALIDARG; + if (pDebugModule && (pDebugModule->Ptr == nullptr || pDebugModule->Size == 0)) + return E_INVALIDARG; std::unique_ptr pM; std::unique_ptr pMemBuf; LLVMContext ctx; if (pDebugModule) { - pMemBuf = llvm::MemoryBuffer::getMemBuffer(StringRef((char *)pDebugModule->Ptr, pDebugModule->Size), false); + pMemBuf = llvm::MemoryBuffer::getMemBuffer(StringRef((char *)pDebugModule->Ptr, pDebugModule->Size), "", false); llvm::ErrorOr > ModuleOrErr = llvm::parseBitcodeFile(pMemBuf->getMemBufferRef(), ctx); if (ModuleOrErr) { pM = std::move(ModuleOrErr.get()); From ad452b15a3c3f8b2a12727a4b0d2be29938ab114 Mon Sep 17 00:00:00 2001 From: Tex Riddell Date: Sat, 20 Mar 2021 00:42:06 -0700 Subject: [PATCH 07/11] Fix Dxil validator compat and test issues --- lib/DXIL/DxilShaderFlags.cpp | 3 ++- .../hlsl/intrinsics/helper/IsHelperLane.hlsl | 4 ++-- .../pixel/attr/attributeAtVertex.hlsl | 2 +- .../pixel/attr/attributeAtVertexNoOpt.hlsl | 2 +- .../hlsl/lifetimes/lifetimes_lib_6_3.hlsl | 2 +- .../objects/CbufferLegacy/cbufferSize1.hlsl | 2 +- .../objects/CbufferLegacy/cbufferSize3.hlsl | 2 +- .../objects/CbufferLegacy/cbufferSize5.hlsl | 2 +- .../objects/CbufferLegacy/cbufferSize6.hlsl | 2 +- .../sv_barycentrics/barycentrics.hlsl | 2 +- .../sv_barycentrics/barycentrics1.hlsl | 2 +- .../hlsl/types/conversions/semantics_Mod.hlsl | 2 +- .../groupshared/groupshared_shadermodels.hlsl | 18 +++++++++--------- tools/clang/unittests/HLSL/CompilerTest.cpp | 2 ++ tools/clang/unittests/HLSL/DxilModuleTest.cpp | 3 +++ tools/clang/unittests/HLSL/LinkerTest.cpp | 1 + 16 files changed, 29 insertions(+), 22 deletions(-) diff --git a/lib/DXIL/DxilShaderFlags.cpp b/lib/DXIL/DxilShaderFlags.cpp index fe93b48928..4d10eaceb5 100644 --- a/lib/DXIL/DxilShaderFlags.cpp +++ b/lib/DXIL/DxilShaderFlags.cpp @@ -373,6 +373,7 @@ ShaderFlags ShaderFlags::CollectShaderFlags(const Function *F, M->GetValidatorVersion(valMajor, valMinor); bool hasMulticomponentUAVLoadsBackCompat = valMajor == 1 && valMinor == 0; bool hasViewportOrRTArrayIndexBackCombat = valMajor == 1 && valMinor < 4; + bool hasBarycentricsBackCompat = valMajor == 1 && valMinor < 6; Type *int16Ty = Type::getInt16Ty(F->getContext()); Type *int64Ty = Type::getInt64Ty(F->getContext()); @@ -630,7 +631,7 @@ ShaderFlags ShaderFlags::CollectShaderFlags(const Function *F, flag.SetViewID(hasViewID); flag.SetViewportAndRTArrayIndex(hasViewportOrRTArrayIndex); flag.SetShadingRate(hasShadingRate); - flag.SetBarycentrics(hasBarycentrics); + flag.SetBarycentrics(hasBarycentricsBackCompat ? false : hasBarycentrics); flag.SetSamplerFeedback(hasSamplerFeedback); flag.SetRaytracingTier1_1(hasRaytracingTier1_1); flag.SetAtomicInt64OnTypedResource(hasAtomicInt64OnTypedResource); diff --git a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/helper/IsHelperLane.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/helper/IsHelperLane.hlsl index f1dfed4db5..0a7a5b5450 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/helper/IsHelperLane.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/helper/IsHelperLane.hlsl @@ -32,7 +32,7 @@ // RUN: %dxc -E cs -T cs_6_0 -Od %s | FileCheck %s -check-prefixes=CHECKCONST // RUN: %dxc -E as -T as_6_5 -Od %s | FileCheck %s -check-prefixes=CHECKCONST // RUN: %dxc -E ms -T ms_6_5 -Od %s | FileCheck %s -check-prefixes=CHECKCONST -// RUN: %dxc -T lib_6_5 %s | FileCheck %s -check-prefixes=CHECKLIBGV +// RUN: %dxilver 1.6 | %dxc -T lib_6_5 %s | FileCheck %s -check-prefixes=CHECKLIBGV // Exactly one call @@ -277,7 +277,7 @@ RWStructuredBuffer SB; void cs(uint gidx : SV_GroupIndex) { float4 result = a + IsHelperLane(); - SB[gidx] = QuadReadAcrossX(result); + SB[gidx] = result; } /// Amplification Shader diff --git a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertex.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertex.hlsl index 33c56eb0b5..f823821226 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertex.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertex.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.1 | %dxc -E main -T ps_6_1 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_1 %s | FileCheck %s // CHECK: Note: shader requires additional functionality: diff --git a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertexNoOpt.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertexNoOpt.hlsl index 12417edfa6..0b0ad597fd 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertexNoOpt.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertexNoOpt.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.1 | %dxc -E main -T ps_6_1 -O0 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_1 -O0 %s | FileCheck %s // CHECK: Note: shader requires additional functionality: // CHECK-NEXT: Barycentrics diff --git a/tools/clang/test/HLSLFileCheck/hlsl/lifetimes/lifetimes_lib_6_3.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/lifetimes/lifetimes_lib_6_3.hlsl index 6df1f31e60..1d20dd02ee 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/lifetimes/lifetimes_lib_6_3.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/lifetimes/lifetimes_lib_6_3.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxc -T lib_6_3 -enable-lifetime-markers %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -T lib_6_3 -enable-lifetime-markers %s | FileCheck %s // This file is identical to lifetimes.hlsl except that it tests for // undef stores instead of lifetime intrinsics (fallback for earlier diff --git a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize1.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize1.hlsl index 7aab2a4c87..fedfa76664 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize1.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize1.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_0 %s | FileCheck %s // CHECK: error: CBuffer size is 65552 bytes, exceeding maximum of 65536 bytes. diff --git a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize3.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize3.hlsl index fc72fb95dc..c7be58441d 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize3.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize3.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_0 %s | FileCheck %s // CHECK: error: CBuffer size is 65540 bytes, exceeding maximum of 65536 bytes. diff --git a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize5.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize5.hlsl index 8f634d6abc..4718c240fe 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize5.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize5.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_0 %s | FileCheck %s // CHECK: error: CBuffer size is 65540 bytes, exceeding maximum of 65536 bytes. diff --git a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize6.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize6.hlsl index 9645b3a7d1..d1a78c83af 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize6.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize6.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_0 %s | FileCheck %s // CHECK: error: CBuffer size is 65540 bytes, exceeding maximum of 65536 bytes. diff --git a/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics.hlsl index 047de79297..bd743c833b 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.1 | %dxc -E main -T ps_6_1 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_1 %s | FileCheck %s // CHECK: Note: shader requires additional functionality: // CHECK-NEXT: Barycentrics diff --git a/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics1.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics1.hlsl index dab891eb3c..96344dee44 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics1.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics1.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.1 | %dxc -E main -T ps_6_1 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_1 %s | FileCheck %s // CHECK: Note: shader requires additional functionality: // CHECK-NEXT: Barycentrics diff --git a/tools/clang/test/HLSLFileCheck/hlsl/types/conversions/semantics_Mod.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/types/conversions/semantics_Mod.hlsl index f08e579e9c..56759ced06 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/types/conversions/semantics_Mod.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/types/conversions/semantics_Mod.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_0 %s | FileCheck %s // CHECK: @main diff --git a/tools/clang/test/HLSLFileCheck/hlsl/types/modifiers/groupshared/groupshared_shadermodels.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/types/modifiers/groupshared/groupshared_shadermodels.hlsl index 544279141f..0a67b5d2ae 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/types/modifiers/groupshared/groupshared_shadermodels.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/types/modifiers/groupshared/groupshared_shadermodels.hlsl @@ -1,12 +1,12 @@ -// RUN: %dxc -E PSMain -T ps_6_0 %s | FileCheck %s -// RUN: %dxc -E VSMain -T vs_6_0 %s | FileCheck %s -// RUN: %dxc -E GSMain -T gs_6_0 %s | FileCheck %s -// RUN: %dxc -E HSMain -T hs_6_0 %s | FileCheck %s -// RUN: %dxc -E DSMain -T ds_6_0 %s | FileCheck %s -// RUN: %dxc -E CSMain -T lib_6_5 %s | FileCheck %s -check-prefix=LIBCHK -// RUN: %dxc -E CSMain -T cs_6_0 %s | FileCheck %s -check-prefix=CSCHK -// RUN: %dxc -E MSMain -T ms_6_5 %s | FileCheck %s -check-prefix=CSCHK -// RUN: %dxc -E ASMain -T as_6_5 %s | FileCheck %s -check-prefix=CSCHK +// RUN: %dxilver 1.6 | %dxc -E PSMain -T ps_6_0 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E VSMain -T vs_6_0 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E GSMain -T gs_6_0 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E HSMain -T hs_6_0 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E DSMain -T ds_6_0 %s | FileCheck %s +// RUN: %dxilver 1.6 | %dxc -E CSMain -T lib_6_5 %s | FileCheck %s -check-prefix=LIBCHK +// RUN: %dxilver 1.6 | %dxc -E CSMain -T cs_6_0 %s | FileCheck %s -check-prefix=CSCHK +// RUN: %dxilver 1.6 | %dxc -E MSMain -T ms_6_5 %s | FileCheck %s -check-prefix=CSCHK +// RUN: %dxilver 1.6 | %dxc -E ASMain -T as_6_5 %s | FileCheck %s -check-prefix=CSCHK // Test that the proper error for groupshared is produced when compiling in non-compute contexts // and that everything is fine when we are diff --git a/tools/clang/unittests/HLSL/CompilerTest.cpp b/tools/clang/unittests/HLSL/CompilerTest.cpp index 3eb0e0a9d8..d010da44c5 100644 --- a/tools/clang/unittests/HLSL/CompilerTest.cpp +++ b/tools/clang/unittests/HLSL/CompilerTest.cpp @@ -1419,6 +1419,7 @@ static void VerifyPdbUtil(dxc::DxcDllSupport &dllSupport, #ifdef _WIN32 TEST_F(CompilerTest, CompileThenTestPdbUtilsStripped) { + if (m_ver.SkipDxilVersion(1, 5)) return; CComPtr pInclude; CComPtr pCompiler; CComPtr pSource; @@ -1610,6 +1611,7 @@ void CompilerTest::TestPdbUtils(bool bSlim, bool bSourceInDebugModule, bool bStr } TEST_F(CompilerTest, CompileThenTestPdbUtils) { + if (m_ver.SkipDxilVersion(1, 5)) return; TestPdbUtils(/*bSlim*/true, /*bSourceInDebugModule*/false, /*strip*/true); // Slim PDB, where source info is stored in its own part, and debug module is NOT present TestPdbUtils(/*bSlim*/false, /*bSourceInDebugModule*/true, /*strip*/false); // Old PDB format, where source info is embedded in the module diff --git a/tools/clang/unittests/HLSL/DxilModuleTest.cpp b/tools/clang/unittests/HLSL/DxilModuleTest.cpp index f7654810b1..0356604977 100644 --- a/tools/clang/unittests/HLSL/DxilModuleTest.cpp +++ b/tools/clang/unittests/HLSL/DxilModuleTest.cpp @@ -47,6 +47,7 @@ class DxilModuleTest : public ::testing::Test { TEST_CLASS_SETUP(InitSupport); dxc::DxcDllSupport m_dllSupport; + VersionSupportInfo m_ver; // Basic loading tests. TEST_METHOD(LoadDxilModule_1_0) @@ -81,6 +82,7 @@ class DxilModuleTest : public ::testing::Test { bool DxilModuleTest::InitSupport() { if (!m_dllSupport.IsEnabled()) { VERIFY_SUCCEEDED(m_dllSupport.Initialize()); + m_ver.Initialize(m_dllSupport); } return true; } @@ -582,6 +584,7 @@ TEST_F(DxilModuleTest, SetValidatorVersion) { } TEST_F(DxilModuleTest, PayloadQualifier) { + if (m_ver.SkipDxilVersion(1, 6)) return; std::vector arguments = { L"-enable-payload-qualifiers" }; Compiler c(m_dllSupport); diff --git a/tools/clang/unittests/HLSL/LinkerTest.cpp b/tools/clang/unittests/HLSL/LinkerTest.cpp index 701dc59a1d..c54b00fae3 100644 --- a/tools/clang/unittests/HLSL/LinkerTest.cpp +++ b/tools/clang/unittests/HLSL/LinkerTest.cpp @@ -816,6 +816,7 @@ TEST_F(LinkerTest, RunLinkToLibWithGlobalCtor) { } TEST_F(LinkerTest, LinkSm63ToSm66) { + if (m_ver.SkipDxilVersion(1, 6)) return; CComPtr pLib0; CompileLib(L"..\\CodeGenHLSL\\linker\\link_to_sm66.hlsl", &pLib0, {}, L"lib_6_3"); From 11d44d4884910176964dab84f80ad77c6b5a2f4e Mon Sep 17 00:00:00 2001 From: Tex Riddell Date: Sat, 20 Mar 2021 01:02:46 -0700 Subject: [PATCH 08/11] Revert "Fix Dxil validator compat and test issues" This reverts commit ad452b15a3c3f8b2a12727a4b0d2be29938ab114. --- lib/DXIL/DxilShaderFlags.cpp | 3 +-- .../hlsl/intrinsics/helper/IsHelperLane.hlsl | 4 ++-- .../pixel/attr/attributeAtVertex.hlsl | 2 +- .../pixel/attr/attributeAtVertexNoOpt.hlsl | 2 +- .../hlsl/lifetimes/lifetimes_lib_6_3.hlsl | 2 +- .../objects/CbufferLegacy/cbufferSize1.hlsl | 2 +- .../objects/CbufferLegacy/cbufferSize3.hlsl | 2 +- .../objects/CbufferLegacy/cbufferSize5.hlsl | 2 +- .../objects/CbufferLegacy/cbufferSize6.hlsl | 2 +- .../sv_barycentrics/barycentrics.hlsl | 2 +- .../sv_barycentrics/barycentrics1.hlsl | 2 +- .../hlsl/types/conversions/semantics_Mod.hlsl | 2 +- .../groupshared/groupshared_shadermodels.hlsl | 18 +++++++++--------- tools/clang/unittests/HLSL/CompilerTest.cpp | 2 -- tools/clang/unittests/HLSL/DxilModuleTest.cpp | 3 --- tools/clang/unittests/HLSL/LinkerTest.cpp | 1 - 16 files changed, 22 insertions(+), 29 deletions(-) diff --git a/lib/DXIL/DxilShaderFlags.cpp b/lib/DXIL/DxilShaderFlags.cpp index 4d10eaceb5..fe93b48928 100644 --- a/lib/DXIL/DxilShaderFlags.cpp +++ b/lib/DXIL/DxilShaderFlags.cpp @@ -373,7 +373,6 @@ ShaderFlags ShaderFlags::CollectShaderFlags(const Function *F, M->GetValidatorVersion(valMajor, valMinor); bool hasMulticomponentUAVLoadsBackCompat = valMajor == 1 && valMinor == 0; bool hasViewportOrRTArrayIndexBackCombat = valMajor == 1 && valMinor < 4; - bool hasBarycentricsBackCompat = valMajor == 1 && valMinor < 6; Type *int16Ty = Type::getInt16Ty(F->getContext()); Type *int64Ty = Type::getInt64Ty(F->getContext()); @@ -631,7 +630,7 @@ ShaderFlags ShaderFlags::CollectShaderFlags(const Function *F, flag.SetViewID(hasViewID); flag.SetViewportAndRTArrayIndex(hasViewportOrRTArrayIndex); flag.SetShadingRate(hasShadingRate); - flag.SetBarycentrics(hasBarycentricsBackCompat ? false : hasBarycentrics); + flag.SetBarycentrics(hasBarycentrics); flag.SetSamplerFeedback(hasSamplerFeedback); flag.SetRaytracingTier1_1(hasRaytracingTier1_1); flag.SetAtomicInt64OnTypedResource(hasAtomicInt64OnTypedResource); diff --git a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/helper/IsHelperLane.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/helper/IsHelperLane.hlsl index 0a7a5b5450..f1dfed4db5 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/helper/IsHelperLane.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/helper/IsHelperLane.hlsl @@ -32,7 +32,7 @@ // RUN: %dxc -E cs -T cs_6_0 -Od %s | FileCheck %s -check-prefixes=CHECKCONST // RUN: %dxc -E as -T as_6_5 -Od %s | FileCheck %s -check-prefixes=CHECKCONST // RUN: %dxc -E ms -T ms_6_5 -Od %s | FileCheck %s -check-prefixes=CHECKCONST -// RUN: %dxilver 1.6 | %dxc -T lib_6_5 %s | FileCheck %s -check-prefixes=CHECKLIBGV +// RUN: %dxc -T lib_6_5 %s | FileCheck %s -check-prefixes=CHECKLIBGV // Exactly one call @@ -277,7 +277,7 @@ RWStructuredBuffer SB; void cs(uint gidx : SV_GroupIndex) { float4 result = a + IsHelperLane(); - SB[gidx] = result; + SB[gidx] = QuadReadAcrossX(result); } /// Amplification Shader diff --git a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertex.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertex.hlsl index f823821226..33c56eb0b5 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertex.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertex.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_1 %s | FileCheck %s +// RUN: %dxilver 1.1 | %dxc -E main -T ps_6_1 %s | FileCheck %s // CHECK: Note: shader requires additional functionality: diff --git a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertexNoOpt.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertexNoOpt.hlsl index 0b0ad597fd..12417edfa6 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertexNoOpt.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/intrinsics/pixel/attr/attributeAtVertexNoOpt.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_1 -O0 %s | FileCheck %s +// RUN: %dxilver 1.1 | %dxc -E main -T ps_6_1 -O0 %s | FileCheck %s // CHECK: Note: shader requires additional functionality: // CHECK-NEXT: Barycentrics diff --git a/tools/clang/test/HLSLFileCheck/hlsl/lifetimes/lifetimes_lib_6_3.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/lifetimes/lifetimes_lib_6_3.hlsl index 1d20dd02ee..6df1f31e60 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/lifetimes/lifetimes_lib_6_3.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/lifetimes/lifetimes_lib_6_3.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.6 | %dxc -T lib_6_3 -enable-lifetime-markers %s | FileCheck %s +// RUN: %dxc -T lib_6_3 -enable-lifetime-markers %s | FileCheck %s // This file is identical to lifetimes.hlsl except that it tests for // undef stores instead of lifetime intrinsics (fallback for earlier diff --git a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize1.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize1.hlsl index fedfa76664..7aab2a4c87 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize1.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize1.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_0 %s | FileCheck %s +// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s // CHECK: error: CBuffer size is 65552 bytes, exceeding maximum of 65536 bytes. diff --git a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize3.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize3.hlsl index c7be58441d..fc72fb95dc 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize3.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize3.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_0 %s | FileCheck %s +// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s // CHECK: error: CBuffer size is 65540 bytes, exceeding maximum of 65536 bytes. diff --git a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize5.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize5.hlsl index 4718c240fe..8f634d6abc 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize5.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize5.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_0 %s | FileCheck %s +// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s // CHECK: error: CBuffer size is 65540 bytes, exceeding maximum of 65536 bytes. diff --git a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize6.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize6.hlsl index d1a78c83af..9645b3a7d1 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize6.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbufferSize6.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_0 %s | FileCheck %s +// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s // CHECK: error: CBuffer size is 65540 bytes, exceeding maximum of 65536 bytes. diff --git a/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics.hlsl index bd743c833b..047de79297 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_1 %s | FileCheck %s +// RUN: %dxilver 1.1 | %dxc -E main -T ps_6_1 %s | FileCheck %s // CHECK: Note: shader requires additional functionality: // CHECK-NEXT: Barycentrics diff --git a/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics1.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics1.hlsl index 96344dee44..dab891eb3c 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics1.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/semantics/sv_barycentrics/barycentrics1.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_1 %s | FileCheck %s +// RUN: %dxilver 1.1 | %dxc -E main -T ps_6_1 %s | FileCheck %s // CHECK: Note: shader requires additional functionality: // CHECK-NEXT: Barycentrics diff --git a/tools/clang/test/HLSLFileCheck/hlsl/types/conversions/semantics_Mod.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/types/conversions/semantics_Mod.hlsl index 56759ced06..f08e579e9c 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/types/conversions/semantics_Mod.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/types/conversions/semantics_Mod.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxilver 1.6 | %dxc -E main -T ps_6_0 %s | FileCheck %s +// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s // CHECK: @main diff --git a/tools/clang/test/HLSLFileCheck/hlsl/types/modifiers/groupshared/groupshared_shadermodels.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/types/modifiers/groupshared/groupshared_shadermodels.hlsl index 0a67b5d2ae..544279141f 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/types/modifiers/groupshared/groupshared_shadermodels.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/types/modifiers/groupshared/groupshared_shadermodels.hlsl @@ -1,12 +1,12 @@ -// RUN: %dxilver 1.6 | %dxc -E PSMain -T ps_6_0 %s | FileCheck %s -// RUN: %dxilver 1.6 | %dxc -E VSMain -T vs_6_0 %s | FileCheck %s -// RUN: %dxilver 1.6 | %dxc -E GSMain -T gs_6_0 %s | FileCheck %s -// RUN: %dxilver 1.6 | %dxc -E HSMain -T hs_6_0 %s | FileCheck %s -// RUN: %dxilver 1.6 | %dxc -E DSMain -T ds_6_0 %s | FileCheck %s -// RUN: %dxilver 1.6 | %dxc -E CSMain -T lib_6_5 %s | FileCheck %s -check-prefix=LIBCHK -// RUN: %dxilver 1.6 | %dxc -E CSMain -T cs_6_0 %s | FileCheck %s -check-prefix=CSCHK -// RUN: %dxilver 1.6 | %dxc -E MSMain -T ms_6_5 %s | FileCheck %s -check-prefix=CSCHK -// RUN: %dxilver 1.6 | %dxc -E ASMain -T as_6_5 %s | FileCheck %s -check-prefix=CSCHK +// RUN: %dxc -E PSMain -T ps_6_0 %s | FileCheck %s +// RUN: %dxc -E VSMain -T vs_6_0 %s | FileCheck %s +// RUN: %dxc -E GSMain -T gs_6_0 %s | FileCheck %s +// RUN: %dxc -E HSMain -T hs_6_0 %s | FileCheck %s +// RUN: %dxc -E DSMain -T ds_6_0 %s | FileCheck %s +// RUN: %dxc -E CSMain -T lib_6_5 %s | FileCheck %s -check-prefix=LIBCHK +// RUN: %dxc -E CSMain -T cs_6_0 %s | FileCheck %s -check-prefix=CSCHK +// RUN: %dxc -E MSMain -T ms_6_5 %s | FileCheck %s -check-prefix=CSCHK +// RUN: %dxc -E ASMain -T as_6_5 %s | FileCheck %s -check-prefix=CSCHK // Test that the proper error for groupshared is produced when compiling in non-compute contexts // and that everything is fine when we are diff --git a/tools/clang/unittests/HLSL/CompilerTest.cpp b/tools/clang/unittests/HLSL/CompilerTest.cpp index d010da44c5..3eb0e0a9d8 100644 --- a/tools/clang/unittests/HLSL/CompilerTest.cpp +++ b/tools/clang/unittests/HLSL/CompilerTest.cpp @@ -1419,7 +1419,6 @@ static void VerifyPdbUtil(dxc::DxcDllSupport &dllSupport, #ifdef _WIN32 TEST_F(CompilerTest, CompileThenTestPdbUtilsStripped) { - if (m_ver.SkipDxilVersion(1, 5)) return; CComPtr pInclude; CComPtr pCompiler; CComPtr pSource; @@ -1611,7 +1610,6 @@ void CompilerTest::TestPdbUtils(bool bSlim, bool bSourceInDebugModule, bool bStr } TEST_F(CompilerTest, CompileThenTestPdbUtils) { - if (m_ver.SkipDxilVersion(1, 5)) return; TestPdbUtils(/*bSlim*/true, /*bSourceInDebugModule*/false, /*strip*/true); // Slim PDB, where source info is stored in its own part, and debug module is NOT present TestPdbUtils(/*bSlim*/false, /*bSourceInDebugModule*/true, /*strip*/false); // Old PDB format, where source info is embedded in the module diff --git a/tools/clang/unittests/HLSL/DxilModuleTest.cpp b/tools/clang/unittests/HLSL/DxilModuleTest.cpp index 0356604977..f7654810b1 100644 --- a/tools/clang/unittests/HLSL/DxilModuleTest.cpp +++ b/tools/clang/unittests/HLSL/DxilModuleTest.cpp @@ -47,7 +47,6 @@ class DxilModuleTest : public ::testing::Test { TEST_CLASS_SETUP(InitSupport); dxc::DxcDllSupport m_dllSupport; - VersionSupportInfo m_ver; // Basic loading tests. TEST_METHOD(LoadDxilModule_1_0) @@ -82,7 +81,6 @@ class DxilModuleTest : public ::testing::Test { bool DxilModuleTest::InitSupport() { if (!m_dllSupport.IsEnabled()) { VERIFY_SUCCEEDED(m_dllSupport.Initialize()); - m_ver.Initialize(m_dllSupport); } return true; } @@ -584,7 +582,6 @@ TEST_F(DxilModuleTest, SetValidatorVersion) { } TEST_F(DxilModuleTest, PayloadQualifier) { - if (m_ver.SkipDxilVersion(1, 6)) return; std::vector arguments = { L"-enable-payload-qualifiers" }; Compiler c(m_dllSupport); diff --git a/tools/clang/unittests/HLSL/LinkerTest.cpp b/tools/clang/unittests/HLSL/LinkerTest.cpp index c54b00fae3..701dc59a1d 100644 --- a/tools/clang/unittests/HLSL/LinkerTest.cpp +++ b/tools/clang/unittests/HLSL/LinkerTest.cpp @@ -816,7 +816,6 @@ TEST_F(LinkerTest, RunLinkToLibWithGlobalCtor) { } TEST_F(LinkerTest, LinkSm63ToSm66) { - if (m_ver.SkipDxilVersion(1, 6)) return; CComPtr pLib0; CompileLib(L"..\\CodeGenHLSL\\linker\\link_to_sm66.hlsl", &pLib0, {}, L"lib_6_3"); From a4843b9909d0170d3b2a7965c451fc172d03729d Mon Sep 17 00:00:00 2001 From: Tex Riddell Date: Sat, 20 Mar 2021 01:29:41 -0700 Subject: [PATCH 09/11] Remove message hooks for 'Use /Zi for source location' --- lib/IR/DiagnosticInfo.cpp | 7 ++----- tools/clang/lib/CodeGen/CodeGenAction.cpp | 4 +--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/IR/DiagnosticInfo.cpp b/lib/IR/DiagnosticInfo.cpp index eadaa94671..76387e1e34 100644 --- a/lib/IR/DiagnosticInfo.cpp +++ b/lib/IR/DiagnosticInfo.cpp @@ -246,15 +246,12 @@ void DiagnosticInfoDxil::print(DiagnosticPrinter &DP) const { DP << "Function: " << Func->getName() << ": "; } - bool ZiPrompt = true; switch (getSeverity()) { - case DiagnosticSeverity::DS_Note: DP << "note: "; ZiPrompt = false; break; - case DiagnosticSeverity::DS_Remark: DP << "remark: "; ZiPrompt = false; break; + case DiagnosticSeverity::DS_Note: DP << "note: "; break; + case DiagnosticSeverity::DS_Remark: DP << "remark: "; break; case DiagnosticSeverity::DS_Warning: DP << "warning: "; break; case DiagnosticSeverity::DS_Error: DP << "error: "; break; } DP << getMsgStr(); - if (!DLoc && ZiPrompt) - DP << " Use /Zi for source location."; } // HLSL Change end - Dxil Diagnostic Info reporter diff --git a/tools/clang/lib/CodeGen/CodeGenAction.cpp b/tools/clang/lib/CodeGen/CodeGenAction.cpp index 76b687b704..f0fd565727 100644 --- a/tools/clang/lib/CodeGen/CodeGenAction.cpp +++ b/tools/clang/lib/CodeGen/CodeGenAction.cpp @@ -556,10 +556,8 @@ BackendConsumer::DxilDiagHandler(const llvm::DiagnosticInfoDxil &D) { } FullSourceLoc Loc(DILoc, SourceMgr); - // If no location information is available, prompt for debug flag - // and add function name to give some information + // If no location information is available, add function name if (Loc.isInvalid()) { - Message += " Use /Zi for source location."; auto *DiagClient = dynamic_cast(Diags.getClient()); auto *func = D.getFunction(); if (DiagClient && func) From 172324f21ed59f3c2be9ac8e997485f63131e6f6 Mon Sep 17 00:00:00 2001 From: Tex Riddell Date: Sat, 20 Mar 2021 01:31:14 -0700 Subject: [PATCH 10/11] Remove checks for 'Use /Zi ...' message --- .../HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt3.hlsl | 1 - .../HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt4.hlsl | 2 -- 2 files changed, 3 deletions(-) diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt3.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt3.hlsl index e5aed61690..8f8a1661b2 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt3.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt3.hlsl @@ -3,7 +3,6 @@ // CHK_DB: 17:7: error: Offsets to texture access operations must be immediate values // CHK_NODB: Offsets to texture access operations must be immediate values. -// CHK_NODB-SAME Use /Zi for source location. SamplerState samp1 : register(s5); Texture2D text1 : register(t3); diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt4.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt4.hlsl index 2e3ab658c7..27340245d0 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt4.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt4.hlsl @@ -5,9 +5,7 @@ // CHK_DB: 21:10: error: Offsets to texture access operations must be immediate values. Unrolling the loop containing the offset value manually and using -O3 may help in some cases. // CHK_NODB: error: Offsets to texture access operations must be immediate values. Unrolling the loop containing the offset value manually and using -O3 may help in some cases. -// CHK_NODB-SAME Use /Zi for source location. // CHK_NODB: error: Offsets to texture access operations must be immediate values. Unrolling the loop containing the offset value manually and using -O3 may help in some cases. -// CHK_NODB-SAME Use /Zi for source location. SamplerState samp1 : register(s5); Texture2D text1 : register(t3); From 12d5a5efcebdb047169ddc698393d18c52855dba Mon Sep 17 00:00:00 2001 From: Tex Riddell Date: Sat, 20 Mar 2021 03:28:39 -0700 Subject: [PATCH 11/11] Additional fixes for ValidateWithDebug path and tests --- .../hlsl/diagnostics/errors/optForNoOpt3.hlsl | 2 +- .../hlsl/diagnostics/errors/optForNoOpt4.hlsl | 4 +- tools/clang/tools/dxcompiler/dxcvalidator.cpp | 62 ++++++++++++------- 3 files changed, 42 insertions(+), 26 deletions(-) diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt3.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt3.hlsl index 8f8a1661b2..b87a6fea0e 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt3.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt3.hlsl @@ -1,7 +1,7 @@ // RUN: %dxc -Zi -E main -Od -T ps_6_0 %s | FileCheck %s -check-prefix=CHK_DB // RUN: %dxc -E main -Od -T ps_6_0 %s | FileCheck %s -check-prefix=CHK_NODB -// CHK_DB: 17:7: error: Offsets to texture access operations must be immediate values +// CHK_DB: 16:7: error: Offsets to texture access operations must be immediate values // CHK_NODB: Offsets to texture access operations must be immediate values. SamplerState samp1 : register(s5); diff --git a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt4.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt4.hlsl index 27340245d0..27c4ba9197 100644 --- a/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt4.hlsl +++ b/tools/clang/test/HLSLFileCheck/hlsl/diagnostics/errors/optForNoOpt4.hlsl @@ -1,8 +1,8 @@ // RUN: %dxc -Zi -E main -Od -T ps_6_0 %s | FileCheck %s -check-prefix=CHK_DB // RUN: %dxc -E main -Od -T ps_6_0 %s | FileCheck %s -check-prefix=CHK_NODB -// CHK_DB: 21:10: error: Offsets to texture access operations must be immediate values. Unrolling the loop containing the offset value manually and using -O3 may help in some cases. -// CHK_DB: 21:10: error: Offsets to texture access operations must be immediate values. Unrolling the loop containing the offset value manually and using -O3 may help in some cases. +// CHK_DB: 19:10: error: Offsets to texture access operations must be immediate values. Unrolling the loop containing the offset value manually and using -O3 may help in some cases. +// CHK_DB: 19:10: error: Offsets to texture access operations must be immediate values. Unrolling the loop containing the offset value manually and using -O3 may help in some cases. // CHK_NODB: error: Offsets to texture access operations must be immediate values. Unrolling the loop containing the offset value manually and using -O3 may help in some cases. // CHK_NODB: error: Offsets to texture access operations must be immediate values. Unrolling the loop containing the offset value manually and using -O3 may help in some cases. diff --git a/tools/clang/tools/dxcompiler/dxcvalidator.cpp b/tools/clang/tools/dxcompiler/dxcvalidator.cpp index e07fdbb6d2..76509df2fd 100644 --- a/tools/clang/tools/dxcompiler/dxcvalidator.cpp +++ b/tools/clang/tools/dxcompiler/dxcvalidator.cpp @@ -67,8 +67,8 @@ class DxcValidator : public IDxcValidator2, HRESULT RunValidation( _In_ IDxcBlob *pShader, // Shader to validate. _In_ UINT32 Flags, // Validation flags. - _In_ llvm::Module *pModule, // Module to validate, if available. - _In_ llvm::Module *pDebugModule, // Debug module to validate, if available + _In_opt_ llvm::Module *pModule, // Module to validate, if available. + _In_opt_ llvm::Module *pDebugModule, // Debug module to validate, if available _In_ AbstractMemoryStream *pDiagStream); HRESULT RunRootSignatureValidation( @@ -87,8 +87,8 @@ class DxcValidator : public IDxcValidator2, HRESULT ValidateWithOptModules( _In_ IDxcBlob *pShader, // Shader to validate. _In_ UINT32 Flags, // Validation flags. - _In_ llvm::Module *pModule, // Module to validate, if available. - _In_ llvm::Module *pDebugModule, // Debug module to validate, if available + _In_opt_ llvm::Module *pModule, // Module to validate, if available. + _In_opt_ llvm::Module *pDebugModule, // Debug module to validate, if available _COM_Outptr_ IDxcOperationResult **ppResult // Validation output status, buffer, and errors ); @@ -103,7 +103,7 @@ class DxcValidator : public IDxcValidator2, HRESULT STDMETHODCALLTYPE ValidateWithDebug( _In_ IDxcBlob *pShader, // Shader to validate. _In_ UINT32 Flags, // Validation flags. - _In_ DxcBuffer *pDebugModule, // Debug module to provide line numbers + _In_opt_ DxcBuffer *pOptDebugBitcode, // Optional debug module bitcode to provide line numbers _COM_Outptr_ IDxcOperationResult **ppResult // Validation output status, buffer, and errors ) override; @@ -124,7 +124,10 @@ HRESULT STDMETHODCALLTYPE DxcValidator::Validate( _COM_Outptr_ IDxcOperationResult **ppResult // Validation output status, buffer, and errors ) { DxcThreadMalloc TM(m_pMalloc); - if (pShader == nullptr || ppResult == nullptr || Flags & ~DxcValidatorFlags_ValidMask) + if (ppResult == nullptr) + return E_INVALIDARG; + *ppResult = nullptr; + if (pShader == nullptr || Flags & ~DxcValidatorFlags_ValidMask) return E_INVALIDARG; if ((Flags & DxcValidatorFlags_ModuleOnly) && (Flags & (DxcValidatorFlags_InPlaceEdit | DxcValidatorFlags_RootSignatureOnly))) return E_INVALIDARG; @@ -134,36 +137,49 @@ HRESULT STDMETHODCALLTYPE DxcValidator::Validate( HRESULT STDMETHODCALLTYPE DxcValidator::ValidateWithDebug( _In_ IDxcBlob *pShader, // Shader to validate. _In_ UINT32 Flags, // Validation flags. - _In_ DxcBuffer *pDebugModule, // Debug module to provide line numbers + _In_opt_ DxcBuffer *pOptDebugBitcode, // Optional debug module bitcode to provide line numbers _COM_Outptr_ IDxcOperationResult **ppResult // Validation output status, buffer, and errors ) { - DxcThreadMalloc TM(m_pMalloc); - if (pShader == nullptr || ppResult == nullptr || Flags & ~DxcValidatorFlags_ValidMask) + if (ppResult == nullptr) + return E_INVALIDARG; + *ppResult = nullptr; + if (pShader == nullptr || Flags & ~DxcValidatorFlags_ValidMask) return E_INVALIDARG; if ((Flags & DxcValidatorFlags_ModuleOnly) && (Flags & (DxcValidatorFlags_InPlaceEdit | DxcValidatorFlags_RootSignatureOnly))) return E_INVALIDARG; - if (pDebugModule && (pDebugModule->Ptr == nullptr || pDebugModule->Size == 0)) + if (pOptDebugBitcode && (pOptDebugBitcode->Ptr == nullptr || pOptDebugBitcode->Size == 0 || + pOptDebugBitcode->Size >= UINT32_MAX)) return E_INVALIDARG; - std::unique_ptr pM; - std::unique_ptr pMemBuf; - LLVMContext ctx; - if (pDebugModule) { - pMemBuf = llvm::MemoryBuffer::getMemBuffer(StringRef((char *)pDebugModule->Ptr, pDebugModule->Size), "", false); - llvm::ErrorOr > ModuleOrErr = llvm::parseBitcodeFile(pMemBuf->getMemBufferRef(), ctx); - if (ModuleOrErr) { - pM = std::move(ModuleOrErr.get()); + HRESULT hr = S_OK; + DxcThreadMalloc TM(m_pMalloc); + try { + LLVMContext Ctx; + CComPtr pDiagStream; + IFT(CreateMemoryStream(m_pMalloc, &pDiagStream)); + raw_stream_ostream DiagStream(pDiagStream); + llvm::DiagnosticPrinterRawOStream DiagPrinter(DiagStream); + PrintDiagnosticContext DiagContext(DiagPrinter); + Ctx.setDiagnosticHandler(PrintDiagnosticContext::PrintDiagnosticHandler, + &DiagContext, true); + std::unique_ptr pDebugModule; + if (pOptDebugBitcode) { + IFT(ValidateLoadModule((const char *)pOptDebugBitcode->Ptr, + (uint32_t)pOptDebugBitcode->Size, pDebugModule, + Ctx, DiagStream, /*bLazyLoad*/ false)); } + return ValidateWithOptModules(pShader, Flags, nullptr, pDebugModule.get(), ppResult); } - return ValidateWithOptModules(pShader, Flags, nullptr, pM.get(), ppResult); + CATCH_CPP_ASSIGN_HRESULT(); + return hr; } HRESULT DxcValidator::ValidateWithOptModules( _In_ IDxcBlob *pShader, // Shader to validate. _In_ UINT32 Flags, // Validation flags. - _In_ llvm::Module *pModule, // Module to validate, if available. - _In_ llvm::Module *pDebugModule, // Debug module to validate, if available + _In_opt_ llvm::Module *pModule, // Module to validate, if available. + _In_opt_ llvm::Module *pDebugModule, // Debug module to validate, if available _COM_Outptr_ IDxcOperationResult **ppResult // Validation output status, buffer, and errors ) { *ppResult = nullptr; @@ -241,8 +257,8 @@ HRESULT STDMETHODCALLTYPE DxcValidator::GetFlags(_Out_ UINT32 *pFlags) { HRESULT DxcValidator::RunValidation( _In_ IDxcBlob *pShader, _In_ UINT32 Flags, // Validation flags. - _In_ llvm::Module *pModule, // Module to validate, if available. - _In_ llvm::Module *pDebugModule, // Debug module to validate, if available + _In_opt_ llvm::Module *pModule, // Module to validate, if available. + _In_opt_ llvm::Module *pDebugModule, // Debug module to validate, if available _In_ AbstractMemoryStream *pDiagStream) { // Run validation may throw, but that indicates an inability to validate,