Skip to content

Commit

Permalink
CP: PIX: Deduplicate globals when referenced in multiple library fns (m…
Browse files Browse the repository at this point in the history
…icrosoft#6305) (microsoft#6313)

PIX's code for parsing debug data operates at the module level. When the
same global is referenced by multiple functions in a module, that
variable is referred to by multiple dbg.value/dbg.declare statements,
and those are mapped (by the PIX passes) to multiple fake allocas using
its usual scheme. This code was written before libraries were a thing,
and wasn't expecting this duplication. A little more attention to the
variable's scope fixes the issue.
Also, the changed code's original "return false" broke the whole process
of discovering variables with the results that PIX's shader debugger
locals window was completely empty. Makes more sense to ignore the one
variable and keep going.

(cherry picked from commit 64cdb9c)
  • Loading branch information
jeffnn authored Feb 15, 2024
1 parent eecba19 commit 476d344
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 5 deletions.
17 changes: 12 additions & 5 deletions lib/DxilDia/DxcPixLiveVariables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "DxcPixLiveVariables_FragmentIterator.h"
#include "DxilDiaSession.h"

#include "dxc/DXIL/DxilUtil.h"
#include "dxc/Support/Global.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/IR/DebugInfo.h"
Expand Down Expand Up @@ -296,12 +297,18 @@ HRESULT dxil_debug_info::LiveVariables::GetLiveVariablesAtInstruction(
S.AscendScopeHierarchy();
}
for (const auto &VarAndInfo : m_pImpl->m_LiveGlobalVarsDbgDeclare) {
if (!LiveVarsName.insert(VarAndInfo.first->getName()).second) {
// There shouldn't ever be a global variable with the same name,
// but it doesn't hurt to check
return false;
// Only consider references to the global variable that are in the same
// function as the instruction.
if (hlsl::dxilutil::DemangleFunctionName(
IP->getParent()->getParent()->getName()) ==
VarAndInfo.first->getScope()->getName()) {
if (!LiveVarsName.insert(VarAndInfo.first->getName()).second) {
// There shouldn't ever be a global variable with the same
// name, but it doesn't hurt to check
continue;
}
LiveVars.emplace_back(VarAndInfo.second.get());
}
LiveVars.emplace_back(VarAndInfo.second.get());
}
return CreateDxilLiveVariables(m_pImpl->m_pDxilDebugInfo, std::move(LiveVars),
ppResult);
Expand Down
48 changes: 48 additions & 0 deletions tools/clang/unittests/HLSL/PixDiaTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ class PixDiaTest {
DxcPixDxilDebugInfo_GlobalBackedGlobalStaticEmbeddedArrays_WithDbgValue)
TEST_METHOD(
DxcPixDxilDebugInfo_GlobalBackedGlobalStaticEmbeddedArrays_ArrayInValues)
TEST_METHOD(DxcPixDxilDebugInfo_DuplicateGlobals)
TEST_METHOD(DxcPixDxilDebugInfo_StructInheritance)
TEST_METHOD(DxcPixDxilDebugInfo_StructContainedResource)
TEST_METHOD(DxcPixDxilDebugInfo_StructStaticInit)
Expand Down Expand Up @@ -2227,6 +2228,53 @@ void main()
TestGlobalStaticCase(hlsl, L"lib_6_6", "float Accumulator", Expected);
}

int CountLiveGlobals(IDxcPixDxilLiveVariables *liveVariables) {
int globalCount = 0;
DWORD varCount;
VERIFY_SUCCEEDED(liveVariables->GetCount(&varCount));
for (DWORD i = 0; i < varCount; ++i) {
CComPtr<IDxcPixVariable> var;
VERIFY_SUCCEEDED(liveVariables->GetVariableByIndex(i, &var));
CComBSTR name;
VERIFY_SUCCEEDED(var->GetName(&name));
if (wcsstr(name, L"global.") != nullptr)
globalCount++;
}
return globalCount;
}

TEST_F(PixDiaTest, DxcPixDxilDebugInfo_DuplicateGlobals) {
if (m_ver.SkipDxilVersion(1, 6))
return;

const char *hlsl = R"(
static float global = 1.0;
struct RayPayload
{
float4 color;
};
typedef BuiltInTriangleIntersectionAttributes MyAttributes;
[shader("closesthit")]
void InnerClosestHitShader(inout RayPayload payload, in MyAttributes attr)
{
payload.color = float4(global, 0, 0, 0); // CHLine
}
[shader("miss")]
void MyMissShader(inout RayPayload payload)
{
payload.color = float4(0, 1, 0, 0); // MSLine
})";

auto dxilDebugger = CompileAndCreateDxcDebug(hlsl, L"lib_6_6").debugInfo;

auto CHVars = GetLiveVariablesAt(hlsl, "CHLine", dxilDebugger);
VERIFY_ARE_EQUAL(1, CountLiveGlobals(CHVars));
auto MSVars = GetLiveVariablesAt(hlsl, "MSLine", dxilDebugger);
VERIFY_ARE_EQUAL(0, CountLiveGlobals(MSVars));
}

TEST_F(PixDiaTest, DxcPixDxilDebugInfo_StructInheritance) {
if (m_ver.SkipDxilVersion(1, 5))
return;
Expand Down

0 comments on commit 476d344

Please sign in to comment.