Skip to content

Commit 64cdb9c

Browse files
authored
PIX: Deduplicate globals when referenced in multiple library fns (microsoft#6305)
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.
1 parent 8497f1d commit 64cdb9c

File tree

2 files changed

+60
-5
lines changed

2 files changed

+60
-5
lines changed

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)
@@ -2229,6 +2230,53 @@ void main()
22292230
TestGlobalStaticCase(hlsl, L"lib_6_6", "float Accumulator", Expected);
22302231
}
22312232

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

0 commit comments

Comments
 (0)