From 33f8bc8c961c075a05dd5df441ffb092c45a3187 Mon Sep 17 00:00:00 2001 From: Damyan Pepper Date: Tue, 25 Aug 2026 01:31:55 -0700 Subject: [PATCH] [PIX] Inline helper functions before shader debugging PIX maps one shader invocation to one record stream in the debug UAV, and one stream to exactly one function. A [noinline] helper instrumented as its own function looks like a second invocation of a thread that runs once. PIX discards those records, and you cannot step into the helper. The debug instrumentation always emits RawBufferStore. That operation is legal only from shader model 6.2, so shader models 6.0 and 6.1 get an invalid module. The pass creates the tools UAV before it knows whether there is anything to instrument. A library that contains only helpers therefore gains a UAV although the pass reports that it changed nothing. Inlining happens before the pass numbers instructions or creates shadow storage. Every prepass does it, so the debug, non-uniform-resource-index, and debug-break pipelines all see the same module shape. A library module is left alone, because each exported function is its own invocation. The runtime invokes the patch-constant function of a hull shader directly. That function therefore survives inlining, and the pass instruments it. The pass selects an invocation of it by primitive alone, because OutputControlPointID is valid only in the control point phase. A function that survives inlining is named in the pass report as UninlinedFunction:, so PIX does not offer a range with no records. The pass reports it and continues, instead of stopping the process. Assisted-by: Copilot Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ac42a8d8-c740-45a3-9a2d-7cfc39b92853 --- .../DxilAnnotateWithVirtualRegister.cpp | 14 + .../DxilDbgValueToDbgDeclare.cpp | 14 + .../DxilDebugInstrumentation.cpp | 146 +++-- lib/DxilPIXPasses/PixPassHelpers.cpp | 87 +++ lib/DxilPIXPasses/PixPassHelpers.h | 30 + ...gBreakInstrumentationInHelperFunction.hlsl | 34 ++ .../pix/DebugHullPatchConstantFunction.hlsl | 79 +++ .../pix/DebugNoInlineHelperFunction.hlsl | 60 ++ .../pix/DebugStoreOpcodeByShaderModel.hlsl | 21 + ...nUniformResourceIndexInHelperFunction.hlsl | 38 ++ tools/clang/unittests/HLSL/PixTest.cpp | 524 +++++++++++++++++- 11 files changed, 1003 insertions(+), 44 deletions(-) create mode 100644 tools/clang/test/HLSLFileCheck/pix/DebugBreakInstrumentationInHelperFunction.hlsl create mode 100644 tools/clang/test/HLSLFileCheck/pix/DebugHullPatchConstantFunction.hlsl create mode 100644 tools/clang/test/HLSLFileCheck/pix/DebugNoInlineHelperFunction.hlsl create mode 100644 tools/clang/test/HLSLFileCheck/pix/DebugStoreOpcodeByShaderModel.hlsl create mode 100644 tools/clang/test/HLSLFileCheck/pix/NonUniformResourceIndexInHelperFunction.hlsl diff --git a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp index 196da98c87..6d413c2cb6 100644 --- a/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp +++ b/lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp @@ -128,6 +128,12 @@ PrintableSubsetOfMangledFunctionName(llvm::StringRef mangled) { } bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { + // Inline first, so each ordinal this pass hands out belongs to a function + // that PIX can attribute to an invocation. + llvm::SmallVector UninlinedFunctions; + PIXPassHelpers::InlineNonEntryFunctions(M.GetOrCreateDxilModule(), + &UninlinedFunctions); + Init(M); if (m_DM == nullptr) { return false; @@ -218,6 +224,14 @@ bool DxilAnnotateWithVirtualRegister::runOnModule(llvm::Module &M) { } if (OSOverride != nullptr) { + // Name each function that survives inlining. Its instruction range is + // advertised above, but no trace record arrives for it, so PIX must not + // offer it as somewhere to step into. + for (llvm::Function *F : UninlinedFunctions) { + *OSOverride << "UninlinedFunction:" + << PrintableSubsetOfMangledFunctionName(F->getName()) << "\n"; + } + // Print a set of strings of the exemplary form "InstructionCount: // " if (m_DM->GetShaderModel()->GetKind() == hlsl::ShaderModel::Kind::Library) diff --git a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp index 15a7e6c666..17566217f7 100644 --- a/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp +++ b/lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp @@ -16,6 +16,7 @@ #include #include "dxc/DXIL/DxilConstants.h" +#include "dxc/DXIL/DxilMetadataHelper.h" #include "dxc/DXIL/DxilModule.h" #include "dxc/DXIL/DxilOperations.h" #include "dxc/DXIL/DxilResourceBase.h" @@ -593,6 +594,19 @@ GlobalStorageMap GatherGlobalEmbeddedArrayStorage(llvm::Module &M) { } bool DxilDbgValueToDbgDeclare::runOnModule(llvm::Module &M) { + // Inline before any shadow storage exists. The stores this pass emits carry + // no debug location on purpose, and llvm::InlineFunction stamps the call site + // location onto each inlined instruction that carries none. Inlining first + // therefore keeps a helper local readable, because its stores stay attributed + // to the helper instead of to the line of the call. + // + // This pass also runs over a plain LLVM module that carries debug info and no + // DXIL, which has no call graph to root the inlining on. + if (M.HasDxilModule() || + M.getNamedMetadata(hlsl::DxilMDHelper::kDxilVersionMDName) != nullptr) { + PIXPassHelpers::InlineNonEntryFunctions(M.GetOrCreateDxilModule()); + } + auto GlobalEmbeddedArrayStorage = GatherGlobalEmbeddedArrayStorage(M); bool Changed = false; diff --git a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp index 3e689ae877..27d3fb3014 100644 --- a/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp +++ b/lib/DxilPIXPasses/DxilDebugInstrumentation.cpp @@ -377,6 +377,28 @@ class DxilDebugInstrumentation : public ModulePass { CountBlockPayloadBytes(std::vector const &IsAndTs); }; +static bool IsInstrumentableShaderKind(DXIL::ShaderKind shaderKind) { + switch (shaderKind) { + case DXIL::ShaderKind::Amplification: + case DXIL::ShaderKind::Mesh: + case DXIL::ShaderKind::Vertex: + case DXIL::ShaderKind::Geometry: + case DXIL::ShaderKind::Pixel: + case DXIL::ShaderKind::Compute: + case DXIL::ShaderKind::RayGeneration: + case DXIL::ShaderKind::Hull: + case DXIL::ShaderKind::Domain: + case DXIL::ShaderKind::Intersection: + case DXIL::ShaderKind::AnyHit: + case DXIL::ShaderKind::ClosestHit: + case DXIL::ShaderKind::Miss: + case DXIL::ShaderKind::Node: + return true; + default: + return false; + } +} + void DxilDebugInstrumentation::applyOptions(PassOptions O) { GetPassOptionUnsigned(O, "FirstInstruction", &m_FirstInstruction, 0); GetPassOptionUnsigned(O, "LastInstruction", &m_LastInstruction, @@ -813,9 +835,17 @@ void DxilDebugInstrumentation::addInvocationSelectionProlog( case DXIL::ShaderKind::Vertex: ParameterTestResult = addVertexShaderProlog(BC, SVIndices); break; - case DXIL::ShaderKind::Hull: - ParameterTestResult = addHullhaderProlog(BC); - break; + case DXIL::ShaderKind::Hull: { + // OutputControlPointID only means something in the control point phase, so + // the patch-constant function is selected by primitive alone. + llvm::Function *function = BC.Builder.GetInsertBlock()->getParent(); + if (function == BC.DM.GetPatchConstantFunction()) { + ParameterTestResult = + addComparePrimitiveIdProlog(BC, m_Parameters.HullShader.PrimitiveId); + } else { + ParameterTestResult = addHullhaderProlog(BC); + } + } break; case DXIL::ShaderKind::Domain: ParameterTestResult = addComparePrimitiveIdProlog(BC, m_Parameters.DomainShader.PrimitiveId); @@ -993,11 +1023,17 @@ uint32_t DxilDebugInstrumentation::addDebugEntryValue(BuilderContext &BC, BC.Builder.CreateFPCast(TheValue, Type::getFloatTy(BC.Ctx), "AsFloat"); BytesToBeEmitted += addDebugEntryValue(BC, AsFloat); } else { + // RawBufferStore is only legal from shader model 6.2 onwards. PIX also + // instruments 6.0 and 6.1 shaders, so fall back to BufferStore (legal from + // 6.0) on those. The two differ only in the trailing alignment operand. + const bool SupportsRawBufferStore = BC.DM.GetShaderModel()->IsSM62Plus(); + const OP::OpCode StoreOpCode = SupportsRawBufferStore + ? OP::OpCode::RawBufferStore + : OP::OpCode::BufferStore; Function *StoreValue = - BC.HlslOP->GetOpFunc(OP::OpCode::RawBufferStore, + BC.HlslOP->GetOpFunc(StoreOpCode, TheValue->getType()); // Type::getInt32Ty(BC.Ctx)); - Constant *StoreValueOpcode = - BC.HlslOP->GetU32Const((unsigned)DXIL::OpCode::RawBufferStore); + Constant *StoreValueOpcode = BC.HlslOP->GetU32Const((unsigned)StoreOpCode); UndefValue *Undef32Arg = UndefValue::get(Type::getInt32Ty(BC.Ctx)); UndefValue *UndefArg = nullptr; if (TheValueTypeID == Type::TypeID::IntegerTyID) { @@ -1014,16 +1050,21 @@ uint32_t DxilDebugInstrumentation::addDebugEntryValue(BuilderContext &BC, auto &values = m_FunctionToValues[BC.Builder.GetInsertBlock()->getParent()]; Constant *RawBufferStoreAlignment = BC.HlslOP->GetU32Const(4); - (void)BC.Builder.CreateCall( - StoreValue, {StoreValueOpcode, // i32 opcode - values.UAVHandle, // %dx.types.Handle, ; resource handle - values.CurrentIndex, // i32 c0: index in bytes into UAV - Undef32Arg, // i32 c1: unused - TheValue, - UndefArg, // unused values - UndefArg, // unused values - UndefArg, // unused values - WriteMask_X, RawBufferStoreAlignment}); + SmallVector StoreArgs{ + StoreValueOpcode, // i32 opcode + values.UAVHandle, // %dx.types.Handle, ; resource handle + values.CurrentIndex, // i32 c0: index in bytes into UAV + Undef32Arg, // i32 c1: unused + TheValue, + UndefArg, // unused values + UndefArg, // unused values + UndefArg, // unused values + WriteMask_X}; + if (SupportsRawBufferStore) { + StoreArgs.push_back(RawBufferStoreAlignment); + } + + (void)BC.Builder.CreateCall(StoreValue, StoreArgs); assert(m_RemainingReservedSpaceInBytes >= 4); // check for underflow m_RemainingReservedSpaceInBytes -= 4; @@ -1313,19 +1354,54 @@ 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; + + std::vector functionsToInstrument; if (shaderKind == DXIL::ShaderKind::Library) { - auto instrumentableFunctions = - PIXPassHelpers::GetAllInstrumentableFunctions(DM); - for (auto *F : instrumentableFunctions) { - if (RunOnFunction(M, DM, uav, F)) { - modified = true; - } - } + functionsToInstrument = PIXPassHelpers::GetAllInstrumentableFunctions(DM); } else { + // Only the functions that the runtime itself invokes are instrumented. A + // helper that the entry point calls is not one of them, and cannot become + // one: PIX names an invocation by a record stream in the debug UAV and maps + // that stream to a single function, so instrumenting a helper produces a + // second invocation for one thread whose records PIX then discards. The + // annotation pass inlines such helpers away before anything is numbered. + // See PIXPassHelpers::InlineNonEntryFunctions. llvm::Function *entryFunction = PIXPassHelpers::GetEntryFunction(DM); - modified = RunOnFunction(M, DM, uav, entryFunction); + functionsToInstrument.push_back(entryFunction); + + // The runtime invokes a hull shader patch-constant function rather than the + // entry point does, so it survives inlining and is numbered and advertised + // to PIX as a steppable range of its own. Instrument it too, or a user who + // steps into it sees instructions with no values behind them. + llvm::Function *patchConstantFunction = DM.GetPatchConstantFunction(); + if (patchConstantFunction != nullptr && + patchConstantFunction != entryFunction) { + functionsToInstrument.push_back(patchConstantFunction); + } + } + + functionsToInstrument.erase( + std::remove_if(functionsToInstrument.begin(), functionsToInstrument.end(), + [&DM](llvm::Function *function) { + return function == nullptr || + !IsInstrumentableShaderKind( + PIXPassHelpers::GetFunctionShaderKind( + DM, function)); + }), + functionsToInstrument.end()); + + // Creating the UAV modifies the module, so nothing may be created before the + // pass knows it has something to instrument. + if (functionsToInstrument.empty()) { + return false; + } + + auto *uav = PIXPassHelpers::CreateGlobalUAVResource(DM, HLSLBindId, "PIXUAV"); + bool modified = false; + for (auto *function : functionsToInstrument) { + if (RunOnFunction(M, DM, uav, function)) { + modified = true; + } } return modified; } @@ -1496,23 +1572,7 @@ bool DxilDebugInstrumentation::RunOnFunction(Module &M, DxilModule &DM, DXIL::ShaderKind shaderKind = PIXPassHelpers::GetFunctionShaderKind(DM, function); - switch (shaderKind) { - case DXIL::ShaderKind::Amplification: - case DXIL::ShaderKind::Mesh: - case DXIL::ShaderKind::Vertex: - case DXIL::ShaderKind::Geometry: - case DXIL::ShaderKind::Pixel: - case DXIL::ShaderKind::Compute: - case DXIL::ShaderKind::RayGeneration: - case DXIL::ShaderKind::Hull: - case DXIL::ShaderKind::Domain: - case DXIL::ShaderKind::Intersection: - case DXIL::ShaderKind::AnyHit: - case DXIL::ShaderKind::ClosestHit: - case DXIL::ShaderKind::Miss: - case DXIL::ShaderKind::Node: - break; - default: + if (!IsInstrumentableShaderKind(shaderKind)) { return false; } llvm::SmallPtrSet RayQueryHandles; diff --git a/lib/DxilPIXPasses/PixPassHelpers.cpp b/lib/DxilPIXPasses/PixPassHelpers.cpp index f05a02c120..4d796eb5b2 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.cpp +++ b/lib/DxilPIXPasses/PixPassHelpers.cpp @@ -22,6 +22,7 @@ #include "llvm/IR/Module.h" #include "llvm/IR/PassManager.h" #include "llvm/Pass.h" +#include "llvm/Transforms/Utils/Cloning.h" #include "PixPassHelpers.h" @@ -437,6 +438,92 @@ GetAllInstrumentableFunctions(hlsl::DxilModule &DM) { return ret; } +bool InlineNonEntryFunctions( + hlsl::DxilModule &DM, + llvm::SmallVectorImpl *UninlinedFunctions) { + if (UninlinedFunctions != nullptr) { + UninlinedFunctions->clear(); + } + + if (DM.GetShaderModel()->IsLib()) { + return false; + } + + // The runtime invokes a hull shader patch-constant function directly, so it + // is a second root of the call graph and stays alongside the entry point. + llvm::Function *const entryFunction = DM.GetEntryFunction(); + llvm::Function *const patchConstantFunction = DM.GetPatchConstantFunction(); + + // A module that names no entry point has no root, and the entry point has no + // caller in the IR. Leave such a module alone rather than erase every + // function in it. + if (entryFunction == nullptr) { + return false; + } + + bool modified = false; + + // HLSL has no recursion, so the call graph is acyclic and inlining leaf-ward + // terminates. A fixed-point loop also reaches a helper that loses its last + // caller only once another helper is inlined away. + bool inlinedACallThisRound = true; + while (inlinedACallThisRound) { + inlinedACallThisRound = false; + + for (llvm::Function *function : GetAllInstrumentableFunctions(DM)) { + if (function == entryFunction || function == patchConstantFunction) { + continue; + } + + // llvm::InlineFunction is the mechanical inliner and ignores inlining + // attributes. Clear the attribute so the module carries no claim that + // contradicts its own shape. + function->removeFnAttr(llvm::Attribute::NoInline); + + // Collect the call sites first, because inlining rewrites the use list. + llvm::SmallVector callSites; + for (llvm::User *user : function->users()) { + if (auto *call = llvm::dyn_cast(user)) { + if (call->getCalledFunction() == function) { + callSites.push_back(call); + } + } + } + + for (llvm::CallInst *callSite : callSites) { + llvm::InlineFunctionInfo inlineFunctionInfo; + if (llvm::InlineFunction(callSite, inlineFunctionInfo)) { + inlinedACallThisRound = true; + modified = true; + } + } + + // A body with no caller still gets numbered and advertised to PIX as + // somewhere to step into. Erase it. DxilModule keeps an entry-property + // map and a type-annotation map keyed on llvm::Function *, so tell it + // first or both keep entries keyed on freed storage. + if (function->use_empty()) { + DM.RemoveFunction(function); + function->eraseFromParent(); + modified = true; + } + } + } + + if (UninlinedFunctions != nullptr) { + // A function reached other than by a direct call, or one that + // llvm::InlineFunction declines, is still here. PIX gets an instruction + // range for it that no trace record arrives for, so report it. + for (llvm::Function *function : GetAllInstrumentableFunctions(DM)) { + if (function != entryFunction && function != patchConstantFunction) { + UninlinedFunctions->push_back(function); + } + } + } + + return modified; +} + hlsl::DXIL::ShaderKind GetFunctionShaderKind(hlsl::DxilModule &DM, llvm::Function *fn) { hlsl::DXIL::ShaderKind shaderKind = hlsl::DXIL::ShaderKind::Invalid; diff --git a/lib/DxilPIXPasses/PixPassHelpers.h b/lib/DxilPIXPasses/PixPassHelpers.h index b2e0feea1f..b7ea71b2fc 100644 --- a/lib/DxilPIXPasses/PixPassHelpers.h +++ b/lib/DxilPIXPasses/PixPassHelpers.h @@ -56,6 +56,36 @@ void EraseIfUnused(hlsl::DxilModule &DM, llvm::Function *OpFunction); void ClearViewIdState(hlsl::DxilModule &DM); std::vector GetAllInstrumentableFunctions(hlsl::DxilModule &DM); +// Inlines each function that the runtime does not invoke into its callers, and +// erases the inlined-away body. +// +// PIX identifies one shader invocation by one record stream in the debug UAV, +// and maps that stream to exactly one function. A helper instrumented as a +// function of its own therefore reads as a second invocation of a thread that +// runs once, and PIX discards its records. An inlined helper stays visible in +// the inlinedAt chain of the debug locations, which is where PIX looks for it. +// +// Call this before any pass numbers instructions or synthesizes shadow storage. +// PIX steps through the ordinals of the module this leaves behind, and +// llvm::InlineFunction stamps the call site debug location onto each inlined +// instruction that carries none. This function is idempotent, so every pass +// that can come first in a PIX pipeline calls it. +// +// A library module keeps every function, because each exported function is an +// invocation of its own. +// +// UninlinedFunctions, when supplied, receives each non-entry function that is +// still in the module afterwards. Such a function keeps an instruction range +// that no trace record arrives for, so the pass that advertises those ranges +// supplies this parameter and reports what it receives. A pass that advertises +// no range supplies nothing and stays silent, which also keeps one pipeline +// from naming the same function twice. +// +// The survivor set is recomputed on every call, so a caller still receives it +// when an earlier caller already inlined the module. +bool InlineNonEntryFunctions( + hlsl::DxilModule &DM, + llvm::SmallVectorImpl *UninlinedFunctions = nullptr); hlsl::DXIL::ShaderKind GetFunctionShaderKind(hlsl::DxilModule &DM, llvm::Function *fn); #ifdef PIX_DEBUG_DUMP_HELPER diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugBreakInstrumentationInHelperFunction.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugBreakInstrumentationInHelperFunction.hlsl new file mode 100644 index 0000000000..3755f05285 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DebugBreakInstrumentationInHelperFunction.hlsl @@ -0,0 +1,34 @@ +// RUN: %dxc -Emain -Tcs_6_10 %s | %opt -S -dxil-annotate-with-virtual-regs -hlsl-dxil-debugbreak-instrumentation -hlsl-dxilemit | %FileCheck %s + +// The debug-break pipeline shares the annotation prepass, so it also sees a +// module whose helpers are inlined away. A DebugBreak inside a [noinline] +// helper must still be found and instrumented once the helper is part of the +// entry point. + +// The helper does not appear as a separate function. +// CHECK-NOT: define {{.*}}BreakInHelper + +// The prepass reports no surviving helper, so PIX offers one steppable range. +// CHECK-NOT: UninlinedFunction: +// CHECK: InstructionRange: {{[0-9]+}} {{[0-9]+}} main cs +// CHECK-NOT: InstructionRange: + +// The break is still recorded, from inside the entry point. +// CHECK: %PixUAVHandle = call %dx.types.Handle @dx.op.createHandleFromBinding( +// CHECK: %DebugBreakBitSet = call i32 @dx.op.atomicBinOp.i32(i32 78, %dx.types.Handle +// CHECK-NOT: @dx.op.debugBreak + +RWStructuredBuffer Output : register(u0); + +[noinline] +uint BreakInHelper(uint value) +{ + DebugBreak(); + return value + 1; +} + +[numthreads(1, 1, 1)] +void main(uint3 threadId : SV_DispatchThreadID) +{ + Output[0] = BreakInHelper(threadId.x); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugHullPatchConstantFunction.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugHullPatchConstantFunction.hlsl new file mode 100644 index 0000000000..dc39758be3 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DebugHullPatchConstantFunction.hlsl @@ -0,0 +1,79 @@ +// RUN: %dxc -Emain -Ths_6_2 %s | %opt -S -dxil-annotate-with-virtual-regs -hlsl-dxil-debug-instrumentation,parameter0=1,parameter1=2 -hlsl-dxilemit | %FileCheck %s + +// The runtime invokes a hull shader patch-constant function rather than the +// entry point does, so the inlining keeps it and the annotation pass numbers it +// as a steppable range of its own. An uninstrumented range emits no trace +// record, so a user who steps into the patch-constant body sees instructions +// with no values behind them. The pass instruments it as well as the entry +// point. +// +// SV_OutputControlPointID only means something in the control point phase, so +// the patch-constant function selects an invocation by primitive alone. + +// Two functions are numbered, so the helper that both of them call is inlined +// away. No third range is advertised, and none survives uninlined. +// CHECK-DAG: InstructionRange: {{[0-9]+ [0-9]+}} main hs +// CHECK-DAG: InstructionRange: {{[0-9]+ [0-9]+}} PatchConstantFunction +// CHECK-NOT: InstructionRange: +// CHECK-NOT: UninlinedFunction: + +// The patch-constant function selects on the primitive alone. +// CHECK: define void @"\01?PatchConstantFunction +// CHECK: %PrimId = call i32 @dx.op.primitiveID.i32(i32 108) +// CHECK-NEXT: %CompareToPrimId = icmp eq i32 %PrimId, 1 +// CHECK-NEXT: br i1 %CompareToPrimId, label %PIXInterestingBlock, label %PIXNonInterestingBlock + +// The entry point selects on the control point and the primitive. +// CHECK: define void @main() +// CHECK: %ControlPointId = call i32 @dx.op.outputControlPointID.i32(i32 107) +// CHECK-NEXT: %PrimId = call i32 @dx.op.primitiveID.i32(i32 108) +// CHECK-NEXT: %CompareToPrimId = icmp eq i32 %PrimId, 1 +// CHECK-NEXT: %CompareToControlPointId = icmp eq i32 %ControlPointId, 2 +// CHECK-NEXT: %CompareBoth = and i1 %CompareToControlPointId, %CompareToPrimId +// CHECK-NEXT: br i1 %CompareBoth, label %PIXInterestingBlock, label %PIXNonInterestingBlock + +// CHECK-NOT: HullHelper + +struct HsConstantData +{ + float Edges[3] : SV_TessFactor; + float Inside : SV_InsideTessFactor; +}; + +struct ControlPoint +{ + float3 position : WORLDPOS; +}; + +struct OutputPoint +{ + float3 vPosition : BEZIERPOS; +}; + +[noinline] +float HullHelper(float value) +{ + return value * 2.f; +} + +HsConstantData PatchConstantFunction(InputPatch ip) +{ + HsConstantData Output; + Output.Edges[0] = HullHelper(ip[0].position.x); + Output.Edges[1] = 8; + Output.Edges[2] = 8; + Output.Inside = 8; + return Output; +} + +[domain("tri")] +[partitioning("integer")] +[outputtopology("triangle_cw")] +[outputcontrolpoints(3)] +[patchconstantfunc("PatchConstantFunction")] +OutputPoint main(InputPatch ip, uint i : SV_OutputControlPointID) +{ + OutputPoint Output; + Output.vPosition = ip[i].position * HullHelper(2.f); + return Output; +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugNoInlineHelperFunction.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugNoInlineHelperFunction.hlsl new file mode 100644 index 0000000000..9da14081dc --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DebugNoInlineHelperFunction.hlsl @@ -0,0 +1,60 @@ +// RUN: %dxc -Emain -Tcs_6_2 /Od /Zi %s | %opt -S -dxil-annotate-with-virtual-regs -hlsl-dxil-debug-instrumentation,parameter0=1,parameter1=2,parameter2=3 -hlsl-dxilemit | %FileCheck %s +// RUN: %dxc -Emain -Tcs_6_2 /Od /Zi %s | %opt -S -dxil-dbg-value-to-dbg-declare -dxil-annotate-with-virtual-regs -hlsl-dxil-debug-instrumentation,parameter0=1,parameter1=2,parameter2=3 -hlsl-dxilemit | %FileCheck %s -check-prefixes=LOCALS,TRACED + +// PIX names a shader invocation by the stream of records that one thread writes +// into the debug UAV, and maps that stream to exactly one function. A helper +// instrumented as a function of its own writes its records under a second +// invocation identity for a thread that runs once, and PIX discards them. +// +// The passes inline a helper into the entry point before anything is numbered. +// PIX recovers the helper frame from the inlinedAt chain of each inlined +// instruction, and the helper locals stay attributed to the helper. + +// One function gives one invocation identity. +// CHECK: InstructionRange: {{[0-9]+}} {{[0-9]+}} main cs +// CHECK-NOT: InstructionRange: + +// Every helper is inlined, so the pass reports none as surviving. +// CHECK-NOT: UninlinedFunction: + +// CHECK: define void @main() +// CHECK-NOT: define {{.*}}ScaleHelper + +// Debug info still names the helper, so PIX rebuilds the call stack. +// CHECK: !DISubprogram(name: "ScaleHelper" +// CHECK: inlinedAt: + +// A helper local must be traced as well as scoped, or PIX reads it as +// unavailable. main holds no float local of its own, so a float alloca that +// carries a virtual register belongs to the inlined helper. +// TRACED: [[SCALED:%[0-9]+]] = alloca [1 x float], i32 0, !pix-alloca-reg + +// The helper local stays scoped to the helper, which puts it under the correct +// frame in the PIX locals view. +// LOCALS: call void @llvm.dbg.declare(metadata [1 x float]* [[SCALED]],{{.*}}; var:"scaled" + +// The store that traces the local carries no debug location. Requiring +// !pix-dxil-inst-num to follow the pointer operand immediately checks this. +// llvm::InlineFunction stamps the call site location onto each inlined +// instruction that carries none, so shadow storage must not exist yet when the +// helper is inlined. +// TRACED: [[SCALEDGEP:%[0-9]+]] = getelementptr [1 x float], [1 x float]* [[SCALED]], i32 0, i32 0 +// TRACED-NEXT: store float %{{[A-Za-z0-9_.]+}}, float* [[SCALEDGEP]], !pix-dxil-inst-num {{![0-9]+}}, !pix-alloca-reg-write + +// LOCALS: ![[HELPER:[0-9]+]] = !DISubprogram(name: "ScaleHelper" +// LOCALS: !DILocalVariable({{.*}}name: "scaled", scope: ![[HELPER]], + +RWStructuredBuffer Output : register(u0); + +[noinline] +float ScaleHelper(float value) +{ + float scaled = value * 3.f; + return scaled; +} + +[numthreads(1, 1, 1)] +void main(uint3 threadId : SV_DispatchThreadID) +{ + Output[threadId.x] = ScaleHelper(threadId.y); +} diff --git a/tools/clang/test/HLSLFileCheck/pix/DebugStoreOpcodeByShaderModel.hlsl b/tools/clang/test/HLSLFileCheck/pix/DebugStoreOpcodeByShaderModel.hlsl new file mode 100644 index 0000000000..38652bd01f --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/DebugStoreOpcodeByShaderModel.hlsl @@ -0,0 +1,21 @@ +// RUN: %dxc -Emain -Tcs_6_0 %s | %opt -S -hlsl-dxil-debug-instrumentation,UAVSize=1024 -hlsl-dxilemit | %FileCheck %s -check-prefix=SM60 +// RUN: %dxc -Emain -Tcs_6_1 %s | %opt -S -hlsl-dxil-debug-instrumentation,UAVSize=1024 -hlsl-dxilemit | %FileCheck %s -check-prefix=SM61 +// RUN: %dxc -Emain -Tcs_6_2 %s | %opt -S -hlsl-dxil-debug-instrumentation,UAVSize=1024 -hlsl-dxilemit | %FileCheck %s -check-prefix=SM62 + +// SM60: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle +// SM60-DAG: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_DebugUAV_Handle +// SM60-DAG: declare void @dx.op.bufferStore.i32(i32, %dx.types.Handle, i32, i32, i32, i32, i32, i32, i8) + +// SM61: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle +// SM61-DAG: call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %PIX_DebugUAV_Handle +// SM61-DAG: declare void @dx.op.bufferStore.i32(i32, %dx.types.Handle, i32, i32, i32, i32, i32, i32, i8) + +// SM62: %PIX_DebugUAV_Handle = call %dx.types.Handle @dx.op.createHandle +// SM62-DAG: call void @dx.op.rawBufferStore.i32(i32 140, %dx.types.Handle %PIX_DebugUAV_Handle +// SM62-DAG: declare void @dx.op.rawBufferStore.i32(i32, %dx.types.Handle, i32, i32, i32, i32, i32, i32, i8, i32) + +[RootSignature("")] +[numthreads(1, 1, 1)] +void main(uint threadId : SV_DispatchThreadID) { + uint value = threadId; +} diff --git a/tools/clang/test/HLSLFileCheck/pix/NonUniformResourceIndexInHelperFunction.hlsl b/tools/clang/test/HLSLFileCheck/pix/NonUniformResourceIndexInHelperFunction.hlsl new file mode 100644 index 0000000000..bae3fcf097 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/pix/NonUniformResourceIndexInHelperFunction.hlsl @@ -0,0 +1,38 @@ +// RUN: %dxc -Emain -Tps_6_0 %s | %opt -S -dxil-annotate-with-virtual-regs -hlsl-dxil-non-uniform-resource-index-instrumentation -hlsl-dxilemit | %FileCheck %s + +// The annotation prepass inlines away each non-entry function of a non-library +// module, because PIX cannot attribute a separately instrumented function to an +// invocation. See PIXPassHelpers::InlineNonEntryFunctions. The +// non-uniform-resource-index pipeline shares that prepass for the instruction +// ordinals its diagnostics use, so an unqualified dynamic index inside a +// [noinline] helper must still be diagnosed once the helper is part of the +// entry point. +// +// The [noinline] attribute is necessary. Without it the front end inlines the +// helper and the prepass has nothing to do. The helper signature holds scalars +// only, because a function that reaches DXIL in a non-library module takes and +// returns no vector. + +// The helper does not appear as a separate function. +// CHECK-NOT: define {{.*}}IndexInHelper + +// The dynamic index is still reported, and it is addressed to a real ordinal +// instead of to bit 0. +// CHECK: @dx.op.waveActiveAllEqual +// CHECK: shl i32 %{{[0-9]+}}, {{[1-9][0-9]*}} +// CHECK: @dx.op.atomicBinOp.i32(i32 78 +// CHECK-NOT: NuriNotInstrumentedMissingInstructionNumber + +Texture2D tex[8] : register(t0); + +[noinline] +float IndexInHelper(float u, float v) +{ + uint index = u * v; + return tex[index].Load(int3(0, 0, 0)).x; +} + +float4 main(float2 uv : TEXCOORD0) : SV_TARGET +{ + return IndexInHelper(uv.x, uv.y); +} diff --git a/tools/clang/unittests/HLSL/PixTest.cpp b/tools/clang/unittests/HLSL/PixTest.cpp index ff2ebf1308..a9a483dfc8 100644 --- a/tools/clang/unittests/HLSL/PixTest.cpp +++ b/tools/clang/unittests/HLSL/PixTest.cpp @@ -167,7 +167,11 @@ class PixTest : public ::testing::Test { TEST_METHOD(ToolsUav_LibraryWithTwoEntryPointsCreatesOnePair) TEST_METHOD(ToolsUav_ExtendsEveryGlobalRootSignatureSubobject) TEST_METHOD(DebugInstrumentation_RawBufferShaderFlagDeclared) + TEST_METHOD(DebugInstrumentation_NothingInstrumentableAddsNoUav) TEST_METHOD(ToolsUav_RootSignatureSerializationFailurePreservesSignature) + TEST_METHOD(DebugInstrumentation_SM60UsesBufferStore) + TEST_METHOD(DebugInstrumentation_SM61UsesBufferStore) + TEST_METHOD(DebugInstrumentation_SM62UsesRawBufferStore) TEST_METHOD(ConstantColor_UnusedIntOverloadIsErased) TEST_METHOD(ConstantColor_NoTargetOverloadsAreErased) TEST_METHOD(ConstantColor_FromConstantBufferIsWellFormed) @@ -214,6 +218,15 @@ class PixTest : public ::testing::Test { TEST_METHOD(NonUniformResourceIndex_DescriptorHeap) TEST_METHOD(NonUniformResourceIndex_Raytracing) + TEST_METHOD(HelperInlining_NoEntryFunctionLeavesModuleAlone) + TEST_METHOD(HelperInlining_SurvivingHelperIsReportedByExactName) + TEST_METHOD(HelperInlining_SurvivingHelperIsReportedAfterEarlierPrepass) + TEST_METHOD(HelperInlining_InlinedHelperIsNotReported) + TEST_METHOD(HelperInlining_InlinedHelperValidates) + TEST_METHOD(HelperInlining_HullPatchConstantFunctionValidates) + TEST_METHOD(HelperInlining_NonUniformResourceIndexHelperValidates) + TEST_METHOD(HelperInlining_DebugBreakHelperValidates) + // Control tests for the PIX pass validation harness below // (ValidateInstrumentedModule / VerifyInstrumentedModuleIsValid). TEST_METHOD(Validation_ControlValidModulePasses) @@ -367,6 +380,19 @@ class PixTest : public ::testing::Test { // Runs the virtual-register annotation pass over textual IR and returns the // pass report. Textual IR builds a module shape that HLSL does not express. std::vector RunAnnotationPassOnText(const std::string &irText) { + return RunPassesOnText(irText, {L"-dxil-annotate-with-virtual-regs"}); + } + + // Runs both prepasses that inline, in the order the debug pipeline uses, so + // the annotation pass sees a module another pass already inlined. + std::vector + RunDbgValueAndAnnotationPassesOnText(const std::string &irText) { + return RunPassesOnText(irText, {L"-dxil-dbg-value-to-dbg-declare", + L"-dxil-annotate-with-virtual-regs"}); + } + + std::vector RunPassesOnText(const std::string &irText, + std::vector passes) { CComPtr pSource; CreateBlobFromText(m_dllSupport, irText.c_str(), &pSource); @@ -376,7 +402,7 @@ class PixTest : public ::testing::Test { std::vector Options; Options.push_back(L"-S"); Options.push_back(L"-opt-mod-passes"); - Options.push_back(L"-dxil-annotate-with-virtual-regs"); + Options.insert(Options.end(), passes.begin(), passes.end()); CComPtr pOptimizedModule; CComPtr pText; @@ -386,6 +412,30 @@ class PixTest : public ::testing::Test { return Tokenize(BlobToUtf8(pText).c_str(), "\n"); } + static bool AnyLineContains(const std::vector &lines, + const char *needle) { + for (const std::string &line : lines) { + if (line.find(needle) != std::string::npos) { + return true; + } + } + return false; + } + + // Collects the whole payload of each report record with the given prefix, so + // a test pins the exact record rather than a substring of it. + static std::vector + RecordsWithPrefix(const std::vector &lines, + const std::string &prefix) { + std::vector records; + for (const std::string &line : lines) { + if (line.compare(0, prefix.size(), prefix) == 0) { + records.push_back(line.substr(prefix.size())); + } + } + return records; + } + // Replaces the one occurrence of needle, and fails the test when the text // does not hold exactly one. static std::string ReplaceOnlyOccurrence(const std::string &text, @@ -400,6 +450,77 @@ class PixTest : public ::testing::Test { return result; } + // Returns the module with a global holding the helper's address, which makes + // the helper reachable other than by a direct call and so unreachable for the + // inliner. HLSL itself expresses no such module. + static std::string TakeAddressOfHelper(const std::string &disassembly, + const char *mangledHelperName) { + const std::string entryDefinition = "define void @main() {"; + return ReplaceOnlyOccurrence( + disassembly, entryDefinition, + "@PIXTestHelperAddress = internal global float (float)* @\"\\01?" + + std::string(mangledHelperName) + "\"\n\n" + entryDefinition); + } + + // Counts the definitions in a disassembly whose name holds the given text. + static unsigned CountFunctionDefinitions(const std::string &disassembly, + const char *nameFragment) { + unsigned count = 0; + for (const std::string &line : Tokenize(disassembly.c_str(), "\n")) { + if (line.compare(0, strlen("define "), "define ") == 0 && + line.find(nameFragment) != std::string::npos) { + count++; + } + } + return count; + } + + static unsigned CountOccurrences(const std::string &text, + const char *needle) { + unsigned count = 0; + size_t needleLength = strlen(needle); + for (size_t position = text.find(needle); position != std::string::npos; + position = text.find(needle, position + needleLength)) { + count++; + } + return count; + } + + // Counts calls to a named operation. A disassembly also holds one declare + // line per operation, which is not a call. + static unsigned CountCallsTo(const std::string &disassembly, + const char *operation) { + unsigned count = 0; + for (const std::string &line : Tokenize(disassembly.c_str(), "\n")) { + if (line.find(operation) != std::string::npos && + line.find("call ") != std::string::npos) { + count++; + } + } + return count; + } + + // Returns the text of the one definition whose name holds the given fragment, + // so a test asserts about one function rather than the whole module. + static std::string ExtractFunctionBody(const std::string &disassembly, + const char *nameFragment) { + std::string body; + bool inside = false; + for (const std::string &line : Tokenize(disassembly.c_str(), "\n")) { + if (!inside) { + inside = line.compare(0, strlen("define "), "define ") == 0 && + line.find(nameFragment) != std::string::npos; + } else if (line.compare(0, 1, "}") == 0) { + break; + } + if (inside) { + body += line + "\n"; + } + } + VERIFY_IS_FALSE(body.empty()); + return body; + } + SinglePassOutput RunSinglePass(IDxcBlob *dxil, LPCWSTR passOption) { CComPtr pOptimizer; VERIFY_SUCCEEDED( @@ -3724,6 +3845,22 @@ void main(uint threadId : SV_DispatchThreadID) "debug instrumentation shader flags"); } +TEST_F(PixTest, DebugInstrumentation_NothingInstrumentableAddsNoUav) { + const char *source = R"x( +export float Helper(float x) +{ + return x * 2; +} +)x"; + + auto compiled = Compile(m_dllSupport, source, L"lib_6_6", {L"-Od"}); + CComPtr dxil = FindModule(DFCC_ShaderDebugInfoDXIL, compiled); + auto output = RunDebugPass(dxil); + + VERIFY_ARE_EQUAL( + 0, CountToolsUAVRecords(Tokenize(Disassemble(output.blob), "\n"))); +} + TEST_F(PixTest, ToolsUav_RootSignatureSerializationFailurePreservesSignature) { const char *source = R"x( [numthreads(1, 1, 1)] @@ -3802,6 +3939,83 @@ void main() VERIFY_IS_TRUE(foundRootSignature); } +static const char *kDebugStoreOpcodeComputeShader = R"x( +[numthreads(1, 1, 1)] +void main(uint threadId : SV_DispatchThreadID) +{ + uint value = threadId; +} +)x"; + +// RawBufferStore is illegal below shader model 6.2. Debug instrumentation of +// 6.0 and 6.1 shaders must emit BufferStore (opcode 69) instead, and the +// instrumented module must validate. The declaration and opcode are checked so +// a test cannot pass by skipping instrumentation. +static void VerifyDebugStoreOpcodeDisassembly(const std::string &disassembly, + bool expectRawBufferStore) { + VERIFY_IS_TRUE(disassembly.find("PIX_DebugUAV_Handle") != std::string::npos); + VERIFY_ARE_NOT_EQUAL( + 0u, PixTest::CountCallsTo(disassembly, "@dx.op.atomicBinOp.i32")); + + if (expectRawBufferStore) { + VERIFY_ARE_NOT_EQUAL( + 0u, PixTest::CountCallsTo(disassembly, "@dx.op.rawBufferStore.i32")); + VERIFY_IS_TRUE( + disassembly.find("call void @dx.op.rawBufferStore.i32(i32 " + "140, %dx.types.Handle %PIX_DebugUAV_Handle") != + std::string::npos); + VERIFY_IS_TRUE( + disassembly.find("declare void @dx.op.rawBufferStore.i32(i32, " + "%dx.types.Handle, i32, i32, i32, i32, i32, i32, i8, " + "i32)") != std::string::npos); + VERIFY_ARE_EQUAL(0u, + PixTest::CountCallsTo(disassembly, "@dx.op.bufferStore")); + VERIFY_IS_TRUE(disassembly.find("declare void @dx.op.bufferStore") == + std::string::npos); + } else { + VERIFY_ARE_NOT_EQUAL( + 0u, PixTest::CountCallsTo(disassembly, "@dx.op.bufferStore.i32")); + VERIFY_IS_TRUE(disassembly.find("call void @dx.op.bufferStore.i32(i32 69, " + "%dx.types.Handle %PIX_DebugUAV_Handle") != + std::string::npos); + VERIFY_IS_TRUE( + disassembly.find( + "declare void @dx.op.bufferStore.i32(i32, %dx.types.Handle, i32, " + "i32, i32, i32, i32, i32, i8)") != std::string::npos); + VERIFY_ARE_EQUAL( + 0u, PixTest::CountCallsTo(disassembly, "@dx.op.rawBufferStore")); + VERIFY_IS_TRUE(disassembly.find("declare void @dx.op.rawBufferStore") == + std::string::npos); + } +} + +TEST_F(PixTest, DebugInstrumentation_SM60UsesBufferStore) { + auto compiled = Compile(m_dllSupport, kDebugStoreOpcodeComputeShader, + L"cs_6_0", {L"-Od"}); + auto output = RunDebugPass(compiled); + VerifyInstrumentedModuleIsValid( + output.blob, "debug instrumentation of an SM 6.0 compute shader"); + VerifyDebugStoreOpcodeDisassembly(Disassemble(output.blob), false); +} + +TEST_F(PixTest, DebugInstrumentation_SM61UsesBufferStore) { + auto compiled = Compile(m_dllSupport, kDebugStoreOpcodeComputeShader, + L"cs_6_1", {L"-Od"}); + auto output = RunDebugPass(compiled); + VerifyInstrumentedModuleIsValid( + output.blob, "debug instrumentation of an SM 6.1 compute shader"); + VerifyDebugStoreOpcodeDisassembly(Disassemble(output.blob), false); +} + +TEST_F(PixTest, DebugInstrumentation_SM62UsesRawBufferStore) { + auto compiled = Compile(m_dllSupport, kDebugStoreOpcodeComputeShader, + L"cs_6_2", {L"-Od"}); + auto output = RunDebugPass(compiled); + VerifyInstrumentedModuleIsValid( + output.blob, "debug instrumentation of an SM 6.2 compute shader"); + VerifyDebugStoreOpcodeDisassembly(Disassemble(output.blob), true); +} + static bool HasUnusedDeclaration(std::vector const &lines, std::string const &functionName) { bool declared = false; @@ -5653,3 +5867,311 @@ float4 main(float4 pos : SV_Position) : SV_Target auto output = RunPixelHitPass(compiled, 16, 64, 0 /*requiredSVPositionRow*/); VerifyInstrumentedModuleIsValid(output.blob, "pixel-hit instrumentation"); } +static const char *const kHullShaderWithHelper = R"x( +struct HsConstantData +{ + float Edges[3] : SV_TessFactor; + float Inside : SV_InsideTessFactor; +}; + +struct ControlPoint +{ + float3 position : WORLDPOS; +}; + +struct OutputPoint +{ + float3 vPosition : BEZIERPOS; +}; + +[noinline] +float HullHelper(float value) +{ + return value * 2.f; +} + +HsConstantData PatchConstantFunction(InputPatch ip) +{ + HsConstantData Output; + Output.Edges[0] = HullHelper(ip[0].position.x); + Output.Edges[1] = 8; + Output.Edges[2] = 8; + Output.Inside = 8; + return Output; +} + +[domain("tri")] +[partitioning("integer")] +[outputtopology("triangle_cw")] +[outputcontrolpoints(3)] +[patchconstantfunc("PatchConstantFunction")] +OutputPoint main(InputPatch ip, uint i : SV_OutputControlPointID) +{ + OutputPoint Output; + Output.vPosition = ip[i].position * HullHelper(2.f); + return Output; +})x"; + +static const char *const kComputeShaderWithHelper = R"x( +RWStructuredBuffer Output : register(u0); + +[noinline] +float ScaleHelper(float value) +{ + float scaled = value * 3.f; + return scaled; +} + +[numthreads(1, 1, 1)] +void main(uint3 threadId : SV_DispatchThreadID) +{ + Output[threadId.x] = ScaleHelper(threadId.y); +})x"; + +// A module names no entry function, so nothing roots the call graph. The +// inlining leaves such a module alone rather than erase every function in it. +// The entry point has no caller in the IR, so it is the first body that an +// inlining rooted on the patch-constant function alone erases. +TEST_F(PixTest, HelperInlining_NoEntryFunctionLeavesModuleAlone) { + auto compiled = + Compile(m_dllSupport, kHullShaderWithHelper, L"hs_6_2", {L"-Od"}); + std::string disassembly = Disassemble(compiled); + + // The baseline names an entry function, so the edit below is what removes it. + VERIFY_IS_TRUE(disassembly.find("!{void ()* @main,") != std::string::npos); + std::string withoutEntry = + ReplaceOnlyOccurrence(disassembly, "!{void ()* @main,", "!{null,"); + + std::vector lines = RunAnnotationPassOnText(withoutEntry); + + // Every function is still here, entry point included. + VERIFY_IS_TRUE(AnyLineContains(lines, "define void @main()")); + VERIFY_IS_TRUE(AnyLineContains(lines, "HullHelper")); + VERIFY_IS_TRUE(AnyLineContains(lines, "PatchConstantFunction")); +} + +// Something other than a direct call reaches a function, so the function +// survives inlining. The annotation pass then advertises it to PIX as a +// steppable range that no trace record arrives for, and names it in the report +// so PIX does not offer it. Taking the address of a helper produces that shape, +// which HLSL itself does not express. +TEST_F(PixTest, HelperInlining_SurvivingHelperIsReportedByExactName) { + auto compiled = + Compile(m_dllSupport, kComputeShaderWithHelper, L"cs_6_2", {L"-Od"}); + std::string withAddressTaken = + TakeAddressOfHelper(Disassemble(compiled), "ScaleHelper@@YAMM@Z"); + + std::vector lines = RunAnnotationPassOnText(withAddressTaken); + + // Exactly one record arrives, and it names the helper exactly. The report + // strips the leading mangling marker that + // PrintableSubsetOfMangledFunctionName removes, so the whole payload is + // compared, not a substring of it. + std::vector records = + RecordsWithPrefix(lines, "UninlinedFunction:"); + VERIFY_ARE_EQUAL(1u, static_cast(records.size())); + VERIFY_ARE_EQUAL(std::string("ScaleHelper@@YAMM@Z"), records[0]); + + // The record is deterministic, so a second run over the same input produces + // the same one record. + std::vector repeatRecords = RecordsWithPrefix( + RunAnnotationPassOnText(withAddressTaken), "UninlinedFunction:"); + VERIFY_ARE_EQUAL(records, repeatRecords); +} + +// Both prepasses inline, and the debug pipeline runs the dbg-value pass first, +// so the annotation pass reaches an already-inlined module and has nothing left +// to inline. It still names the survivor, because it is the pass that +// advertises the instruction range the survivor keeps. +TEST_F(PixTest, HelperInlining_SurvivingHelperIsReportedAfterEarlierPrepass) { + auto compiled = + Compile(m_dllSupport, kComputeShaderWithHelper, L"cs_6_2", {L"-Od"}); + std::string withAddressTaken = + TakeAddressOfHelper(Disassemble(compiled), "ScaleHelper@@YAMM@Z"); + + std::vector annotateOnly = RecordsWithPrefix( + RunAnnotationPassOnText(withAddressTaken), "UninlinedFunction:"); + VERIFY_ARE_EQUAL(1u, static_cast(annotateOnly.size())); + + // The report survives the earlier prepass, and arrives once rather than once + // per pass that inlines. + std::vector bothPrepasses = + RecordsWithPrefix(RunDbgValueAndAnnotationPassesOnText(withAddressTaken), + "UninlinedFunction:"); + VERIFY_ARE_EQUAL(1u, static_cast(bothPrepasses.size())); + VERIFY_ARE_EQUAL(annotateOnly, bothPrepasses); + + // The range the report is about is advertised, so the report is not vacuous. + std::vector ranges = + RecordsWithPrefix(RunDbgValueAndAnnotationPassesOnText(withAddressTaken), + "InstructionRange: "); + VERIFY_ARE_EQUAL(2u, static_cast(ranges.size())); + VERIFY_IS_TRUE(AnyLineContains(ranges, "ScaleHelper")); +} + +// An inlined helper produces no record at all, so the presence of a record +// means what the test above asserts it means. +TEST_F(PixTest, HelperInlining_InlinedHelperIsNotReported) { + auto compiled = + Compile(m_dllSupport, kComputeShaderWithHelper, L"cs_6_2", {L"-Od"}); + std::vector lines = + RunAnnotationPassOnText(Disassemble(compiled)); + + VERIFY_ARE_EQUAL(0u, + static_cast( + RecordsWithPrefix(lines, "UninlinedFunction:").size())); + + // The report stream also carries the module, so the same lines show that the + // helper has no body left to advertise. + for (const std::string &line : lines) { + if (line.compare(0, strlen("define "), "define ") == 0) { + VERIFY_IS_TRUE(line.find("ScaleHelper") == std::string::npos); + } + } +} + +// The runtime invokes a hull shader patch-constant function rather than the +// entry point does, so it survives inlining and needs instrumentation of its +// own. The helper that both of them call is inlined away, and the emitted DXIL +// validates. +TEST_F(PixTest, HelperInlining_HullPatchConstantFunctionValidates) { + auto compiled = + Compile(m_dllSupport, kHullShaderWithHelper, L"hs_6_2", {L"-Od"}); + auto output = RunDebugPass(compiled); + std::string disassembly = Disassemble(output.blob); + + // Two ranges are advertised, one of them the patch-constant function, so the + // helper is inlined away and nothing else is offered. + std::vector ranges = + RecordsWithPrefix(output.lines, "InstructionRange: "); + VERIFY_ARE_EQUAL(2u, static_cast(ranges.size())); + VERIFY_IS_TRUE(AnyLineContains(ranges, "PatchConstantFunction")); + VERIFY_IS_TRUE(AnyLineContains(ranges, "main hs")); + VERIFY_ARE_EQUAL( + 0u, static_cast( + RecordsWithPrefix(output.lines, "UninlinedFunction:").size())); + + VERIFY_ARE_EQUAL(0u, CountFunctionDefinitions(disassembly, "HullHelper")); + + // Both functions carry a selection prolog, so PIX receives trace records for + // either of them. + VERIFY_ARE_EQUAL(2u, CountOccurrences(disassembly, "PIXInterestingBlock:")); + + // OutputControlPointID is legal only in the control point phase, so the + // patch-constant function selects on the primitive alone. + std::string patchConstantBody = + ExtractFunctionBody(disassembly, "PatchConstantFunction"); + VERIFY_ARE_EQUAL( + 0u, CountCallsTo(patchConstantBody, "@dx.op.outputControlPointID.i32")); + VERIFY_ARE_EQUAL(1u, + CountCallsTo(patchConstantBody, "@dx.op.primitiveID.i32")); + + VerifyInstrumentedModuleIsValid( + output.blob, "debug instrumentation of a hull shader whose " + "patch-constant function is instrumented too"); +} + +// The non-uniform-resource-index pipeline shares the annotation prepass, so it +// also sees inlined shaders. A dynamic index inside a helper is still +// diagnosed, and the emitted DXIL validates. +TEST_F(PixTest, HelperInlining_NonUniformResourceIndexHelperValidates) { + const char *source = R"x( +Texture2D tex[8] : register(t0); + +[noinline] +float IndexInHelper(float u, float v) +{ + uint index = u * v; + return tex[index].Load(int3(0, 0, 0)).x; +} + +float4 main(float2 uv : TEXCOORD0) : SV_TARGET +{ + return IndexInHelper(uv.x, uv.y); +})x"; + + auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"}); + std::string outputText; + auto output = + RunDxilNonUniformResourceIndexInstrumentation(compiled, outputText); + std::string disassembly = Disassemble(output.blob); + + VERIFY_ARE_EQUAL(0u, CountFunctionDefinitions(disassembly, "IndexInHelper")); + + // The index is still reported, and the instrumentation is emitted. + VERIFY_IS_TRUE(outputText.find("FoundDynamicIndexingNoNuri") != + std::string::npos); + VERIFY_IS_TRUE( + outputText.find("NuriNotInstrumentedMissingInstructionNumber") == + std::string::npos); + VERIFY_ARE_EQUAL(1u, CountCallsTo(disassembly, "@dx.op.waveActiveAllEqual")); + + VerifyInstrumentedModuleIsValid( + output.blob, "non-uniform resource index instrumentation of a dynamic " + "index inside an inlined helper"); +} + +// The debug-break pipeline shares the annotation prepass too. A DebugBreak +// inside a helper is still found once the helper is part of the entry point, +// and the emitted DXIL validates. +TEST_F(PixTest, HelperInlining_DebugBreakHelperValidates) { + const char *source = R"x( +RWStructuredBuffer Output : register(u0); + +[noinline] +uint BreakInHelper(uint value) +{ + DebugBreak(); + return value + 1; +} + +[numthreads(1, 1, 1)] +void main(uint3 threadId : SV_DispatchThreadID) +{ + Output[0] = BreakInHelper(threadId.x); +})x"; + + if (m_ver.SkipDxilVersion(1, 10)) { + return; + } + + auto compiled = Compile(m_dllSupport, source, L"cs_6_10", {L"-Od"}); + auto output = RunDebugBreakPass(compiled); + std::string disassembly = Disassemble(output.blob); + + VERIFY_ARE_EQUAL(0u, CountFunctionDefinitions(disassembly, "BreakInHelper")); + + // The break is recorded from inside the entry point, and the original call is + // gone. + VERIFY_ARE_EQUAL(1u, CountCallsTo(disassembly, "@dx.op.atomicBinOp.i32")); + VERIFY_ARE_EQUAL(0u, CountCallsTo(disassembly, "@dx.op.debugBreak")); + + VerifyInstrumentedModuleIsValid( + output.blob, "debug-break instrumentation of a break inside an inlined " + "helper"); +} + +// The whole debug pipeline runs over a shader with a [noinline] helper. The +// helper becomes part of the entry point, one instruction range is advertised, +// the helper local is still traced, and the emitted DXIL validates. +TEST_F(PixTest, HelperInlining_InlinedHelperValidates) { + auto compiled = + Compile(m_dllSupport, kComputeShaderWithHelper, L"cs_6_2", {L"-Od"}); + auto output = RunDebugPass(compiled); + std::string disassembly = Disassemble(output.blob); + + // One function gives PIX one invocation identity. + std::vector ranges = + RecordsWithPrefix(output.lines, "InstructionRange: "); + VERIFY_ARE_EQUAL(1u, static_cast(ranges.size())); + VERIFY_IS_TRUE(ranges[0].find("main cs") != std::string::npos); + VERIFY_ARE_EQUAL( + 0u, static_cast( + RecordsWithPrefix(output.lines, "UninlinedFunction:").size())); + + VERIFY_ARE_EQUAL(0u, CountFunctionDefinitions(disassembly, "ScaleHelper")); + + VerifyInstrumentedModuleIsValid( + output.blob, "debug instrumentation of a shader whose [noinline] helper " + "is inlined away"); +}