From c4e3f96f575788bc1c8eebb685e8e5389da20df8 Mon Sep 17 00:00:00 2001 From: Jake Turner Date: Wed, 15 Nov 2023 15:51:40 +0000 Subject: [PATCH] VK Shader Debugger support for Buffer Device Address --- .../driver/shaders/spirv/spirv_debug.cpp | 68 ++- renderdoc/driver/shaders/spirv/spirv_debug.h | 13 + .../shaders/spirv/spirv_debug_setup.cpp | 416 ++++++++++++++---- .../driver/shaders/spirv/spirv_processor.cpp | 8 + .../driver/shaders/spirv/spirv_processor.h | 2 + .../driver/shaders/spirv/spirv_reflect.cpp | 12 + renderdoc/driver/vulkan/vk_info.h | 1 + renderdoc/driver/vulkan/vk_shaderdebug.cpp | 101 ++++- .../vulkan/wrappers/vk_resource_funcs.cpp | 2 + 9 files changed, 517 insertions(+), 106 deletions(-) diff --git a/renderdoc/driver/shaders/spirv/spirv_debug.cpp b/renderdoc/driver/shaders/spirv/spirv_debug.cpp index 75ba75c66..cccf6ef9e 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug.cpp @@ -78,6 +78,7 @@ inline uint64_t CountOnes(uint64_t value) namespace rdcspv { const BindpointIndex DebugAPIWrapper::invalidBind = BindpointIndex(-12345, -12345, ~0U); +const BindpointIndex DebugAPIWrapper::pointerBind = BindpointIndex(-12345, -67890, ~0U); ThreadState::ThreadState(uint32_t workgroupIdx, Debugger &debug, const GlobalState &globalState) : debugger(debug), global(globalState) @@ -256,6 +257,8 @@ void ThreadState::WritePointerValue(Id pointer, const ShaderVariable &val) // if var is a pointer we update the underlying storage and generate at least one change, // plus any additional ones for other pointers. Id ptrid = debugger.GetPointerBaseId(var); + if(ptrid == Id()) + ptrid = pointer; // only track local writes when we don't have debug info, so we can track variables first // becoming alive @@ -265,7 +268,7 @@ void ThreadState::WritePointerValue(Id pointer, const ShaderVariable &val) ShaderVariableChange basechange; - if(debugger.IsOpaquePointer(ids[ptrid])) + if(debugger.IsOpaquePointer(ids[ptrid]) || debugger.IsPhysicalPointer(ids[ptrid])) { // if this is a write to a SSBO pointer, don't record any alias changes, just record a no-op // change to this pointer @@ -355,7 +358,7 @@ void ThreadState::SetDst(Id id, const ShaderVariable &val) auto it = std::lower_bound(live.begin(), live.end(), id); live.insert(it - live.begin(), id); - if(val.type == VarType::GPUPointer) + if(val.type == VarType::GPUPointer && !debugger.IsPhysicalPointer(val)) { Id ptrId = debugger.GetPointerBaseId(val); if(ptrId != Id() && ptrId != id) @@ -392,7 +395,8 @@ void ThreadState::ProcessScopeChange(const rdcarray &oldLive, const rdcarray m_State->changes.push_back({debugger.GetPointerValue(ids[id])}); - if(ids[id].type == VarType::GPUPointer && !debugger.IsOpaquePointer(ids[id])) + if(ids[id].type == VarType::GPUPointer && !debugger.IsOpaquePointer(ids[id]) && + !debugger.IsPhysicalPointer(ids[id])) { Id ptrId = debugger.GetPointerBaseId(ids[id]); pointersForId[ptrId].removeOne(id); @@ -699,7 +703,28 @@ void ThreadState::StepNext(ShaderDebugState *state, const rdcarray SetDst(chain.result, debugger.MakeCompositePointer( ids[chain.base], debugger.GetPointerBaseId(ids[chain.base]), indices)); + break; + } + case Op::PtrAccessChain: + case Op::InBoundsPtrAccessChain: + { + OpPtrAccessChain chain(it); + rdcarray indices; + // evaluate the indices + indices.reserve(chain.indexes.size()); + for(Id id : chain.indexes) + indices.push_back(uintComp(GetSrc(id), 0)); + + ShaderVariable base = ids[chain.base]; + PointerVal val = base.GetPointer(); + int32_t element = intComp(GetSrc(chain.element), 0); + // adjust the address by the element. We should have the array stride since the base pointer + // must point into an array and we can't go outside it. + base.SetTypedPointer(val.pointer + element * debugger.GetPointerArrayStride(base), val.shader, + val.pointerTypeID); + SetDst(chain.result, + debugger.MakeCompositePointer(base, debugger.GetPointerBaseId(base), indices)); break; } case Op::ArrayLength: @@ -708,6 +733,9 @@ void ThreadState::StepNext(ShaderDebugState *state, const rdcarray ShaderVariable structPointer = GetSrc(len.structure); + // "Structure must be a logical pointer..." which is opaqaue in RD terminolgoy + RDCASSERT(debugger.IsOpaquePointer(structPointer)); + // get the pointer base offset (should be zero for any binding but could be non-zero for a // buffer_device_address pointer) uint64_t offset = debugger.GetPointerByteOffset(structPointer); @@ -760,6 +788,24 @@ void ThreadState::StepNext(ShaderDebugState *state, const rdcarray SetDst(equal.result, var); break; } + // physical storage pointers + case Op::ConvertPtrToU: + { + OpConvertPtrToU convert(it); + ShaderVariable ptr = GetSrc(convert.pointer); + const DataType &resultType = debugger.GetType(convert.resultType); + ptr.type = resultType.scalar().Type(); + SetDst(convert.result, ptr); + break; + } + case Op::ConvertUToPtr: + { + OpConvertUToPtr convert(it); + ShaderVariable ptr = GetSrc(convert.integerValue); + const DataType &type = debugger.GetType(convert.resultType); + SetDst(convert.result, debugger.MakeTypedPointer(ptr.value.u64v[0], type)); + break; + } ////////////////////////////////////////////////////////////////////////////// // @@ -1280,7 +1326,12 @@ void ThreadState::StepNext(ShaderDebugState *state, const rdcarray const DataType &type = debugger.GetType(cast.resultType); ShaderVariable var = GetSrc(cast.operand); - if((type.type == DataType::ScalarType && var.columns == 1) || type.vector().count == var.columns) + if(type.type == DataType::PointerType) + { + var = debugger.MakeTypedPointer(var.value.u64v[0], type); + } + else if((type.type == DataType::ScalarType && var.columns == 1) || + type.vector().count == var.columns) { // if the column count is unchanged, just change the underlying type var.type = type.scalar().Type(); @@ -3577,16 +3628,11 @@ void ThreadState::StepNext(ShaderDebugState *state, const rdcarray break; } - // TODO physical storage pointers - case Op::ConvertPtrToU: - case Op::ConvertUToPtr: - case Op::PtrAccessChain: - case Op::InBoundsPtrAccessChain: case Op::PtrDiff: { RDCERR( - "Physical storage pointers not supported. SPIR-V should have been rejected by " - "capability!"); + "Variable pointers are not supported, PtrDiff must only be used with variable pointers, " + "not physical pointers"); ShaderVariable var("", 0U, 0U, 0U, 0U); var.columns = 1; diff --git a/renderdoc/driver/shaders/spirv/spirv_debug.h b/renderdoc/driver/shaders/spirv/spirv_debug.h index f8b1d90c9..fc6e1a322 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug.h +++ b/renderdoc/driver/shaders/spirv/spirv_debug.h @@ -52,12 +52,17 @@ public: virtual ~DebugAPIWrapper() {} virtual void AddDebugMessage(MessageCategory c, MessageSeverity sv, MessageSource src, rdcstr d) = 0; + virtual ResourceId GetShaderID() = 0; + virtual uint64_t GetBufferLength(BindpointIndex bind) = 0; virtual void ReadBufferValue(BindpointIndex bind, uint64_t offset, uint64_t byteSize, void *dst) = 0; virtual void WriteBufferValue(BindpointIndex bind, uint64_t offset, uint64_t byteSize, const void *src) = 0; + virtual void ReadAddress(uint64_t address, uint64_t byteSize, void *dst) = 0; + virtual void WriteAddress(uint64_t address, uint64_t byteSize, const void *src) = 0; + virtual bool ReadTexel(BindpointIndex imageBind, const ShaderVariable &coord, uint32_t sample, ShaderVariable &output) = 0; virtual bool WriteTexel(BindpointIndex imageBind, const ShaderVariable &coord, uint32_t sample, @@ -78,6 +83,7 @@ public: }; static const BindpointIndex invalidBind; + static const BindpointIndex pointerBind; virtual bool CalculateSampleGather(ThreadState &lane, Op opcode, TextureType texType, BindpointIndex imageBind, BindpointIndex samplerBind, @@ -383,8 +389,12 @@ public: DebugAPIWrapper::TextureType GetTextureType(const ShaderVariable &img) const; ShaderVariable MakePointerVariable(Id id, const ShaderVariable *v, uint8_t scalar0 = 0xff, uint8_t scalar1 = 0xff) const; + ShaderVariable MakeTypedPointer(uint64_t value, const DataType &type) const; Id GetPointerBaseId(const ShaderVariable &v) const; + uint32_t GetPointerArrayStride(const ShaderVariable &ptr) const; bool IsOpaquePointer(const ShaderVariable &v) const; + bool IsPhysicalPointer(const ShaderVariable &v) const; + bool ArePointersAndEqual(const ShaderVariable &a, const ShaderVariable &b) const; void WriteThroughPointer(ShaderVariable &ptr, const ShaderVariable &val); ShaderVariable MakeCompositePointer(const ShaderVariable &base, Id id, rdcarray &indices); @@ -453,6 +463,9 @@ private: SparseIdMap labelInstruction; + SparseIdMap idToPointerType; + rdcarray pointerTypeToId; + // the live mutable global variables, to initialise a stack frame's live list rdcarray liveGlobals; diff --git a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp index 412c3630c..2622c24ee 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp @@ -46,6 +46,7 @@ enum class PointerFlags RowMajorMatrix = 0x1, SSBO = 0x2, GlobalArrayBinding = 0x4, + DereferencedPhysical = 0x8, }; BITMASK_OPERATORS(PointerFlags); @@ -176,6 +177,17 @@ rdcspv::Id getBufferTypeId(const ShaderVariable &var) return rdcspv::Id::fromWord((uint32_t)var.value.u64v[8]); } +// slot 9 is the array stride. Can't be shared with matrix stride (slot 5) in the case of matrix arrays. +void setArrayStride(ShaderVariable &var, uint32_t stride) +{ + var.value.u64v[9] = stride; +} + +uint32_t getArrayStride(const ShaderVariable &var) +{ + return (uint32_t)var.value.u64v[9]; +} + static ShaderVariable *pointerIfMutable(const ShaderVariable &var) { return NULL; @@ -415,6 +427,7 @@ void Reflector::CheckDebuggable(bool &debuggable, rdcstr &debugStatus) const "SPV_EXT_fragment_invocation_density", "SPV_KHR_no_integer_wrap_decoration", "SPV_KHR_float_controls", + "SPV_EXT_physical_storage_buffer", "SPV_KHR_shader_clock", "SPV_EXT_demote_to_helper_invocation", "SPV_KHR_non_semantic_info", @@ -422,6 +435,7 @@ void Reflector::CheckDebuggable(bool &debuggable, rdcstr &debugStatus) const "SPV_KHR_terminate_invocation", "SPV_EXT_shader_image_int64", "SPV_GOOGLE_user_type", + "SPV_KHR_physical_storage_buffer", }; // whitelist supported extensions @@ -538,6 +552,7 @@ void Reflector::CheckDebuggable(bool &debuggable, rdcstr &debugStatus) const case Capability::BitInstructions: case Capability::UniformDecoration: case Capability::SignedZeroInfNanPreserve: + case Capability::PhysicalStorageBufferAddresses: { supported = true; break; @@ -558,9 +573,6 @@ void Reflector::CheckDebuggable(bool &debuggable, rdcstr &debugStatus) const // we plan to support these but needs additional testing/proving - // physical pointers - case Capability::PhysicalStorageBufferAddresses: - // MSAA custom interpolation case Capability::InterpolationFunction: @@ -1107,6 +1119,10 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *api, const ShaderStage s { this->apiWrapper->ReadBufferValue(bindpoint, offset, VarByteSize(var), var.value.u8v.data()); + + if(type.type == DataType::PointerType) + var.SetTypedPointer(var.value.u64v[0], this->apiWrapper->GetShaderID(), + idToPointerType[type.InnerType()]); } else { @@ -2023,6 +2039,24 @@ rdcarray Debugger::ContinueDebug() return ret; } +ShaderVariable Debugger::MakeTypedPointer(uint64_t value, const DataType &type) const +{ + rdcspv::Id typeId = type.InnerType(); + ShaderVariable var; + var.rows = var.columns = 1; + var.type = VarType::GPUPointer; + var.SetTypedPointer(value, apiWrapper ? apiWrapper->GetShaderID() : ResourceId(), + idToPointerType[typeId]); + + const Decorations &dec = decorations[type.id]; + if(dec.flags & Decorations::HasArrayStride) + { + uint32_t arrayStride = dec.arrayStride; + setArrayStride(var, arrayStride); + } + return var; +} + ShaderVariable Debugger::MakePointerVariable(Id id, const ShaderVariable *v, uint8_t scalar0, uint8_t scalar1) const { @@ -2042,13 +2076,17 @@ ShaderVariable Debugger::MakeCompositePointer(const ShaderVariable &base, Id id, { const ShaderVariable *leaf = &base; - // if the base is a plain value, we just start walking down the chain. If the base is a pointer - // though, we want to step down the chain in the underlying storage, so dereference first. - if(base.type == VarType::GPUPointer) - leaf = getPointer(base); - + bool physicalPointer = IsPhysicalPointer(base); bool isArray = false; + if(!physicalPointer) + { + // if the base is a plain value, we just start walking down the chain. If the base is a pointer + // though, we want to step down the chain in the underlying storage, so dereference first. + if(base.type == VarType::GPUPointer) + leaf = getPointer(base); + } + // if this is an arrayed opaque binding, the first index is a 'virtual' array index into the // binding. // We only take this if this is the FIRST dereference from the global pointer. @@ -2061,19 +2099,42 @@ ShaderVariable Debugger::MakeCompositePointer(const ShaderVariable &base, Id id, isArray = true; } - if(leaf->type == VarType::ReadWriteResource && checkPointerFlags(*leaf, PointerFlags::SSBO)) + if((leaf->type == VarType::ReadWriteResource && checkPointerFlags(*leaf, PointerFlags::SSBO)) || + physicalPointer) { - ShaderVariable ret = MakePointerVariable(id, leaf); + ShaderVariable ret; + uint64_t byteOffset = 0; + const DataType *type = NULL; + + if(physicalPointer) + { + // work purely with the pointer itself. All we're going to do effectively is move the address + // and set the sub-type pointed to so that we know how to dereference it later + ret = *leaf; + // if this hasn't been dereferenced yet we should have a valid pointer type ID for a physical + // pointer, which we can then use to get the base ID (and there will be no buffer type ID + // below). If not, we rely on the buffer type ID. + if(!checkPointerFlags(ret, PointerFlags::DereferencedPhysical)) + { + rdcspv::Id typeId = pointerTypeToId[ret.GetPointer().pointerTypeID]; + RDCASSERT(typeId != rdcspv::Id()); + type = &dataTypes[typeId]; + } + } + else + { + ret = MakePointerVariable(id, leaf); + + byteOffset = getByteOffset(base); + type = &dataTypes[idTypes[id]]; + + RDCASSERT(type->type == DataType::PointerType); + type = &dataTypes[type->InnerType()]; + } - uint64_t byteOffset = getByteOffset(base); setMatrixStride(ret, getMatrixStride(base)); setPointerFlags(ret, getPointerFlags(base)); - const DataType *type = &dataTypes[idTypes[id]]; - - RDCASSERT(type->type == DataType::PointerType); - type = &dataTypes[type->InnerType()]; - rdcspv::Id typeId = getBufferTypeId(base); if(typeId != rdcspv::Id()) @@ -2095,6 +2156,8 @@ ShaderVariable Debugger::MakeCompositePointer(const ShaderVariable &base, Id id, Decorations curDecorations = decorations[type->id]; + uint32_t arrayStride = 0; + while(i < indices.size() && (type->type == DataType::ArrayType || type->type == DataType::StructType)) { @@ -2105,7 +2168,8 @@ ShaderVariable Debugger::MakeCompositePointer(const ShaderVariable &base, Id id, RDCASSERT(dec.flags & Decorations::HasArrayStride); // offset increases by index * arrayStride - byteOffset += indices[i] * dec.arrayStride; + arrayStride = dec.arrayStride; + byteOffset += indices[i] * arrayStride; // new type is the inner type type = &dataTypes[type->InnerType()]; @@ -2126,14 +2190,6 @@ ShaderVariable Debugger::MakeCompositePointer(const ShaderVariable &base, Id id, i++; } - if(curDecorations.flags & Decorations::HasMatrixStride) - setMatrixStride(ret, curDecorations.matrixStride); - - if(curDecorations.flags & Decorations::RowMajor) - enablePointerFlags(ret, PointerFlags::RowMajorMatrix); - else if(curDecorations.flags & Decorations::ColMajor) - disablePointerFlags(ret, PointerFlags::RowMajorMatrix); - size_t remaining = indices.size() - i; if(remaining == 2) { @@ -2186,9 +2242,39 @@ ShaderVariable Debugger::MakeCompositePointer(const ShaderVariable &base, Id id, } } - setBufferTypeId(ret, type->id); - setByteOffset(ret, byteOffset); + if(curDecorations.flags & Decorations::HasMatrixStride) + setMatrixStride(ret, curDecorations.matrixStride); + if(curDecorations.flags & Decorations::RowMajor) + enablePointerFlags(ret, PointerFlags::RowMajorMatrix); + else if(curDecorations.flags & Decorations::ColMajor) + disablePointerFlags(ret, PointerFlags::RowMajorMatrix); + + setBufferTypeId(ret, type->id); + setArrayStride(ret, arrayStride); + if(physicalPointer) + { + PointerVal ptrval = ret.GetPointer(); + // we use the opaque type ID to ensure we don't accidentally leak the wrong type ID. + // we check where the pointer is dereferenced to use the physical address instead of the inner + // binding + ret.SetTypedPointer(ptrval.pointer + byteOffset, ptrval.shader, OpaquePointerTypeID); + } + else + { + setByteOffset(ret, byteOffset); + } + // this flag is only used for physical pointers, to indicate that it's been dereferenced and + // the pointer type should be fetched from our ID above and it returned as a plain value, rather + // than showing the pointer 'natively'. This is because we may not have a pointer type to + // reference if e.g. the only pointer type registered is struct foo { } and we've dereferenced + // into inner struct bar { } + // + // effectively physical pointers currently decay into opaque pointers after any access chain + // (but opaque that still uses an address, not that uses a global pointer inner value as in + // other opaque pointers) + if(physicalPointer) + enablePointerFlags(ret, PointerFlags::DereferencedPhysical); return ret; } @@ -2265,6 +2351,11 @@ ShaderVariable Debugger::GetPointerValue(const ShaderVariable &ptr) const ret.SetBinding(bind.bindset, bind.bind, getBindArrayIndex(ptr)); return ret; } + // physical pointers which haven't been dereferenced are returned as-is, they're ready for display + else if(IsPhysicalPointer(ptr) && !checkPointerFlags(ptr, PointerFlags::DereferencedPhysical)) + { + return ptr; + } // every other kind of pointer displays as its contents return ReadFromPointer(ptr); @@ -2275,35 +2366,72 @@ ShaderVariable Debugger::ReadFromPointer(const ShaderVariable &ptr) const if(ptr.type != VarType::GPUPointer) return ptr; - const ShaderVariable *inner = getPointer(ptr); - ShaderVariable ret; - - if(inner->type == VarType::ReadWriteResource && checkPointerFlags(*inner, PointerFlags::SSBO)) + // values for setting up pointer reads, either from a physical pointer or from an opaque pointer + rdcspv::Id typeId; + Decorations parentDecorations; + uint64_t baseAddress; + BindpointIndex bind; + uint64_t byteOffset = 0; + std::function pointerReadCallback; + if(IsPhysicalPointer(ptr)) { - rdcspv::Id typeId = getBufferTypeId(ptr); - uint64_t byteOffset = getByteOffset(ptr); - - BindpointIndex bind = inner->GetBinding(); - bind.arrayIndex = getBindArrayIndex(ptr); - - uint32_t varMatrixStride = getMatrixStride(ptr); - - Decorations parentDecorations; - if(checkPointerFlags(ptr, PointerFlags::RowMajorMatrix)) - parentDecorations.flags = Decorations::RowMajor; + if(checkPointerFlags(ptr, PointerFlags::DereferencedPhysical)) + typeId = getBufferTypeId(ptr); else - parentDecorations.flags = Decorations::ColMajor; + typeId = pointerTypeToId[ptr.GetPointer().pointerTypeID]; + + RDCASSERT(typeId != rdcspv::Id()); + + parentDecorations = decorations[typeId]; + uint32_t varMatrixStride = getMatrixStride(ptr); if(varMatrixStride != 0) { + if(checkPointerFlags(ptr, PointerFlags::RowMajorMatrix)) + parentDecorations.flags = Decorations::RowMajor; + else + parentDecorations.flags = Decorations::ColMajor; parentDecorations.flags = Decorations::Flags(parentDecorations.flags | Decorations::HasMatrixStride); parentDecorations.matrixStride = varMatrixStride; } + baseAddress = ptr.GetPointer().pointer; + pointerReadCallback = [this, baseAddress](uint64_t offset, uint64_t size, void *dst) { + apiWrapper->ReadAddress(baseAddress + offset, size, dst); + }; + } + else + { + const ShaderVariable *inner = getPointer(ptr); + if(inner->type == VarType::ReadWriteResource && checkPointerFlags(*inner, PointerFlags::SSBO)) + { + typeId = getBufferTypeId(ptr); + byteOffset = getByteOffset(ptr); + bind = inner->GetBinding(); + bind.arrayIndex = getBindArrayIndex(ptr); + uint32_t varMatrixStride = getMatrixStride(ptr); + if(varMatrixStride != 0) + { + if(checkPointerFlags(ptr, PointerFlags::RowMajorMatrix)) + parentDecorations.flags = Decorations::RowMajor; + else + parentDecorations.flags = Decorations::ColMajor; + parentDecorations.flags = + Decorations::Flags(parentDecorations.flags | Decorations::HasMatrixStride); + parentDecorations.matrixStride = varMatrixStride; + } + pointerReadCallback = [this, bind](uint64_t offset, uint64_t size, void *dst) { + apiWrapper->ReadBufferValue(bind, offset, size, dst); + }; + } + } - auto readCallback = [this, bind](ShaderVariable &var, const Decorations &dec, - const DataType &type, uint64_t offset, const rdcstr &) { + if(pointerReadCallback) + { + auto readCallback = [this, pointerReadCallback](ShaderVariable &var, const Decorations &dec, + const DataType &type, uint64_t offset, + const rdcstr &) { // ignore any callbacks we get on the way up for structs/arrays, we don't need it we only read // or write at primitive level if(!var.members.empty()) @@ -2320,9 +2448,8 @@ ShaderVariable Debugger::ReadFromPointer(const ShaderVariable &ptr) const { for(uint8_t r = 0; r < var.rows; r++) { - apiWrapper->ReadBufferValue(bind, offset + r * matrixStride, - VarTypeByteSize(var.type) * var.columns, - VarElemPointer(var, r * var.columns)); + pointerReadCallback(offset + r * matrixStride, VarTypeByteSize(var.type) * var.columns, + VarElemPointer(var, r * var.columns)); } } else @@ -2333,9 +2460,8 @@ ShaderVariable Debugger::ReadFromPointer(const ShaderVariable &ptr) const // read column-wise for(uint8_t c = 0; c < var.columns; c++) { - apiWrapper->ReadBufferValue(bind, offset + c * matrixStride, - VarTypeByteSize(var.type) * var.rows, - VarElemPointer(tmp, c * var.rows)); + pointerReadCallback(offset + c * matrixStride, VarTypeByteSize(var.type) * var.rows, + VarElemPointer(tmp, c * var.rows)); } // transpose into our row major storage @@ -2349,21 +2475,36 @@ ShaderVariable Debugger::ReadFromPointer(const ShaderVariable &ptr) const if(!rowMajor) { // we can read a vector at a time if the matrix is column major - apiWrapper->ReadBufferValue(bind, offset, VarTypeByteSize(var.type) * var.columns, - VarElemPointer(var, 0)); + pointerReadCallback(offset, VarTypeByteSize(var.type) * var.columns, + VarElemPointer(var, 0)); } else { for(uint8_t c = 0; c < var.columns; c++) { - apiWrapper->ReadBufferValue(bind, offset + c * matrixStride, VarTypeByteSize(var.type), - VarElemPointer(var, c)); + pointerReadCallback(offset + c * matrixStride, VarTypeByteSize(var.type), + VarElemPointer(var, c)); } } } - else if(type.type == DataType::ScalarType) + else if(type.type == DataType::ScalarType || type.type == DataType::PointerType) { - apiWrapper->ReadBufferValue(bind, offset, VarTypeByteSize(var.type), VarElemPointer(var, 0)); + pointerReadCallback(offset, VarTypeByteSize(var.type), VarElemPointer(var, 0)); + if(type.type == DataType::PointerType) + { + auto it = idToPointerType.find(type.InnerType()); + if(it != idToPointerType.end()) + { + var.SetTypedPointer(var.value.u64v[0], this->apiWrapper->GetShaderID(), it->second); + } + else + { + var.SetTypedPointer(var.value.u64v[0], ResourceId(), OpaquePointerTypeID); + enablePointerFlags(var, PointerFlags::DereferencedPhysical); + setMatrixStride(var, matrixStride); + setBufferTypeId(var, type.InnerType()); + } + } } }; @@ -2374,13 +2515,18 @@ ShaderVariable Debugger::ReadFromPointer(const ShaderVariable &ptr) const return ret; } + // this is the case of 'reading' from a pointer where the data is entirely contained within the + // inner pointed variable. Either opaque sampler/image etc which is just the binding, or a + // cbuffer pointer which was already evaluated + const ShaderVariable *inner = getPointer(ptr); + ret = *inner; ret.name = ptr.name; if(inner->type == VarType::ReadOnlyResource || inner->type == VarType::ReadWriteResource || inner->type == VarType::Sampler) { - BindpointIndex bind = ret.GetBinding(); + bind = ret.GetBinding(); ret.SetBinding(bind.bindset, bind.bind, getBindArrayIndex(ptr)); } @@ -2441,14 +2587,18 @@ Id Debugger::GetPointerBaseId(const ShaderVariable &ptr) const return getBaseId(ptr); } +uint32_t Debugger::GetPointerArrayStride(const ShaderVariable &ptr) const +{ + RDCASSERT(ptr.type == VarType::GPUPointer); + return getArrayStride(ptr); +} + bool Debugger::IsOpaquePointer(const ShaderVariable &ptr) const { if(ptr.type != VarType::GPUPointer) return false; - PointerVal val = ptr.GetPointer(); - - if(val.pointerTypeID != OpaquePointerTypeID) + if(IsPhysicalPointer(ptr)) return false; const ShaderVariable *inner = getPointer(ptr); @@ -2456,6 +2606,20 @@ bool Debugger::IsOpaquePointer(const ShaderVariable &ptr) const inner->type == VarType::ReadWriteResource; } +bool Debugger::IsPhysicalPointer(const ShaderVariable &ptr) const +{ + if(ptr.type == VarType::GPUPointer) + { + // non-dereferenced physical pointer + if(ptr.GetPointer().pointerTypeID != OpaquePointerTypeID) + return true; + // dereferenced physical pointer + if(checkPointerFlags(ptr, PointerFlags::DereferencedPhysical)) + return true; + } + return false; +} + bool Debugger::ArePointersAndEqual(const ShaderVariable &a, const ShaderVariable &b) const { // we can do a pointer comparison by checking the values, since we store all pointer-related @@ -2468,25 +2632,78 @@ bool Debugger::ArePointersAndEqual(const ShaderVariable &a, const ShaderVariable void Debugger::WriteThroughPointer(ShaderVariable &ptr, const ShaderVariable &val) { - ShaderVariable *storage = getPointer(ptr); + // values for setting up pointer reads, either from a physical pointer or from an opaque pointer + rdcspv::Id typeId; + Decorations parentDecorations; + uint64_t baseAddress; + BindpointIndex bind; + uint64_t byteOffset = 0; + std::function pointerWriteCallback; - if(storage->type == VarType::ReadWriteResource) + if(IsPhysicalPointer(ptr)) { - rdcspv::Id typeId = getBufferTypeId(ptr); - uint64_t byteOffset = getByteOffset(ptr); + if(checkPointerFlags(ptr, PointerFlags::DereferencedPhysical)) + typeId = getBufferTypeId(ptr); + else + typeId = pointerTypeToId[ptr.GetPointer().pointerTypeID]; - BindpointIndex bind = storage->GetBinding(); + RDCASSERT(typeId != rdcspv::Id()); + parentDecorations = decorations[typeId]; + uint32_t varMatrixStride = getMatrixStride(ptr); + if(varMatrixStride != 0) + { + if(checkPointerFlags(ptr, PointerFlags::RowMajorMatrix)) + parentDecorations.flags = Decorations::RowMajor; + else + parentDecorations.flags = Decorations::ColMajor; + parentDecorations.flags = + Decorations::Flags(parentDecorations.flags | Decorations::HasMatrixStride); + parentDecorations.matrixStride = varMatrixStride; + } + baseAddress = ptr.GetPointer().pointer; + pointerWriteCallback = [this, baseAddress](uint64_t offset, uint64_t size, const void *src) { + apiWrapper->WriteAddress(baseAddress + offset, size, src); + }; + } + else + { + const ShaderVariable *inner = getPointer(ptr); + if(inner->type == VarType::ReadWriteResource && checkPointerFlags(*inner, PointerFlags::SSBO)) + { + typeId = getBufferTypeId(ptr); + byteOffset = getByteOffset(ptr); + bind = inner->GetBinding(); + bind.arrayIndex = getBindArrayIndex(ptr); + uint32_t varMatrixStride = getMatrixStride(ptr); + if(varMatrixStride != 0) + { + if(checkPointerFlags(ptr, PointerFlags::RowMajorMatrix)) + parentDecorations.flags = Decorations::RowMajor; + else + parentDecorations.flags = Decorations::ColMajor; + parentDecorations.flags = + Decorations::Flags(parentDecorations.flags | Decorations::HasMatrixStride); + parentDecorations.matrixStride = varMatrixStride; + } + pointerWriteCallback = [this, bind](uint64_t offset, uint64_t size, const void *src) { + apiWrapper->WriteBufferValue(bind, offset, size, src); + }; + } + } - bind.arrayIndex = getBindArrayIndex(ptr); - uint32_t matrixStride = getMatrixStride(ptr); - bool rowMajor = checkPointerFlags(ptr, PointerFlags::RowMajorMatrix); - - auto writeCallback = [this, bind, matrixStride, rowMajor]( - const ShaderVariable &var, const Decorations &, const DataType &type, - uint64_t offset, const rdcstr &) { + if(pointerWriteCallback) + { + auto writeCallback = [pointerWriteCallback](const ShaderVariable &var, const Decorations &dec, + const DataType &type, uint64_t offset, + const rdcstr &) { + // ignore any callbacks we get on the way up for structs/arrays, we don't need it we only + // read or write at primitive level if(!var.members.empty()) return; + bool rowMajor = (dec.flags & Decorations::RowMajor) != 0; + uint32_t matrixStride = dec.matrixStride; + if(type.type == DataType::MatrixType) { RDCASSERT(matrixStride != 0); @@ -2495,9 +2712,8 @@ void Debugger::WriteThroughPointer(ShaderVariable &ptr, const ShaderVariable &va { for(uint8_t r = 0; r < var.rows; r++) { - apiWrapper->WriteBufferValue(bind, offset + r * matrixStride, - VarTypeByteSize(var.type) * var.columns, - VarElemPointer(var, r * var.columns)); + pointerWriteCallback(offset + r * matrixStride, VarTypeByteSize(var.type) * var.columns, + VarElemPointer(var, r * var.columns)); } } else @@ -2510,12 +2726,11 @@ void Debugger::WriteThroughPointer(ShaderVariable &ptr, const ShaderVariable &va for(uint8_t c = 0; c < var.columns; c++) copyComp(tmp, c * var.rows + r, var, r * var.columns + c); - // read column-wise + // write column-wise for(uint8_t c = 0; c < var.columns; c++) { - apiWrapper->WriteBufferValue(bind, offset + c * matrixStride, - VarTypeByteSize(var.type) * var.rows, - VarElemPointer(tmp, c * var.rows)); + pointerWriteCallback(offset + c * matrixStride, VarTypeByteSize(var.type) * var.rows, + VarElemPointer(tmp, c * var.rows)); } } } @@ -2524,28 +2739,30 @@ void Debugger::WriteThroughPointer(ShaderVariable &ptr, const ShaderVariable &va if(!rowMajor) { // we can write a vector at a time if the matrix is column major - apiWrapper->WriteBufferValue(bind, offset, VarTypeByteSize(var.type) * var.columns, - VarElemPointer(var, 0)); + pointerWriteCallback(offset, VarTypeByteSize(var.type) * var.columns, + VarElemPointer(var, 0)); } else { for(uint8_t c = 0; c < var.columns; c++) - apiWrapper->WriteBufferValue(bind, offset + c * matrixStride, VarTypeByteSize(var.type), - VarElemPointer(var, c)); + pointerWriteCallback(offset + c * matrixStride, VarTypeByteSize(var.type), + VarElemPointer(var, c)); } } - else if(type.type == DataType::ScalarType) + else if(type.type == DataType::ScalarType || type.type == DataType::PointerType) { - apiWrapper->WriteBufferValue(bind, offset, VarTypeByteSize(var.type), VarElemPointer(var, 0)); + pointerWriteCallback(offset, VarTypeByteSize(var.type), VarElemPointer(var, 0)); } }; - WalkVariable(Decorations(), dataTypes[typeId], byteOffset, val, + WalkVariable(parentDecorations, dataTypes[typeId], byteOffset, val, rdcstr(), writeCallback); return; } + ShaderVariable *storage = getPointer(ptr); + // we don't support pointers to scalars since our 'unit' of pointer is a ShaderVariable, so check // if we have scalar indices to apply: uint8_t scalar0 = 0, scalar1 = 0; @@ -2838,6 +3055,18 @@ uint32_t Debugger::WalkVariable( break; } case DataType::PointerType: + { + RDCASSERT((dataTypes[type.id].pointerType.storage == StorageClass::PhysicalStorageBuffer) || + (dataTypes[type.id].pointerType.storage == StorageClass::PhysicalStorageBufferEXT)); + if(outVar) + { + outVar->type = VarType::GPUPointer; + outVar->rows = 1; + outVar->columns = 1; + } + numLocations = 1; + break; + } case DataType::ImageType: case DataType::SamplerType: case DataType::SampledImageType: @@ -3082,6 +3311,19 @@ void Debugger::PostParse() { Processor::PostParse(); + // declare pointerTypes for all declared physical pointer types. This will match the reflection + for(auto it = dataTypes.begin(); it != dataTypes.end(); ++it) + { + if(it->second.type == DataType::PointerType && + it->second.pointerType.storage == rdcspv::StorageClass::PhysicalStorageBuffer) + { + idToPointerType.insert(std::make_pair(it->second.InnerType(), (uint16_t)idToPointerType.size())); + } + } + pointerTypeToId.resize(idToPointerType.size()); + for(auto it = idToPointerType.begin(); it != idToPointerType.end(); ++it) + pointerTypeToId[it->second] = it->first; + for(const MemberName &mem : memberNames) dataTypes[mem.id].children[mem.member].name = mem.name; diff --git a/renderdoc/driver/shaders/spirv/spirv_processor.cpp b/renderdoc/driver/shaders/spirv/spirv_processor.cpp index 64a816956..6a8d88f52 100644 --- a/renderdoc/driver/shaders/spirv/spirv_processor.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_processor.cpp @@ -215,6 +215,14 @@ void Decorations::Register(const DecorationAndParamData &decoration) { flags = Flags(flags | ColMajor); } + else if(decoration == Decoration::Restrict) + { + flags = Flags(flags | Restrict); + } + else if(decoration == Decoration::Aliased) + { + flags = Flags(flags | Aliased); + } else if(decoration == Decoration::Location) { RDCASSERT(!(flags & HasArrayStride)); diff --git a/renderdoc/driver/shaders/spirv/spirv_processor.h b/renderdoc/driver/shaders/spirv/spirv_processor.h index a814087eb..6f9bb7042 100644 --- a/renderdoc/driver/shaders/spirv/spirv_processor.h +++ b/renderdoc/driver/shaders/spirv/spirv_processor.h @@ -272,6 +272,8 @@ struct Decorations BufferBlock = 0x2, RowMajor = 0x4, ColMajor = 0x8, + Restrict = 0x10, + Aliased = 0x20, // which packed decorations have been set HasLocation = 0x01000000, diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp index 06167d5c6..ec22d8b3b 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp @@ -1125,12 +1125,24 @@ void Reflector::MakeReflection(const GraphicsAPI sourceAPI, const ShaderStage st // specialisation constant gathering ConstantBlock specblock; + // declare pointerTypes for all declared physical pointer types first. This allows the debugger + // to easily match pointer types itself + for(auto it = dataTypes.begin(); it != dataTypes.end(); ++it) + { + if(it->second.type == DataType::PointerType && + it->second.pointerType.storage == rdcspv::StorageClass::PhysicalStorageBuffer) + { + pointerTypes.insert(std::make_pair(it->second.InnerType(), (uint16_t)pointerTypes.size())); + } + } + for(const Variable &global : globals) { if(global.storage == StorageClass::Input || global.storage == StorageClass::Output) { // variable type must be a pointer of the same storage class RDCASSERT(dataTypes[global.type].type == DataType::PointerType); + RDCASSERT(dataTypes[global.type].pointerType.storage == global.storage); const DataType &baseType = dataTypes[dataTypes[global.type].InnerType()]; const bool isInput = (global.storage == StorageClass::Input); diff --git a/renderdoc/driver/vulkan/vk_info.h b/renderdoc/driver/vulkan/vk_info.h index dc2efb416..604c89806 100644 --- a/renderdoc/driver/vulkan/vk_info.h +++ b/renderdoc/driver/vulkan/vk_info.h @@ -626,6 +626,7 @@ struct VulkanCreationInfo VkMemoryRequirements mrq; }; std::unordered_map m_Buffer; + rdcflatmap m_BufferAddresses; struct BufferView { diff --git a/renderdoc/driver/vulkan/vk_shaderdebug.cpp b/renderdoc/driver/vulkan/vk_shaderdebug.cpp index ff37ae3e5..f39deea52 100644 --- a/renderdoc/driver/vulkan/vk_shaderdebug.cpp +++ b/renderdoc/driver/vulkan/vk_shaderdebug.cpp @@ -151,8 +151,11 @@ class VulkanAPIWrapper : public rdcspv::DebugAPIWrapper public: VulkanAPIWrapper(WrappedVulkan *vk, VulkanCreationInfo &creation, VkShaderStageFlagBits stage, - uint32_t eid) - : m_DebugData(vk->GetReplay()->GetShaderDebugData()), m_Creation(creation), m_EventID(eid) + uint32_t eid, ResourceId shadId) + : m_DebugData(vk->GetReplay()->GetShaderDebugData()), + m_Creation(creation), + m_EventID(eid), + m_ShaderID(shadId) { m_pDriver = vk; @@ -355,6 +358,8 @@ public: m_pDriver->AddDebugMessage(c, sv, src, d); } + virtual ResourceId GetShaderID() override { return m_ShaderID; } + virtual uint64_t GetBufferLength(BindpointIndex bind) override { return PopulateBuffer(bind).size(); @@ -378,6 +383,22 @@ public: memcpy(data.data() + (size_t)offset, src, (size_t)byteSize); } + virtual void ReadAddress(uint64_t address, uint64_t byteSize, void *dst) override + { + size_t offset; + const bytebuf &data = PopulateBuffer(address, offset); + if(offset + byteSize <= data.size()) + memcpy(dst, data.data() + offset, (size_t)byteSize); + } + + virtual void WriteAddress(uint64_t address, uint64_t byteSize, const void *src) override + { + size_t offset; + bytebuf &data = PopulateBuffer(address, offset); + if(offset + byteSize <= data.size()) + memcpy(data.data() + offset, src, (size_t)byteSize); + } + virtual bool ReadTexel(BindpointIndex imageBind, const ShaderVariable &coord, uint32_t sample, ShaderVariable &output) override { @@ -1553,6 +1574,7 @@ private: bool m_ResourcesDirty = false; uint32_t m_EventID; + ResourceId m_ShaderID; std::map m_SampleViews; @@ -1649,6 +1671,69 @@ private: return elemData[index.arrayIndex]; } + bytebuf &PopulateBuffer(uint64_t address, size_t &offs) + { + // pick a non-overlapping bind namespace for direct pointer access + BindpointIndex bind = pointerBind; + uint64_t base; + uint64_t end; + ResourceId id; + bool valid = false; + if(m_Creation.m_BufferAddresses.empty()) + { + bind.arrayIndex = 0; + auto insertIt = bufferCache.insert(std::make_pair(bind, bytebuf())); + m_pDriver->AddDebugMessage( + MessageCategory::Execution, MessageSeverity::High, MessageSource::RuntimeWarning, + StringFormat::Fmt("pointer access detected but no address-capable buffers allocated.")); + return insertIt.first->second; + } + else + { + auto it = m_Creation.m_BufferAddresses.lower_bound(address); + // lower_bound puts us at the same or next item. Since we want the buffer that contains + // this address, we go to the previous iter unless we're already on the first or + // it's an exact match + if(address != it->first && it != m_Creation.m_BufferAddresses.begin()) + it--; + // use the index in the map as a unique buffer identifier that's not 64-bit + bind.arrayIndex = uint32_t(it - m_Creation.m_BufferAddresses.begin()); + { + base = it->first; + id = it->second; + end = base + m_Creation.m_Buffer[id].size; + if(base <= address && address < end) + { + offs = (size_t)(address - base); + valid = true; + } + } + } + if(!valid) + { + m_pDriver->AddDebugMessage( + MessageCategory::Execution, MessageSeverity::High, MessageSource::RuntimeWarning, + StringFormat::Fmt("out of bounds pointer access of address %#18llx detected.Closest " + "buffer is address range %#18llx -> %#18llx (%s)", + address, base, end, ToStr(id).c_str())); + } + auto insertIt = bufferCache.insert(std::make_pair(bind, bytebuf())); + bytebuf &data = insertIt.first->second; + if(insertIt.second && valid) + { + // if the resources might be dirty from side-effects from the action, replay back to right + // before it. + if(m_ResourcesDirty) + { + VkMarkerRegion region("un-dirtying resources"); + m_pDriver->ReplayLog(0, m_EventID, eReplay_WithoutDraw); + m_ResourcesDirty = false; + } + m_pDriver->GetDebugManager()->GetBufferData(id, 0, 0, data); + } + return data; + } + bytebuf &PopulateBuffer(BindpointIndex bind) { auto insertIt = bufferCache.insert(std::make_pair(bind, bytebuf())); @@ -3899,8 +3984,8 @@ ShaderDebugTrace *VulkanReplay::DebugVertex(uint32_t eventId, uint32_t vertid, u shadRefl.PopulateDisassembly(shader.spirv); - VulkanAPIWrapper *apiWrapper = - new VulkanAPIWrapper(m_pDriver, c, VK_SHADER_STAGE_VERTEX_BIT, eventId); + VulkanAPIWrapper *apiWrapper = new VulkanAPIWrapper(m_pDriver, c, VK_SHADER_STAGE_VERTEX_BIT, + eventId, shadRefl.refl->resourceId); // clamp the view index to the number of multiviews, just to be sure size_t numViews; @@ -4152,8 +4237,8 @@ ShaderDebugTrace *VulkanReplay::DebugPixel(uint32_t eventId, uint32_t x, uint32_ shadRefl.PopulateDisassembly(shader.spirv); - VulkanAPIWrapper *apiWrapper = - new VulkanAPIWrapper(m_pDriver, c, VK_SHADER_STAGE_FRAGMENT_BIT, eventId); + VulkanAPIWrapper *apiWrapper = new VulkanAPIWrapper(m_pDriver, c, VK_SHADER_STAGE_FRAGMENT_BIT, + eventId, shadRefl.refl->resourceId); std::map &builtins = apiWrapper->builtin_inputs; builtins[ShaderBuiltin::DeviceIndex] = ShaderVariable(rdcstr(), 0U, 0U, 0U, 0U); @@ -4846,8 +4931,8 @@ ShaderDebugTrace *VulkanReplay::DebugThread(uint32_t eventId, shadRefl.PopulateDisassembly(shader.spirv); - VulkanAPIWrapper *apiWrapper = - new VulkanAPIWrapper(m_pDriver, c, VK_SHADER_STAGE_COMPUTE_BIT, eventId); + VulkanAPIWrapper *apiWrapper = new VulkanAPIWrapper(m_pDriver, c, VK_SHADER_STAGE_COMPUTE_BIT, + eventId, shadRefl.refl->resourceId); uint32_t threadDim[3]; threadDim[0] = shadRefl.refl->dispatchThreadsDimension[0]; diff --git a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp index 3c3162546..544b13578 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp @@ -1430,6 +1430,7 @@ bool WrappedVulkan::Serialise_vkBindBufferMemory(SerialiserType &ser, VkDevice d bufInfo.gpuAddress = ObjDisp(device)->GetBufferDeviceAddress(Unwrap(device), &getInfo); else if(GetExtensions(GetRecord(device)).ext_EXT_buffer_device_address) bufInfo.gpuAddress = ObjDisp(device)->GetBufferDeviceAddressEXT(Unwrap(device), &getInfo); + m_CreationInfo.m_BufferAddresses[bufInfo.gpuAddress] = GetResID(buffer); } m_CreationInfo.m_Memory[GetResID(memory)].BindMemory(memoryOffset, mrq.size, @@ -2840,6 +2841,7 @@ bool WrappedVulkan::Serialise_vkBindBufferMemory2(SerialiserType &ser, VkDevice bufInfo.gpuAddress = ObjDisp(device)->GetBufferDeviceAddress(Unwrap(device), &getInfo); else if(GetExtensions(GetRecord(device)).ext_EXT_buffer_device_address) bufInfo.gpuAddress = ObjDisp(device)->GetBufferDeviceAddressEXT(Unwrap(device), &getInfo); + m_CreationInfo.m_BufferAddresses[bufInfo.gpuAddress] = GetResID(bindInfo.buffer); } // the memory is immediately dirty because we don't use dirty tracking, it's too expensive to