Skip to content

Commit 71afbcc

Browse files
authored
PIX: Fix recent regression in debug instrumentation of void instructions (microsoft#6053)
The misplacement of that "return" in DxilDebugInstrumentation.cpp meant that a thread would continue to the following call to addStepDebugEntryValue, even if pix_dxil::PixDxilReg::FromInst had failed (i.e. returned false), which means that RegNum is not valid (although initialized to 0). This meant that PIX was instrumenting a bunch of void-return DXIL instructions that it shouldn't have. Didn't think to test that it WASN'T instrumenting instructions, but herein is added a test to do just that.
1 parent 1b405f6 commit 71afbcc

File tree

2 files changed

+21
-5
lines changed

2 files changed

+21
-5
lines changed

lib/DxilPIXPasses/DxilDebugInstrumentation.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -894,13 +894,12 @@ void DxilDebugInstrumentation::addStepDebugEntry(BuilderContext &BC,
894894
}
895895

896896
std::uint32_t RegNum;
897-
if (!pix_dxil::PixDxilReg::FromInst(Inst, &RegNum))
898-
if (Inst->getOpcode() == Instruction::Ret) {
897+
if (!pix_dxil::PixDxilReg::FromInst(Inst, &RegNum)) {
898+
if (Inst->getOpcode() == Instruction::Ret)
899899
addStepEntryForType<void>(DebugShaderModifierRecordTypeDXILStepTerminator,
900900
BC, InstNum, nullptr, 0, 0);
901-
return;
902-
}
903-
901+
return;
902+
}
904903
addStepDebugEntryValue(BC, InstNum, Inst, RegNum, BC.Builder.getInt32(0));
905904
}
906905

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// RUN: %dxc -Tlib_6_6 %s | %opt -S -dxil-annotate-with-virtual-regs -hlsl-dxil-debug-instrumentation | %FileCheck %s
2+
3+
// Check that the instrumentation does NOT instrument an instruction that has no dxil-inst-num metadata
4+
// The load instruction should not be instrumented. If it is, we can expect an "atomicBinOp", emitted
5+
// by the instrumentation, to be generated before the handle value is used, so assert that there
6+
// is no such atomicBinOp:
7+
8+
// CHECK: [[HandleNum:%[0-9]+]] = load %dx.types.Handle,
9+
// CHECK-NOT: call i32 @dx.op.atomicBinOp.i32(i32 78
10+
// CHECK: @dx.op.createHandleForLib.dx.types.Handle(i32 160, %dx.types.Handle [[HandleNum]]
11+
12+
RWByteAddressBuffer buffer : register(u0);
13+
14+
[shader("raygeneration")]
15+
void main() {
16+
buffer.Store(0, 42);
17+
}

0 commit comments

Comments
 (0)