Skip to content

Commit

Permalink
Move debug counter out of harm's way (microsoft#2887)
Browse files Browse the repository at this point in the history
Previously, the counter-of-instrumentation-bytes-written for the shader debug pass was at offset zero into the UAV. This presented problems when overflow/wraparound occurred. This change moves the counter up into the unused overflow space in the UAV. The counter for the mesh shader output pass is moved to the same place for simplicity.
PIX bug # 26371771.
  • Loading branch information
jeffnn authored May 19, 2020
1 parent 64b6772 commit 36ecb43
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 13 deletions.
25 changes: 16 additions & 9 deletions lib/DxilPIXPasses/DxilDebugInstrumentation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,13 @@ using namespace hlsl;
// caller will than allocate a UAV that is twice the size and try again, up to a
// predefined maximum.

// Keep this in sync with the same-named value in the debugger application's
// Keep these in sync with the same-named value in the debugger application's
// WinPixShaderUtils.h

constexpr uint64_t DebugBufferDumpingGroundSize = 64 * 1024;
// The actual max size per record is much smaller than this, but it never
// hurts to be generous.
constexpr size_t CounterOffsetBeyondUsefulData = DebugBufferDumpingGroundSize / 2;

// These definitions echo those in the debugger application's
// debugshaderrecord.h file
Expand Down Expand Up @@ -222,6 +226,8 @@ class DxilDebugInstrumentation : public ModulePass {

Constant *m_OffsetMask = nullptr;

Constant *m_CounterOffset = nullptr;

struct BuilderContext {
Module &M;
DxilModule &DM;
Expand Down Expand Up @@ -623,6 +629,8 @@ void DxilDebugInstrumentation::addInvocationSelectionProlog(
InverseOffsetMultiplicand, "OffsetAddend");
m_OffsetMask = BC.HlslOP->GetU32Const(UAVDumpingGroundOffset() - 1);

m_CounterOffset = BC.HlslOP->GetU32Const(UAVDumpingGroundOffset() + CounterOffsetBeyondUsefulData);

m_SelectionCriterion = ParameterTestResult;
}

Expand All @@ -640,7 +648,6 @@ void DxilDebugInstrumentation::reserveDebugEntrySpace(BuilderContext &BC,
BC.HlslOP->GetU32Const((unsigned)OP::OpCode::AtomicBinOp);
Constant *AtomicAdd =
BC.HlslOP->GetU32Const((unsigned)DXIL::AtomicBinOpCode::Add);
Constant *Zero32Arg = BC.HlslOP->GetU32Const(0);
UndefValue *UndefArg = UndefValue::get(Type::getInt32Ty(BC.Ctx));

// so inc will be zero for uninteresting invocations:
Expand All @@ -651,13 +658,13 @@ void DxilDebugInstrumentation::reserveDebugEntrySpace(BuilderContext &BC,
auto PreviousValue = BC.Builder.CreateCall(
AtomicOpFunc,
{
AtomicBinOpcode, // i32, ; opcode
m_HandleForUAV, // %dx.types.Handle, ; resource handle
AtomicAdd, // i32, ; binary operation code : EXCHANGE, IADD, AND, OR,
// XOR, IMIN, IMAX, UMIN, UMAX
Zero32Arg, // i32, ; coordinate c0: index in bytes
UndefArg, // i32, ; coordinate c1 (unused)
UndefArg, // i32, ; coordinate c2 (unused)
AtomicBinOpcode, // i32, ; opcode
m_HandleForUAV, // %dx.types.Handle, ; resource handle
AtomicAdd, // i32, ; binary operation code : EXCHANGE, IADD, AND, OR,
// XOR, IMIN, IMAX, UMIN, UMAX
m_CounterOffset, // i32, ; coordinate c0: index in bytes
UndefArg, // i32, ; coordinate c1 (unused)
UndefArg, // i32, ; coordinate c2 (unused)
IncrementForThisInvocation, // i32); increment value
},
"UAVIncResult");
Expand Down
11 changes: 7 additions & 4 deletions lib/DxilPIXPasses/DxilPIXMeshShaderOutputInstrumentation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,13 @@
#include <winerror.h>
#endif

// Keep this in sync with the same-named value in the debugger application's
// Keep these in sync with the same-named value in the debugger application's
// WinPixShaderUtils.h

constexpr uint64_t DebugBufferDumpingGroundSize = 64 * 1024;
constexpr uint64_t MaxSizePerRecord = 64;
// The actual max size per record is much smaller than this, but it never
// hurts to be generous.
constexpr size_t CounterOffsetBeyondUsefulData = DebugBufferDumpingGroundSize / 2;

// Keep these in sync with the same-named values in PIX's MeshShaderOutput.cpp
constexpr uint32_t triangleIndexIndicator = 1;
Expand Down Expand Up @@ -177,7 +180,7 @@ Value *DxilPIXMeshShaderOutputInstrumentation::reserveDebugEntrySpace(

// Check that the caller didn't ask for so much memory that it will
// overwrite the offset counter:
assert(m_RemainingReservedSpaceInBytes < MaxSizePerRecord);
assert(m_RemainingReservedSpaceInBytes < CounterOffsetBeyondUsefulData);

m_RemainingReservedSpaceInBytes = SpaceInBytes;

Expand All @@ -189,7 +192,7 @@ Value *DxilPIXMeshShaderOutputInstrumentation::reserveDebugEntrySpace(
Constant *AtomicAdd =
BC.HlslOP->GetU32Const((unsigned)DXIL::AtomicBinOpCode::Add);
Constant *OffsetArg =
BC.HlslOP->GetU32Const(UAVDumpingGroundOffset() + MaxSizePerRecord);
BC.HlslOP->GetU32Const(UAVDumpingGroundOffset() + CounterOffsetBeyondUsefulData);
UndefValue *UndefArg = UndefValue::get(Type::getInt32Ty(BC.Ctx));

Constant *Increment = BC.HlslOP->GetU32Const(SpaceInBytes);
Expand Down

0 comments on commit 36ecb43

Please sign in to comment.