Skip to content

Commit 476d344

Browse files
authored
CP: PIX: Deduplicate globals when referenced in multiple library fns (#6305) (#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)
1 parent eecba19 commit 476d344

2 files changed

Lines changed: 60 additions & 5 deletions

File tree

lib/DxilDia/DxcPixLiveVariables.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "DxcPixLiveVariables_FragmentIterator.h"
1818
#include "DxilDiaSession.h"
1919

20+
#include "dxc/DXIL/DxilUtil.h"
2021
#include "dxc/Support/Global.h"
2122
#include "llvm/ADT/SmallVector.h"
2223
#include "llvm/IR/DebugInfo.h"
@@ -296,12 +297,18 @@ HRESULT dxil_debug_info::LiveVariables::GetLiveVariablesAtInstruction(
296297
S.AscendScopeHierarchy();
297298
}
298299
for (const auto &VarAndInfo : m_pImpl->m_LiveGlobalVarsDbgDeclare) {
299-
if (!LiveVarsName.insert(VarAndInfo.first->getName()).second) {
300-
// There shouldn't ever be a global variable with the same name,
301-
// but it doesn't hurt to check
302-
return false;
300+
// Only consider references to the global variable that are in the same
301+
// function as the instruction.
302+
if (hlsl::dxilutil::DemangleFunctionName(
303+
IP->getParent()->getParent()->getName()) ==
304+
VarAndInfo.first->getScope()->getName()) {
305+
if (!LiveVarsName.insert(VarAndInfo.first->getName()).second) {
306+
// There shouldn't ever be a global variable with the same
307+
// name, but it doesn't hurt to check
308+
continue;
309+
}
310+
LiveVars.emplace_back(VarAndInfo.second.get());
303311
}
304-
LiveVars.emplace_back(VarAndInfo.second.get());
305312
}
306313
return CreateDxilLiveVariables(m_pImpl->m_pDxilDebugInfo, std::move(LiveVars),
307314
ppResult);

tools/clang/unittests/HLSL/PixDiaTest.cpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ class PixDiaTest {
168168
DxcPixDxilDebugInfo_GlobalBackedGlobalStaticEmbeddedArrays_WithDbgValue)
169169
TEST_METHOD(
170170
DxcPixDxilDebugInfo_GlobalBackedGlobalStaticEmbeddedArrays_ArrayInValues)
171+
TEST_METHOD(DxcPixDxilDebugInfo_DuplicateGlobals)
171172
TEST_METHOD(DxcPixDxilDebugInfo_StructInheritance)
172173
TEST_METHOD(DxcPixDxilDebugInfo_StructContainedResource)
173174
TEST_METHOD(DxcPixDxilDebugInfo_StructStaticInit)
@@ -2227,6 +2228,53 @@ void main()
22272228
TestGlobalStaticCase(hlsl, L"lib_6_6", "float Accumulator", Expected);
22282229
}
22292230

2231+
int CountLiveGlobals(IDxcPixDxilLiveVariables *liveVariables) {
2232+
int globalCount = 0;
2233+
DWORD varCount;
2234+
VERIFY_SUCCEEDED(liveVariables->GetCount(&varCount));
2235+
for (DWORD i = 0; i < varCount; ++i) {
2236+
CComPtr<IDxcPixVariable> var;
2237+
VERIFY_SUCCEEDED(liveVariables->GetVariableByIndex(i, &var));
2238+
CComBSTR name;
2239+
VERIFY_SUCCEEDED(var->GetName(&name));
2240+
if (wcsstr(name, L"global.") != nullptr)
2241+
globalCount++;
2242+
}
2243+
return globalCount;
2244+
}
2245+
2246+
TEST_F(PixDiaTest, DxcPixDxilDebugInfo_DuplicateGlobals) {
2247+
if (m_ver.SkipDxilVersion(1, 6))
2248+
return;
2249+
2250+
const char *hlsl = R"(
2251+
static float global = 1.0;
2252+
struct RayPayload
2253+
{
2254+
float4 color;
2255+
};
2256+
typedef BuiltInTriangleIntersectionAttributes MyAttributes;
2257+
2258+
[shader("closesthit")]
2259+
void InnerClosestHitShader(inout RayPayload payload, in MyAttributes attr)
2260+
{
2261+
payload.color = float4(global, 0, 0, 0); // CHLine
2262+
}
2263+
2264+
[shader("miss")]
2265+
void MyMissShader(inout RayPayload payload)
2266+
{
2267+
payload.color = float4(0, 1, 0, 0); // MSLine
2268+
})";
2269+
2270+
auto dxilDebugger = CompileAndCreateDxcDebug(hlsl, L"lib_6_6").debugInfo;
2271+
2272+
auto CHVars = GetLiveVariablesAt(hlsl, "CHLine", dxilDebugger);
2273+
VERIFY_ARE_EQUAL(1, CountLiveGlobals(CHVars));
2274+
auto MSVars = GetLiveVariablesAt(hlsl, "MSLine", dxilDebugger);
2275+
VERIFY_ARE_EQUAL(0, CountLiveGlobals(MSVars));
2276+
}
2277+
22302278
TEST_F(PixDiaTest, DxcPixDxilDebugInfo_StructInheritance) {
22312279
if (m_ver.SkipDxilVersion(1, 5))
22322280
return;

0 commit comments

Comments
 (0)