From bf24b7a54d7d615bdf1cb225158fe4a99bb8102c Mon Sep 17 00:00:00 2001 From: Jeff Noyle Date: Tue, 3 Sep 2024 09:02:07 -0700 Subject: [PATCH] PIX: Rationalize UAV generation (#6883) The impetus for these changes was unexplained crashes in a display driver while attempting to create a debug-instrumented shader for PIX. The heart of it is the new test in pixtest.cpp: use the compiler to generate a raw UAV, and then compare the generated DXIL with what PIX generates for the same purpose. Some of the PIX passes need only one UAV for a module, but some need two or more. In the latter case, the previous code was a bit loose about what it was doing with respect to adding the UAV resource, and creating its handles for each interested function. Most of the actual changes herein are to do with that. Lastly, the PIX UAV is raw at the D3D API level, but the instrumentation had been doing non-raw writes. No driver seemed to care, but I've fixed it anyway. --- .../DxilAddPixelHitInstrumentation.cpp | 4 +- .../DxilDebugInstrumentation.cpp | 27 ++-- .../DxilOutputColorBecomesConstant.cpp | 2 +- .../DxilPIXDXRInvocationsLog.cpp | 4 +- ...DxilPIXMeshShaderOutputInstrumentation.cpp | 2 +- .../DxilShaderAccessTracking.cpp | 11 +- lib/DxilPIXPasses/PixPassHelpers.cpp | 97 +++++++------ lib/DxilPIXPasses/PixPassHelpers.h | 10 +- .../HLSLFileCheck/pix/AccessTracking.hlsl | 6 +- .../pix/AccessTrackingForSamplerFeedback.hlsl | 10 +- .../test/HLSLFileCheck/pix/DebugBasic.hlsl | 3 +- .../HLSLFileCheck/pix/DebugCSParameters.hlsl | 3 +- .../clang/test/HLSLFileCheck/pix/DebugMs.hlsl | 3 +- .../HLSLFileCheck/pix/DebugPSParameters.hlsl | 3 +- .../pix/DebugPreexistingSVInstance.hlsl | 3 +- .../pix/DebugPreexistingSVPosition.hlsl | 3 +- .../pix/DebugPreexistingSVVertex.hlsl | 3 +- .../HLSLFileCheck/pix/DebugVSParameters.hlsl | 3 +- .../HLSLFileCheck/pix/rawBufferStore.hlsl | 16 +-- tools/clang/unittests/HLSL/PixTest.cpp | 128 ++++++++++++++++-- 20 files changed, 245 insertions(+), 96 deletions(-) diff --git a/lib/DxilPIXPasses/DxilAddPixelHitInstrumentation.cpp b/lib/DxilPIXPasses/DxilAddPixelHitInstrumentation.cpp index 3c6c882b53..23bc224826 100644 --- a/lib/DxilPIXPasses/DxilAddPixelHitInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilAddPixelHitInstrumentation.cpp @@ -78,8 +78,8 @@ bool DxilAddPixelHitInstrumentation::runOnModule(Module &M) { IRBuilder<> Builder(dxilutil::FirstNonAllocaInsertionPt( PIXPassHelpers::GetEntryFunction(DM))); - HandleForUAV = - PIXPassHelpers::CreateUAV(DM, Builder, 0, "PIX_CountUAV_Handle"); + HandleForUAV = PIXPassHelpers::CreateUAVOnceForModule( + DM, Builder, 0, "PIX_CountUAV_Handle"); DM.ReEmitDxilResources(); } diff --git a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp index 451631a7fd..21c1199f91 100644 --- a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp @@ -322,7 +322,8 @@ class DxilDebugInstrumentation : public ModulePass { void applyOptions(PassOptions O) override; bool runOnModule(Module &M) override; - bool RunOnFunction(Module &M, DxilModule &DM, llvm::Function *function); + bool RunOnFunction(Module &M, DxilModule &DM, hlsl::DxilResource *uav, + llvm::Function *function); private: SystemValueIndices addRequiredSystemValues(BuilderContext &BC, @@ -447,6 +448,7 @@ DxilDebugInstrumentation::addRequiredSystemValues(BuilderContext &BC, case DXIL::ShaderKind::AnyHit: case DXIL::ShaderKind::ClosestHit: case DXIL::ShaderKind::Miss: + case DXIL::ShaderKind::Node: // Dispatch* thread Id is not in the input signature break; case DXIL::ShaderKind::Vertex: { @@ -703,6 +705,9 @@ void DxilDebugInstrumentation::addInvocationSelectionProlog( case DXIL::ShaderKind::Miss: ParameterTestResult = addRaygenShaderProlog(BC); break; + case DXIL::ShaderKind::Node: + ParameterTestResult = BC.HlslOP->GetI1Const(1); + break; case DXIL::ShaderKind::Compute: case DXIL::ShaderKind::Amplification: case DXIL::ShaderKind::Mesh: @@ -895,10 +900,10 @@ uint32_t DxilDebugInstrumentation::addDebugEntryValue(BuilderContext &BC, BytesToBeEmitted += addDebugEntryValue(BC, AsFloat); } else { Function *StoreValue = - BC.HlslOP->GetOpFunc(OP::OpCode::BufferStore, + BC.HlslOP->GetOpFunc(OP::OpCode::RawBufferStore, TheValue->getType()); // Type::getInt32Ty(BC.Ctx)); Constant *StoreValueOpcode = - BC.HlslOP->GetU32Const((unsigned)DXIL::OpCode::BufferStore); + BC.HlslOP->GetU32Const((unsigned)DXIL::OpCode::RawBufferStore); UndefValue *Undef32Arg = UndefValue::get(Type::getInt32Ty(BC.Ctx)); UndefValue *UndefArg = nullptr; if (TheValueTypeID == Type::TypeID::IntegerTyID) { @@ -913,6 +918,7 @@ uint32_t DxilDebugInstrumentation::addDebugEntryValue(BuilderContext &BC, Constant *WriteMask_X = BC.HlslOP->GetI8Const(1); auto &values = m_FunctionToValues[BC.Builder.GetInsertBlock()->getParent()]; + Constant *RawBufferStoreAlignment = BC.HlslOP->GetU32Const(4); (void)BC.Builder.CreateCall( StoreValue, {StoreValueOpcode, // i32 opcode @@ -923,7 +929,7 @@ uint32_t DxilDebugInstrumentation::addDebugEntryValue(BuilderContext &BC, UndefArg, // unused values UndefArg, // unused values UndefArg, // unused values - WriteMask_X}); + WriteMask_X, RawBufferStoreAlignment}); assert(m_RemainingReservedSpaceInBytes >= 4); // check for underflow m_RemainingReservedSpaceInBytes -= 4; @@ -1211,19 +1217,20 @@ bool DxilDebugInstrumentation::runOnModule(Module &M) { auto ShaderModel = DM.GetShaderModel(); auto shaderKind = ShaderModel->GetKind(); - + auto HLSLBindId = 0; + auto *uav = PIXPassHelpers::CreateGlobalUAVResource(DM, HLSLBindId, "PIXUAV"); bool modified = false; if (shaderKind == DXIL::ShaderKind::Library) { auto instrumentableFunctions = PIXPassHelpers::GetAllInstrumentableFunctions(DM); for (auto *F : instrumentableFunctions) { - if (RunOnFunction(M, DM, F)) { + if (RunOnFunction(M, DM, uav, F)) { modified = true; } } } else { llvm::Function *entryFunction = PIXPassHelpers::GetEntryFunction(DM); - modified = RunOnFunction(M, DM, entryFunction); + modified = RunOnFunction(M, DM, uav, entryFunction); } return modified; } @@ -1379,6 +1386,7 @@ DxilDebugInstrumentation::FindInstrumentableInstructionsInBlock( } bool DxilDebugInstrumentation::RunOnFunction(Module &M, DxilModule &DM, + hlsl::DxilResource *uav, llvm::Function *function) { DXIL::ShaderKind shaderKind = PIXPassHelpers::GetFunctionShaderKind(DM, function); @@ -1397,6 +1405,7 @@ bool DxilDebugInstrumentation::RunOnFunction(Module &M, DxilModule &DM, case DXIL::ShaderKind::AnyHit: case DXIL::ShaderKind::ClosestHit: case DXIL::ShaderKind::Miss: + case DXIL::ShaderKind::Node: break; default: return false; @@ -1431,8 +1440,8 @@ bool DxilDebugInstrumentation::RunOnFunction(Module &M, DxilModule &DM, break; } - values.UAVHandle = PIXPassHelpers::CreateUAV(DM, Builder, UAVRegisterId, - "PIX_DebugUAV_Handle"); + values.UAVHandle = PIXPassHelpers::CreateHandleForResource( + DM, Builder, uav, "PIX_DebugUAV_Handle"); auto SystemValues = addRequiredSystemValues(BC, shaderKind); addInvocationSelectionProlog(BC, SystemValues, shaderKind); diff --git a/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp b/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp index fd007a8799..a0749fbe0f 100644 --- a/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp +++ b/lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp @@ -161,7 +161,7 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) { std::unique_ptr pCBuf = llvm::make_unique(); pCBuf->SetGlobalName("PIX_ConstantColorCBName"); pCBuf->SetGlobalSymbol(UndefValue::get(CBStructTy)); - pCBuf->SetID(0); + pCBuf->SetID(static_cast(DM.GetCBuffers().size())); pCBuf->SetSpaceID( (unsigned int)-2); // This is the reserved-for-tools register space pCBuf->SetLowerBound(0); diff --git a/lib/DxilPIXPasses/DxilPIXDXRInvocationsLog.cpp b/lib/DxilPIXPasses/DxilPIXDXRInvocationsLog.cpp index fb7ee2de20..c9f553a4b6 100644 --- a/lib/DxilPIXPasses/DxilPIXDXRInvocationsLog.cpp +++ b/lib/DxilPIXPasses/DxilPIXDXRInvocationsLog.cpp @@ -88,9 +88,9 @@ bool DxilPIXDXRInvocationsLog::runOnModule(Module &M) { IRBuilder<> Builder(dxilutil::FirstNonAllocaInsertionPt(entryFunction)); // Add the UAVs that we're going to write to - CallInst *HandleForCountUAV = PIXPassHelpers::CreateUAV( + CallInst *HandleForCountUAV = PIXPassHelpers::CreateUAVOnceForModule( DM, Builder, /* registerID */ 0, "PIX_CountUAV_Handle"); - CallInst *HandleForUAV = PIXPassHelpers::CreateUAV( + CallInst *HandleForUAV = PIXPassHelpers::CreateUAVOnceForModule( DM, Builder, /* registerID */ 1, "PIX_UAV_Handle"); DM.ReEmitDxilResources(); diff --git a/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp b/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp index 4b0f84b968..507a718a4c 100644 --- a/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp @@ -337,7 +337,7 @@ bool DxilPIXMeshShaderOutputInstrumentation::runOnModule(Module &M) { m_OffsetMask = BC.HlslOP->GetU32Const(UAVDumpingGroundOffset() - 1); - m_OutputUAV = CreateUAV(DM, Builder, 0, "PIX_DebugUAV_Handle"); + m_OutputUAV = CreateUAVOnceForModule(DM, Builder, 0, "PIX_DebugUAV_Handle"); if (FirstNewStructGetMeshPayload == nullptr) { Instruction *firstInsertionPt = dxilutil::FirstNonAllocaInsertionPt( diff --git a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp index ffc3463db3..8299ff87d2 100644 --- a/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp +++ b/lib/DxilPIXPasses/DxilShaderAccessTracking.cpp @@ -907,6 +907,9 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { } } + auto uav = + PIXPassHelpers::CreateGlobalUAVResource(DM, 0u, "PIX_ShaderAccessUAV"); + for (auto *F : instrumentableFunctions) { DXIL::ShaderKind shaderKind = DXIL::ShaderKind::Invalid; if (!DM.HasDxilFunctionProps(F)) { @@ -922,8 +925,8 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { IRBuilder<> Builder(F->getEntryBlock().getFirstInsertionPt()); - m_FunctionToUAVHandle[F] = - PIXPassHelpers::CreateUAV(DM, Builder, 0u, "PIX_CountUAV_Handle"); + m_FunctionToUAVHandle[F] = PIXPassHelpers::CreateHandleForResource( + DM, Builder, uav, "PIX_ShaderAccessUAV_Handle"); OP *HlslOP = DM.GetOP(); for (int accessStyle = static_cast(ResourceAccessStyle::None); accessStyle < static_cast(ResourceAccessStyle::EndOfEnum); @@ -998,6 +1001,10 @@ bool DxilShaderAccessTracking::runOnModule(Module &M) { } for (unsigned iParam : handleParams) { + // Don't instrument the accesses to the UAV that we just added + if (Call->getArgOperand(iParam) == + m_FunctionToUAVHandle[CallerParent->getParent()]) + continue; auto res = GetResourceFromHandle(Call->getArgOperand(iParam), DM); if (res.accessStyle == AccessStyle::None) { continue; diff --git a/lib/DxilPIXPasses/PixPassHelpers.cpp b/lib/DxilPIXPasses/PixPassHelpers.cpp index 8220babdac..dfb4b3aa83 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.cpp +++ b/lib/DxilPIXPasses/PixPassHelpers.cpp @@ -66,20 +66,6 @@ void FindRayQueryHandlesForFunction(llvm::Function *F, } } -static unsigned int -GetNextRegisterIdForClass(hlsl::DxilModule &DM, - DXIL::ResourceClass resourceClass) { - switch (resourceClass) { - case DXIL::ResourceClass::CBuffer: - return static_cast(DM.GetCBuffers().size()); - case DXIL::ResourceClass::UAV: - return static_cast(DM.GetUAVs().size()); - default: - DXASSERT(false, "Unexpected resource class"); - return 0; - } -} - static bool IsDynamicResourceShaderModel(DxilModule &DM) { return DM.GetShaderModel()->IsSMAtLeast(6, 6); } @@ -88,6 +74,16 @@ static bool ShaderModelRequiresAnnotateHandle(DxilModule &DM) { return DM.GetShaderModel()->IsSMAtLeast(6, 6); } +static char const *RawUAVType() { return "struct.RWByteAddressBuffer"; } +static char const *ShaderModelHandleTypeName(DxilModule &DM) { + // Prior to sm6.6, lib handles were typed after the resource they denote. + // In 6.6 and after, and in all non-lib shader models, + // all handles are dx.types.Handle. + if (!DM.GetShaderModel()->IsLib() || DM.GetShaderModel()->IsSM66Plus()) + return "dx.types.Handle"; + return RawUAVType(); +} + llvm::CallInst *CreateHandleForResource(hlsl::DxilModule &DM, llvm::IRBuilder<> &Builder, hlsl::DxilResourceBase *resource, @@ -98,16 +94,16 @@ llvm::CallInst *CreateHandleForResource(hlsl::DxilModule &DM, DXIL::ResourceClass resourceClass = resource->GetClass(); - unsigned int resourceMetaDataId = - GetNextRegisterIdForClass(DM, resourceClass); - auto const *shaderModel = DM.GetShaderModel(); + Type *resourceHandleType = + DM.GetModule()->getTypeByName(ShaderModelHandleTypeName(DM)); if (shaderModel->IsLib()) { llvm::Constant *object = resource->GetGlobalSymbol(); - auto *load = Builder.CreateLoad(object); + Value *load = Builder.CreateLoad(object, resourceHandleType); + llvm::cast(load)->setAlignment(4); + llvm::cast(load)->setVolatile(false); Function *CreateHandleForLibOpFunc = - HlslOP->GetOpFunc(DXIL::OpCode::CreateHandleForLib, - resource->GetHLSLType()->getVectorElementType()); + HlslOP->GetOpFunc(DXIL::OpCode::CreateHandleForLib, load->getType()); Constant *CreateHandleForLibOpcodeArg = HlslOP->GetU32Const((unsigned)DXIL::OpCode::CreateHandleForLib); auto *handle = Builder.CreateCall(CreateHandleForLibOpFunc, @@ -170,10 +166,8 @@ llvm::CallInst *CreateHandleForResource(hlsl::DxilModule &DM, Constant *ClassArg = HlslOP->GetI8Const( static_cast::type>( resourceClass)); - Constant *MetaDataArg = HlslOP->GetU32Const( - resourceMetaDataId); // position of the metadata record in the - // corresponding metadata list - Constant *IndexArg = HlslOP->GetU32Const(0); // + Constant *MetaDataArg = HlslOP->GetU32Const(resource->GetID()); + Constant *IndexArg = HlslOP->GetU32Const(0); Constant *FalseArg = HlslOP->GetI1Const(0); // non-uniform resource index: false return Builder.CreateCall( @@ -275,29 +269,34 @@ static void AddUAVToDxilDefinedGlobalRootSignatures(DxilModule &DM) { } // Set up a UAV with structure of a single int -llvm::CallInst *CreateUAV(DxilModule &DM, IRBuilder<> &Builder, - unsigned int registerId, const char *name) { +hlsl::DxilResource *CreateGlobalUAVResource(hlsl::DxilModule &DM, + unsigned int hlslBindIndex, + const char *name) { LLVMContext &Ctx = DM.GetModule()->getContext(); - const char *PIXStructTypeName = "struct.RWByteAddressBuffer"; + const char *PIXStructTypeName = ShaderModelHandleTypeName(DM); llvm::StructType *UAVStructTy = DM.GetModule()->getTypeByName(PIXStructTypeName); + if (UAVStructTy == nullptr) { SmallVector Elements{Type::getInt32Ty(Ctx)}; UAVStructTy = llvm::StructType::create(Elements, PIXStructTypeName); - - // Since we only have to do this once per module, we can do it now when - // we're adding the singular UAV structure type to the module: - AddUAVToDxilDefinedGlobalRootSignatures(DM); - AddUAVToShaderAttributeRootSignature(DM); } + // Since this function should only be called once per module, + // we can modify the root sig at the same time: + AddUAVToDxilDefinedGlobalRootSignatures(DM); + AddUAVToShaderAttributeRootSignature(DM); + + unsigned int Id = static_cast(DM.GetUAVs().size()); std::unique_ptr pUAV = llvm::make_unique(); + pUAV->SetID(Id); auto const *shaderModel = DM.GetShaderModel(); + std::string PixUavName = "PIXUAV" + std::to_string(hlslBindIndex); if (shaderModel->IsLib()) { - auto *Global = DM.GetModule()->getOrInsertGlobal( - ("PIXUAV" + std::to_string(registerId)).c_str(), UAVStructTy); + auto *Global = + DM.GetModule()->getOrInsertGlobal(PixUavName.c_str(), UAVStructTy); GlobalVariable *NewGV = cast(Global); NewGV->setConstant(true); NewGV->setLinkage(GlobalValue::ExternalLinkage); @@ -308,17 +307,24 @@ llvm::CallInst *CreateUAV(DxilModule &DM, IRBuilder<> &Builder, pUAV->SetGlobalSymbol(UndefValue::get(UAVStructTy->getPointerTo())); } pUAV->SetGlobalName(name); - pUAV->SetID(GetNextRegisterIdForClass(DM, DXIL::ResourceClass::UAV)); pUAV->SetRW(true); // sets UAV class pUAV->SetSpaceID( - (unsigned int)-2); // This is the reserved-for-tools register space - pUAV->SetSampleCount(1); + (unsigned int)-2); // This is the reserved-for-tools register space + pUAV->SetSampleCount(0); // This is what compiler generates for a raw UAV pUAV->SetGloballyCoherent(false); pUAV->SetHasCounter(false); - pUAV->SetCompType(CompType::getI32()); - pUAV->SetLowerBound(registerId); + pUAV->SetCompType( + CompType::getInvalid()); // This is what compiler generates for a raw UAV + pUAV->SetLowerBound(hlslBindIndex); pUAV->SetRangeSize(1); + pUAV->SetElementStride(1); pUAV->SetKind(DXIL::ResourceKind::RawBuffer); + auto HLSLType = DM.GetModule()->getTypeByName(RawUAVType()); + if (HLSLType == nullptr) { + SmallVector Elements{Type::getInt32Ty(Ctx)}; + HLSLType = llvm::StructType::create(Elements, RawUAVType()); + } + pUAV->SetHLSLType(HLSLType->getPointerTo()); auto pAnnotation = DM.GetTypeSystem().GetStructAnnotation(UAVStructTy); if (pAnnotation == nullptr) { @@ -330,9 +336,18 @@ llvm::CallInst *CreateUAV(DxilModule &DM, IRBuilder<> &Builder, pAnnotation->GetFieldAnnotation(0).SetFieldName("count"); } - auto *handle = CreateHandleForResource(DM, Builder, pUAV.get(), name); - + auto *ret = pUAV.get(); DM.AddUAV(std::move(pUAV)); + return ret; +} + +// Set up a UAV with structure of a single int +llvm::CallInst *CreateUAVOnceForModule(hlsl::DxilModule &DM, + llvm::IRBuilder<> &Builder, + unsigned int hlslBindIndex, + const char *name) { + auto uav = CreateGlobalUAVResource(DM, hlslBindIndex, name); + auto *handle = CreateHandleForResource(DM, Builder, uav, name); return handle; } diff --git a/lib/DxilPIXPasses/PixPassHelpers.h b/lib/DxilPIXPasses/PixPassHelpers.h index a1b5ac3521..4cd0e1a549 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.h +++ b/lib/DxilPIXPasses/PixPassHelpers.h @@ -34,8 +34,14 @@ class ScopedInstruction { void FindRayQueryHandlesForFunction( llvm::Function *F, llvm::SmallPtrSetImpl &RayQueryHandles); -llvm::CallInst *CreateUAV(hlsl::DxilModule &DM, llvm::IRBuilder<> &Builder, - unsigned int registerId, const char *name); +enum class PixUAVHandleMode { NonLib, Lib }; +llvm::CallInst *CreateUAVOnceForModule(hlsl::DxilModule &DM, + llvm::IRBuilder<> &Builder, + unsigned int hlslBindIndex, + const char *name); +hlsl::DxilResource *CreateGlobalUAVResource(hlsl::DxilModule &DM, + unsigned int hlslBindIndex, + const char *name); llvm::CallInst *CreateHandleForResource(hlsl::DxilModule &DM, llvm::IRBuilder<> &Builder, hlsl::DxilResourceBase *resource, diff --git a/tools/clang/test/HLSLFileCheck/pix/AccessTracking.hlsl b/tools/clang/test/HLSLFileCheck/pix/AccessTracking.hlsl index b142af2d21..885d3c6d51 100644 --- a/tools/clang/test/HLSLFileCheck/pix/AccessTracking.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/AccessTracking.hlsl @@ -1,7 +1,7 @@ // RUN: %dxc -ECSMain -Tcs_6_0 %s | %opt -S -hlsl-dxil-pix-shader-access-instrumentation,config=S0:1:1i1;U0:2:10i0;.0;0;0. | %FileCheck %s -// Check we added the UAV: -// CHECK: %PIX_CountUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 1, i32 0, i1 false) +// Check we added the UAV: v----metadata position: not important for this check +// CHECK: %PIX_ShaderAccessUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 [[S:[0-9]+]], i32 0, i1 false) // check for correct out-of-bounds calculation // CHECK: CompareWithSlotLimit = icmp uge i32 @@ -12,7 +12,7 @@ // CHECK: slotIndex = mul i32 // Check for udpate of UAV: -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle ByteAddressBuffer inBuffer : register(t0); diff --git a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingForSamplerFeedback.hlsl b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingForSamplerFeedback.hlsl index 836c2403bd..32b8e743ca 100644 --- a/tools/clang/test/HLSLFileCheck/pix/AccessTrackingForSamplerFeedback.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/AccessTrackingForSamplerFeedback.hlsl @@ -1,16 +1,16 @@ // RUN: %dxc -Emain -Tcs_6_5 %s | %opt -S -hlsl-dxil-pix-shader-access-instrumentation,config=M0:0:1i0;S0:1:1i0;U0:2:10i0;.0;0;0. | %FileCheck %s -// Check we added the UAV: -// CHECK: %PIX_CountUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 1, i32 0, i1 false) +// Check we added the UAV: v----metadata position: not important for this check +// CHECK: %PIX_ShaderAccessUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 [[S:[0-9]+]], i32 0, i1 false) // Feedback UAV: -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle, i32 28 +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle, i32 28 // Texture: -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle, i32 12 +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle, i32 12 // Sampler: -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle, i32 0 +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle, i32 0 Texture2D texture : register(t0); SamplerState samplerState : register(s0); diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl index fece667083..6d6cd5454d 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugBasic.hlsl @@ -2,7 +2,8 @@ // Check that the basic starting header is present: -// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 0, i1 false) +// Check we added the UAV: v----metadata position: not important for this check +// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 [[S:[0-9]+]], i32 0, i1 false) // CHECK: %XPos = call float @dx.op.loadInput.f32(i32 4, i32 0, i32 0, i8 0, i32 undef) // CHECK: %YPos = call float @dx.op.loadInput.f32(i32 4, i32 0, i32 0, i8 1, i32 undef) // CHECK: %XIndex = fptoui float %XPos to i32 diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugCSParameters.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugCSParameters.hlsl index 644050133c..562e3b07d1 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugCSParameters.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugCSParameters.hlsl @@ -2,7 +2,8 @@ // Check that the CS thread IDs are added properly -// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 0, i1 false) +// Check we added the UAV: v----metadata position: not important for this check +// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 [[S:[0-9]+]], i32 0, i1 false) // CHECK: %ThreadIdX = call i32 @dx.op.threadId.i32(i32 93, i32 0) // CHECK: %ThreadIdY = call i32 @dx.op.threadId.i32(i32 93, i32 1) // CHECK: %ThreadIdZ = call i32 @dx.op.threadId.i32(i32 93, i32 2) diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugMs.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugMs.hlsl index 43b950f99c..9e86c60649 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugMs.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugMs.hlsl @@ -2,7 +2,8 @@ // Check that the MS thread IDs are added properly -// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 0, i1 false) +// Check we added the UAV: v----metadata position: not important for this check +// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 [[S:[0-9]+]], i32 0, i1 false) // CHECK: %ThreadIdX = call i32 @dx.op.threadId.i32(i32 93, i32 0) // CHECK: %ThreadIdY = call i32 @dx.op.threadId.i32(i32 93, i32 1) // CHECK: %ThreadIdZ = call i32 @dx.op.threadId.i32(i32 93, i32 2) diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugPSParameters.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugPSParameters.hlsl index 12144ba642..b1c9b99a98 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugPSParameters.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugPSParameters.hlsl @@ -2,7 +2,8 @@ // Check that the basic starting header is present: -// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 0, i1 false) +// Check we added the UAV: v----metadata position: not important for this check +// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 [[S:[0-9]+]], i32 0, i1 false) // CHECK: %XPos = call float @dx.op.loadInput.f32(i32 4, i32 0, i32 0, i8 0, i32 undef) // CHECK: %YPos = call float @dx.op.loadInput.f32(i32 4, i32 0, i32 0, i8 1, i32 undef) // CHECK: %XIndex = fptoui float %XPos to i32 diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVInstance.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVInstance.hlsl index d505d904c0..fc2ccc1b54 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVInstance.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVInstance.hlsl @@ -2,7 +2,8 @@ // Check that the SV_InstanceId check is present: -// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 0, i1 false) +// Check we added the UAV: v----metadata position: not important for this check +// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 [[S:[0-9]+]], i32 0, i1 false) // CHECK: %VertId = call i32 @dx.op.loadInput.i32(i32 4, i32 1, i32 0, i8 0, i32 undef) // CHECK: %InstanceId = call i32 @dx.op.loadInput.i32(i32 4, i32 0, i32 0, i8 0, i32 undef) // CHECK: %CompareToVertId = icmp eq i32 %VertId, 0 diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVPosition.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVPosition.hlsl index 7b5a6f31c6..6165f2afe4 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVPosition.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVPosition.hlsl @@ -2,7 +2,8 @@ // Check that the basic SV_Position check is present: -// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 0, i1 false) +// Check we added the UAV: v----metadata position: not important for this check +// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 [[S:[0-9]+]], i32 0, i1 false) // CHECK: %XPos = call float @dx.op.loadInput.f32(i32 4, i32 0, i32 0, i8 0, i32 undef) // CHECK: %YPos = call float @dx.op.loadInput.f32(i32 4, i32 0, i32 0, i8 1, i32 undef) // CHECK: %XIndex = fptoui float %XPos to i32 diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVVertex.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVVertex.hlsl index f9c047bcac..0d48d37250 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVVertex.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugPreexistingSVVertex.hlsl @@ -2,7 +2,8 @@ // Check that the vertex id check is present: -// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 0, i1 false) +// Check we added the UAV: v----metadata position: not important for this check +// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 [[S:[0-9]+]], i32 0, i1 false) // CHECK: %VertId = call i32 @dx.op.loadInput.i32(i32 4, i32 0, i32 0, i8 0, i32 undef) // CHECK: %InstanceId = call i32 @dx.op.loadInput.i32(i32 4, i32 1, i32 0, i8 0, i32 undef) // CHECK: %CompareToVertId = icmp eq i32 %VertId, 0 diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugVSParameters.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugVSParameters.hlsl index dad857a8ff..4f629ee0d6 100644 --- a/tools/clang/test/HLSLFileCheck/pix/DebugVSParameters.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/DebugVSParameters.hlsl @@ -2,7 +2,8 @@ // Check that the instance and vertex id are parsed and present: -// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 0, i1 false) +// Check we added the UAV: v----metadata position: not important for this check +// CHECK: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 [[S:[0-9]+]], i32 0, i1 false) // CHECK: %VertId = call i32 @dx.op.loadInput.i32(i32 4, i32 0, i32 0, i8 0, i32 undef) // CHECK: %InstanceId = call i32 @dx.op.loadInput.i32(i32 4, i32 1, i32 0, i8 0, i32 undef) // CHECK: %CompareToVertId = icmp eq i32 %VertId, 1 diff --git a/tools/clang/test/HLSLFileCheck/pix/rawBufferStore.hlsl b/tools/clang/test/HLSLFileCheck/pix/rawBufferStore.hlsl index a2c3d9d557..141cfc4eb8 100644 --- a/tools/clang/test/HLSLFileCheck/pix/rawBufferStore.hlsl +++ b/tools/clang/test/HLSLFileCheck/pix/rawBufferStore.hlsl @@ -2,24 +2,24 @@ // Check that the expected PIX UAV read-tracking is emitted (the atomicBinOp "|= 1") followed by the expected raw read: -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle // CHECK: call %dx.types.ResRet.f32 @dx.op.rawBufferLoad.f32 -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle // CHECK: call %dx.types.ResRet.i32 @dx.op.rawBufferLoad.i32 -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle // CHECK: call %dx.types.ResRet.f16 @dx.op.rawBufferLoad.f16 -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle // CHECK: call %dx.types.ResRet.i16 @dx.op.rawBufferLoad.i16 // Now the writes with atomicBinOp "|=2": -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle // CHECK: call void @dx.op.rawBufferStore.f32 -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle // CHECK: call void @dx.op.rawBufferStore.i32 -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle // CHECK: call void @dx.op.rawBufferStore.f16 -// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_CountUAV_Handle +// CHECK: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_ShaderAccessUAV_Handle // CHECK: call void @dx.op.rawBufferStore.i16 struct S diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index 58289767b7..34a727a794 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -99,6 +99,10 @@ class PixTest : public ::testing::Test { TEST_CLASS_SETUP(InitSupport); + TEST_METHOD(DebugUAV_CS_6_1) + TEST_METHOD(DebugUAV_CS_6_2) + TEST_METHOD(DebugUAV_lib_6_3_through_6_8) + TEST_METHOD(CompileDebugDisasmPDB) TEST_METHOD(AddToASPayload) @@ -208,6 +212,8 @@ class PixTest : public ::testing::Test { std::wstring debugArg = L"-hlsl-dxil-debug-instrumentation,UAVSize=" + std::to_wstring(UAVSize); Options.push_back(debugArg.c_str()); + Options.push_back(L"-viewid-state"); + Options.push_back(L"-hlsl-dxilemit"); CComPtr pOptimizedModule; CComPtr pText; @@ -349,21 +355,31 @@ class PixTest : public ::testing::Test { public: ModuleAndHangersOn(IDxcBlob *pBlob) { - // Verify we have a valid dxil container. + + // Assume we were given a dxil part first: + const DxilProgramHeader *pProgramHeader = + reinterpret_cast( + pBlob->GetBufferPointer()); + uint32_t partSize = static_cast(pBlob->GetBufferSize()); + // Check if we were given a valid dxil container instead: const DxilContainerHeader *pContainer = IsDxilContainerLike( pBlob->GetBufferPointer(), pBlob->GetBufferSize()); - VERIFY_IS_NOT_NULL(pContainer); - VERIFY_IS_TRUE(IsValidDxilContainer(pContainer, pBlob->GetBufferSize())); - - // Get Dxil part from container. - DxilPartIterator it = - std::find_if(begin(pContainer), end(pContainer), - DxilPartIsType(DFCC_ShaderDebugInfoDXIL)); - VERIFY_IS_FALSE(it == end(pContainer)); + if (pContainer != nullptr) { + VERIFY_IS_TRUE( + IsValidDxilContainer(pContainer, pBlob->GetBufferSize())); + + // Get Dxil part from container. + DxilPartIterator it = + std::find_if(begin(pContainer), end(pContainer), + DxilPartIsType(DFCC_ShaderDebugInfoDXIL)); + VERIFY_IS_FALSE(it == end(pContainer)); + + pProgramHeader = + reinterpret_cast(GetDxilPartData(*it)); + partSize = (*it)->PartSize; + } - const DxilProgramHeader *pProgramHeader = - reinterpret_cast(GetDxilPartData(*it)); - VERIFY_IS_TRUE(IsValidDxilProgramHeader(pProgramHeader, (*it)->PartSize)); + VERIFY_IS_TRUE(IsValidDxilProgramHeader(pProgramHeader, partSize)); // Get a pointer to the llvm bitcode. const char *pIL; @@ -417,6 +433,8 @@ class PixTest : public ::testing::Test { std::string RunDxilPIXAddTidToAmplificationShaderPayloadPass(IDxcBlob *blob); CComPtr RunDxilPIXMeshShaderOutputPass(IDxcBlob *blob); CComPtr RunDxilPIXDXRInvocationsLog(IDxcBlob *blob); + void TestPixUAVCase(char const *hlsl, wchar_t const *model, + wchar_t const *entry); }; bool PixTest::InitSupport() { @@ -427,6 +445,92 @@ bool PixTest::InitSupport() { return true; } +void PixTest::TestPixUAVCase(char const *hlsl, wchar_t const *model, + wchar_t const *entry) { + auto mod = Compile(m_dllSupport, hlsl, model, {}, entry); + CComPtr dxilPart = FindModule(DFCC_ShaderDebugInfoDXIL, mod); + PassOutput passOutput = RunDebugPass(dxilPart); + CComPtr modifiedDxilContainer; + ReplaceDxilBlobPart(mod->GetBufferPointer(), mod->GetBufferSize(), + passOutput.blob, &modifiedDxilContainer); + + ModuleAndHangersOn moduleEtc(modifiedDxilContainer); + auto &compilerGeneratedUAV = moduleEtc.GetDxilModule().GetUAV(0); + auto &pixDebugGeneratedUAV = moduleEtc.GetDxilModule().GetUAV(1); + VERIFY_ARE_EQUAL(compilerGeneratedUAV.GetClass(), + pixDebugGeneratedUAV.GetClass()); + VERIFY_ARE_EQUAL(compilerGeneratedUAV.GetKind(), + pixDebugGeneratedUAV.GetKind()); + VERIFY_ARE_EQUAL(compilerGeneratedUAV.GetHLSLType(), + pixDebugGeneratedUAV.GetHLSLType()); + VERIFY_ARE_EQUAL(compilerGeneratedUAV.GetSampleCount(), + pixDebugGeneratedUAV.GetSampleCount()); + VERIFY_ARE_EQUAL(compilerGeneratedUAV.GetElementStride(), + pixDebugGeneratedUAV.GetElementStride()); + VERIFY_ARE_EQUAL(compilerGeneratedUAV.GetBaseAlignLog2(), + pixDebugGeneratedUAV.GetBaseAlignLog2()); + VERIFY_ARE_EQUAL(compilerGeneratedUAV.GetCompType(), + pixDebugGeneratedUAV.GetCompType()); + VERIFY_ARE_EQUAL(compilerGeneratedUAV.GetSamplerFeedbackType(), + pixDebugGeneratedUAV.GetSamplerFeedbackType()); + VERIFY_ARE_EQUAL(compilerGeneratedUAV.IsGloballyCoherent(), + pixDebugGeneratedUAV.IsGloballyCoherent()); + VERIFY_ARE_EQUAL(compilerGeneratedUAV.HasCounter(), + pixDebugGeneratedUAV.HasCounter()); + VERIFY_ARE_EQUAL(compilerGeneratedUAV.HasAtomic64Use(), + pixDebugGeneratedUAV.HasAtomic64Use()); + + VERIFY_ARE_EQUAL(compilerGeneratedUAV.GetGlobalSymbol()->getType(), + pixDebugGeneratedUAV.GetGlobalSymbol()->getType()); +} + +TEST_F(PixTest, DebugUAV_CS_6_1) { + const char *hlsl = R"( +RWByteAddressBuffer RawUAV : register(u0); +[numthreads(1, 1, 1)] +void CSMain() +{ + RawUAV.Store(0, RawUAV.Load(4)); +} +)"; + TestPixUAVCase(hlsl, L"cs_6_1", L"CSMain"); +} + +TEST_F(PixTest, DebugUAV_CS_6_2) { + const char *hlsl = R"( +RWByteAddressBuffer RawUAV : register(u0); +[numthreads(1, 1, 1)] +void CSMain() +{ + RawUAV.Store(0, RawUAV.Load(4)); +} +)"; + // In 6.2, rawBufferLoad replaced bufferLoad for UAVs, but we don't + // expect this test to notice the difference. We just test 6.2 + TestPixUAVCase(hlsl, L"cs_6_2", L"CSMain"); +} + +TEST_F(PixTest, DebugUAV_lib_6_3_through_6_8) { + const char *hlsl = R"( +RWByteAddressBuffer RawUAV : register(u0); +struct [raypayload] Payload +{ + double a : read(caller, closesthit, anyhit) : write(caller, miss, closesthit); +}; +[shader("miss")] +void Miss( inout Payload payload ) +{ + RawUAV.Store(0, RawUAV.Load(4)); + payload.a = 4.2; +})"; + TestPixUAVCase(hlsl, L"lib_6_3", L""); + TestPixUAVCase(hlsl, L"lib_6_4", L""); + TestPixUAVCase(hlsl, L"lib_6_5", L""); + TestPixUAVCase(hlsl, L"lib_6_6", L""); + TestPixUAVCase(hlsl, L"lib_6_7", L""); + TestPixUAVCase(hlsl, L"lib_6_8", L""); +} + TEST_F(PixTest, CompileDebugDisasmPDB) { const char *hlsl = R"( [RootSignature("")]