[PIX] Inline helper functions before shader debugging

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<llvm::Function *, 4> 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: <n>
     // <fnName>"
     if (m_DM->GetShaderModel()->GetKind() == hlsl::ShaderModel::Kind::Library)

lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp

@@ -16,6 +16,7 @@
 #include <utility>
 
 #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;

lib/DxilPIXPasses/DxilDebugInstrumentation.cpp

@@ -377,6 +377,28 @@ class DxilDebugInstrumentation : public ModulePass {
   CountBlockPayloadBytes(std::vector<InstructionToInstrument> 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<Value *, 10> 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<llvm::Function *> 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<Value *, 16> RayQueryHandles;

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<llvm::Function *> *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<llvm::CallInst *, 8> callSites;
+      for (llvm::User *user : function->users()) {
+        if (auto *call = llvm::dyn_cast<llvm::CallInst>(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;

lib/DxilPIXPasses/PixPassHelpers.h

@@ -56,6 +56,36 @@ void EraseIfUnused(hlsl::DxilModule &DM, llvm::Function *OpFunction);
 void ClearViewIdState(hlsl::DxilModule &DM);
 std::vector<llvm::Function *>
 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<llvm::Function *> *UninlinedFunctions = nullptr);
 hlsl::DXIL::ShaderKind GetFunctionShaderKind(hlsl::DxilModule &DM,
                                              llvm::Function *fn);
 #ifdef PIX_DEBUG_DUMP_HELPER