From 668fa75660b09e9ebaeff1ee01da782fcb354f78 Mon Sep 17 00:00:00 2001 From: Jake Turner Date: Wed, 17 Sep 2025 10:08:00 +0100 Subject: [PATCH] Change some asserts to be development only in Spirv Debugger Asserts used during development of multi-threading and related to the consistency of expected state --- .../driver/shaders/spirv/spirv_debug.cpp | 25 ++++++----- renderdoc/driver/shaders/spirv/spirv_debug.h | 28 +++++++++--- .../shaders/spirv/spirv_debug_setup.cpp | 44 +++++++++---------- 3 files changed, 58 insertions(+), 39 deletions(-) diff --git a/renderdoc/driver/shaders/spirv/spirv_debug.cpp b/renderdoc/driver/shaders/spirv/spirv_debug.cpp index 0186f708c..919d30b9d 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug.cpp @@ -398,12 +398,12 @@ DeviceOpResult ThreadState::WritePointerValue(Id pointer, const ShaderVariable & if(id != ptrid && live.contains(id)) { opResult = debugger.GetPointerValue(ids[id], changes[i].before); - RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); + SPIRV_DEBUG_RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); } } opResult = debugger.WriteThroughPointer(var, val); - RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); + SPIRV_DEBUG_RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); // now evaluate the value after for(size_t i = 0; i < pointers.size(); i++) @@ -412,7 +412,7 @@ DeviceOpResult ThreadState::WritePointerValue(Id pointer, const ShaderVariable & if(id != ptrid && live.contains(id)) { opResult = debugger.GetPointerValue(ids[id], changes[i].after); - RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); + SPIRV_DEBUG_RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); } } @@ -421,7 +421,7 @@ DeviceOpResult ThreadState::WritePointerValue(Id pointer, const ShaderVariable & if(gsmPtrIt != gsmPointers.end()) { opResult = debugger.WriteThroughPointer(gsmPtrIt->second, val); - RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); + SPIRV_DEBUG_RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); } // if the pointer we're writing is one of the aliased pointers, be sure we add it even if @@ -447,7 +447,7 @@ DeviceOpResult ThreadState::WritePointerValue(Id pointer, const ShaderVariable & // always add a change for the base storage variable written itself, even if that's a no-op. // This one is not included in any of the pointers lists above opResult = debugger.GetPointerValue(ids[ptrid], basechange.after); - RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); + SPIRV_DEBUG_RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); // if this is the first local write, mark this variable as becoming alive here, instead of at // its declaration @@ -540,7 +540,7 @@ void ThreadState::SetDst(Id id, const ShaderVariable &val) { // The variable was live and written to, it should be cached opResult = debugger.GetPointerValue(prev, change.before); - RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); + SPIRV_DEBUG_RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); } change.after = afterVal; pendingDebugState.changes.push_back(change); @@ -567,7 +567,8 @@ void ThreadState::ProcessScopeChange(const rdcarray &oldLive, const rdcarray if(liveGlobals.contains(id)) continue; - RDCASSERTEQUAL(debugger.GetPointerValue(ids[id], val), DeviceOpResult::Succeeded); + DeviceOpResult opResult = debugger.GetPointerValue(ids[id], val); + SPIRV_DEBUG_RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); pendingDebugState.changes.push_back({val}); if(ids[id].type == VarType::GPUPointer && !debugger.IsOpaquePointer(ids[id]) && @@ -584,7 +585,8 @@ void ThreadState::ProcessScopeChange(const rdcarray &oldLive, const rdcarray if(liveGlobals.contains(id)) continue; - RDCASSERTEQUAL(debugger.GetPointerValue(ids[id], val), DeviceOpResult::Succeeded); + DeviceOpResult opResult = debugger.GetPointerValue(ids[id], val); + SPIRV_DEBUG_RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); pendingDebugState.changes.push_back({ShaderVariable(), val}); } } @@ -1171,7 +1173,8 @@ void ThreadState::StepNext(ShaderDebugState *state, const rdcarray // then evaluate it, to get the extracted value ShaderVariable val; - RDCASSERTEQUAL(debugger.ReadFromPointer(ptr, val), DeviceOpResult::Succeeded); + DeviceOpResult opResult = debugger.ReadFromPointer(ptr, val); + SPIRV_DEBUG_RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); SetDst(extract.result, val); break; @@ -5327,7 +5330,7 @@ void ThreadState::ExecuteMemoryBarrier(Id semanticsId) void ThreadState::QueueMathOp(GLSLstd450 op, const rdcarray ¶mVars, const ShaderVariable &result) { - RDCASSERT(!IsPendingResultPending()); + SPIRV_DEBUG_RDCASSERT(!IsPendingResultPending()); pendingResultData = result; queuedGpuMathOp.workgroupIndex = workgroupIndex; queuedGpuMathOp.op = op; @@ -5344,7 +5347,7 @@ void ThreadState::QueueSampleGather(Op opcode, DebugAPIWrapper::TextureType texT const ImageOperandsAndParamDatas &operands, const ShaderVariable &result) { - RDCASSERT(!IsPendingResultPending()); + SPIRV_DEBUG_RDCASSERT(!IsPendingResultPending()); pendingResultData = result; queuedGpuSampleGatherOp.workgroupIndex = workgroupIndex; queuedGpuSampleGatherOp.opcode = opcode; diff --git a/renderdoc/driver/shaders/spirv/spirv_debug.h b/renderdoc/driver/shaders/spirv/spirv_debug.h index 2de5430fd..4bb0f1360 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug.h +++ b/renderdoc/driver/shaders/spirv/spirv_debug.h @@ -30,6 +30,22 @@ #include "spirv_common.h" #include "spirv_processor.h" +#if defined(RELEASE) +#define SPIRV_DEBUG_RDCASSERT(...) \ + do \ + { \ + (void)(__VA_ARGS__); \ + } while((void)0, 0) +#define SPIRV_DEBUG_RDCASSERTEQUAL(...) \ + do \ + { \ + (void)(__VA_ARGS__); \ + } while((void)0, 0) +#else +#define SPIRV_DEBUG_RDCASSERT(...) RDCASSERTMSG("", __VA_ARGS__) +#define SPIRV_DEBUG_RDCASSERTEQUAL(a, b) RDCASSERTEQUAL(a, b) +#endif + struct SPIRVInterfaceAccess; struct SPIRVPatchData; @@ -370,12 +386,12 @@ struct ThreadState void SetPendingResultUnknown() { SetPendingResultStatus(PendingResultStatus::Unknown); } void SetPendingResultReady() { - RDCASSERTEQUAL(GetPendingResultStatus(), PendingResultStatus::Pending); + SPIRV_DEBUG_RDCASSERTEQUAL(GetPendingResultStatus(), PendingResultStatus::Pending); SetPendingResultStatus(PendingResultStatus::Ready); } const ShaderVariable &GetPendingResult() const { - RDCASSERTEQUAL(GetPendingResultStatus(), PendingResultStatus::Ready); + SPIRV_DEBUG_RDCASSERTEQUAL(GetPendingResultStatus(), PendingResultStatus::Ready); return pendingResultData; } void SetStepQueued() @@ -408,14 +424,14 @@ struct ThreadState bool StepNeedsDeviceThread() const { return (AtomicLoad(&atomic_stepNeedsDeviceThread) == 1); } const GpuMathOperation &GetQueuedGpuMathOp() const { - RDCASSERT(AtomicLoad(&atomic_stepNeedsGpuMathOp)); - RDCASSERT(IsPendingResultPending()); + SPIRV_DEBUG_RDCASSERT(AtomicLoad(&atomic_stepNeedsGpuMathOp)); + SPIRV_DEBUG_RDCASSERT(IsPendingResultPending()); return queuedGpuMathOp; } const GpuSampleGatherOperation &GetQueuedGpuSampleGatherOp() const { - RDCASSERT(AtomicLoad(&atomic_stepNeedsGpuSampleGatherOp)); - RDCASSERT(IsPendingResultPending()); + SPIRV_DEBUG_RDCASSERT(AtomicLoad(&atomic_stepNeedsGpuSampleGatherOp)); + SPIRV_DEBUG_RDCASSERT(IsPendingResultPending()); return queuedGpuSampleGatherOp; } diff --git a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp index ef5df334e..55e14309a 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp @@ -4559,8 +4559,8 @@ void Debugger::RegisterOp(Iter it) void Debugger::QueueGpuMathOp(uint32_t lane) { ThreadState &thread = workgroup[lane]; - RDCASSERT(thread.IsSimulationStepActive()); - RDCASSERT(!queuedGpuMathOps[lane]); + SPIRV_DEBUG_RDCASSERT(thread.IsSimulationStepActive()); + SPIRV_DEBUG_RDCASSERT(!queuedGpuMathOps[lane]); queuedGpuMathOps[lane] = true; } @@ -4568,8 +4568,8 @@ void Debugger::QueueGpuMathOp(uint32_t lane) void Debugger::QueueGpuSampleGatherOp(uint32_t lane) { ThreadState &thread = workgroup[lane]; - RDCASSERT(thread.IsSimulationStepActive()); - RDCASSERT(!queuedGpuSampleGatherOps[lane]); + SPIRV_DEBUG_RDCASSERT(thread.IsSimulationStepActive()); + SPIRV_DEBUG_RDCASSERT(!queuedGpuSampleGatherOps[lane]); queuedGpuSampleGatherOps[lane] = true; } @@ -4623,7 +4623,7 @@ void Debugger::ProcessQueuedGpuMathOps() memset(&result.value, 0, sizeof(result.value)); } - RDCASSERT(!pendingLanes[workgroupIndex]); + SPIRV_DEBUG_RDCASSERT(!pendingLanes[workgroupIndex]); pendingLanes[workgroupIndex] = true; } } @@ -4660,7 +4660,7 @@ void Debugger::ProcessQueuedGpuSampleGatherOps() if(!hasResult) pendingGpuSampleGatherOpsResults.push_back(sampleGatherOp.result); - RDCASSERT(!pendingLanes[workgroupIndex]); + SPIRV_DEBUG_RDCASSERT(!pendingLanes[workgroupIndex]); pendingLanes[workgroupIndex] = true; } } @@ -4709,7 +4709,7 @@ void Debugger::StepThread(uint32_t lane, StepThreadMode stepMode) ThreadState &thread = workgroup[lane]; bool isActiveThread = lane == activeLaneIndex; bool simulateStep = true; - RDCASSERT(thread.IsSimulationStepActive()); + SPIRV_DEBUG_RDCASSERT(thread.IsSimulationStepActive()); int curActiveSteps = isActiveThread ? steps : 0; while(simulateStep) @@ -4738,7 +4738,7 @@ void Debugger::StepThread(uint32_t lane, StepThreadMode stepMode) simulateStep = thread.CanRunAnotherStep(); if(simulateStep) { - RDCASSERT(thread.IsSimulationStepActive()); + SPIRV_DEBUG_RDCASSERT(thread.IsSimulationStepActive()); } if(simulateStep) thread.SetStepQueued(); @@ -4750,35 +4750,35 @@ void Debugger::StepThread(uint32_t lane, StepThreadMode stepMode) if(isActiveThread) steps = curActiveSteps; - RDCASSERT(thread.IsSimulationStepActive()); + SPIRV_DEBUG_RDCASSERT(thread.IsSimulationStepActive()); // The queueing has to be when the thread is not being simulated if(thread.StepNeedsGpuSampleGatherOp()) { - RDCASSERT(!simulateStep); + SPIRV_DEBUG_RDCASSERT(!simulateStep); QueueGpuSampleGatherOp(lane); return; } if(thread.StepNeedsGpuMathOp()) { - RDCASSERT(!simulateStep); + SPIRV_DEBUG_RDCASSERT(!simulateStep); QueueGpuMathOp(lane); return; } if(thread.StepNeedsDeviceThread()) { - RDCASSERT(!simulateStep); + SPIRV_DEBUG_RDCASSERT(!simulateStep); QueueDeviceThreadStep(lane); return; } if(simulateStep) { - RDCASSERTEQUAL(stepMode, StepThreadMode::QUEUE_MULTIPLE_STEPS); + SPIRV_DEBUG_RDCASSERTEQUAL(stepMode, StepThreadMode::QUEUE_MULTIPLE_STEPS); QueueJob(lane); return; } - RDCASSERT(!thread.IsPendingResultPending()); + SPIRV_DEBUG_RDCASSERT(!thread.IsPendingResultPending()); thread.SetSimulationStepCompleted(); } @@ -4794,10 +4794,10 @@ void Debugger::InternalStepThread(uint32_t lane) if(retireIDs) { { - RDCASSERT(activeDebugState.callstack.empty()); - RDCASSERT(activeDebugState.changes.empty()); - RDCASSERT(activeDebugState.flags == ShaderEvents::NoEvent); - RDCASSERT(activeDebugState.nextInstruction == 0); + SPIRV_DEBUG_RDCASSERT(activeDebugState.callstack.empty()); + SPIRV_DEBUG_RDCASSERT(activeDebugState.changes.empty()); + SPIRV_DEBUG_RDCASSERT(activeDebugState.flags == ShaderEvents::NoEvent); + SPIRV_DEBUG_RDCASSERT(activeDebugState.nextInstruction == 0); } for(size_t l = 0; l < thread.live.size();) { @@ -4808,7 +4808,7 @@ void Debugger::InternalStepThread(uint32_t lane) ShaderVariableChange change; DeviceOpResult opResult = GetPointerValue(thread.ids[id], change.before); // The variable was live and written to, it should be cached - RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); + SPIRV_DEBUG_RDCASSERTEQUAL(opResult, DeviceOpResult::Succeeded); activeDebugState.changes.push_back(change); continue; } @@ -4960,9 +4960,9 @@ void Debugger::AddDebugMessage(MessageCategory c, MessageSeverity sv, MessageSou void Debugger::QueueDeviceThreadStep(uint32_t lane) { ThreadState &thread = workgroup[lane]; - RDCASSERT(thread.IsSimulationStepActive()); + SPIRV_DEBUG_RDCASSERT(thread.IsSimulationStepActive()); thread.SetStepQueued(); - RDCASSERT(!queuedDeviceThreadSteps[lane]); + SPIRV_DEBUG_RDCASSERT(!queuedDeviceThreadSteps[lane]); queuedDeviceThreadSteps[lane] = true; } @@ -4977,7 +4977,7 @@ void Debugger::ProcessQueuedDeviceThreadSteps() queuedDeviceThreadSteps[lane] = false; ThreadState &thread = workgroup[lane]; thread.SetPendingResultUnknown(); - RDCASSERT(thread.IsSimulationStepActive()); + SPIRV_DEBUG_RDCASSERT(thread.IsSimulationStepActive()); StepThread(lane, StepThreadMode::QUEUE_MULTIPLE_STEPS); } }