[PIX] Fix debug variable storage layout by damyanp · Pull Request #8851 · microsoft/DirectXShaderCompiler

Damyan Pepper (damyanp) · 2026-08-27T23:45:46Z

Part 10 of 14 in the PIX instrumentation stack. It targets users/damyanp/pix-fixes-09. Its content depends on PR 2 for the shared helpers.

The debug passes tell PIX where each variable lives in the shadow storage. Four defects make that information disagree with the shader.

The offset calculation ignores the full extent of an array, the padding in an aggregate, the position of a bitfield, and the layout of a matrix. An offset can therefore point into the storage of another variable.

A dynamically indexed write to an alloca takes its register span from the length of the array. DxilAnnotateWithVirtualRegister records the true span in !pix-alloca-reg-write. The two values disagree for an array of aggregates, so a dynamic index can be limited against the wrong bound.

IsAllocaRegisterWrite examines only one level of the ancestor GEP chain. A value three or more aggregates deep receives no metadata, so the debugger shows a stale value or no value. The function also uses a member index without a bound check and without a safe cast.

The pass finds the storage of an embedded array by the name of the debug variable. DXC can flatten a multi-dimensional array and rename the module global. The debug variable keeps its name, so the lookup misses the global and every shadow store for that array is dropped.

Two decisions are worth attention. IsAllocaRegisterWrite returns false for a member index that is not constant or is out of range, because a guess would attach metadata the debug record cannot honour. A backward move of the layout writes a message and continues, and a debug build does not stop the process.

Assisted-by: Copilot

This changes only the PIX instrumentation, so it needs no release note.

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

github-actions · 2026-08-27T23:46:39Z

✅ With the latest revision this PR passed the C/C++ code formatter.

Copilot

Pull request overview

Fixes PIX shadow-storage metadata so debugger variable locations better match shader layouts.

Changes:

Corrects aggregate, array, bitfield, matrix, and constant-value handling.
Uses annotated register spans and traverses deeper GEP chains.
Adds extensive PIX regression coverage.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`lib/DxilPIXPasses/DxilDebugInstrumentation.cpp`	Uses metadata register spans for dynamic writes.
`lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp`	Corrects shadow-storage layout and global-array handling.
`lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp`	Traverses ancestor GEP chains.
`tools/clang/unittests/HLSL/PixTest.cpp`	Adds pass-level regression tests.
`tools/clang/test/HLSLFileCheck/pix/DebugInstrumentation_dynamic_index_span.hlsl`	Tests dynamic-index payload sizing.
`tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_nested_aggregate_padding.hlsl`	Tests nested padding.
`tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_multidim_array.hlsl`	Tests flattened multidimensional arrays.
`tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_mixed_width_bitfields.hlsl`	Tests mixed-width bitfields.
`tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_matrix_after_padding.hlsl`	Tests matrix placement after padding.
`tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_front_and_tail_padding.hlsl`	Tests front and tail padding.
`tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_constant_local.hlsl`	Tests constant local shadow stores.
`tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_array_of_padded_structs.hlsl`	Tests arrays of padded structures.
`tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_aggregate_tail_padding.hlsl`	Tests aggregate tail padding.
`tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_16bit_tail_padding.hlsl`	Tests 16-bit tail padding.
`tools/clang/test/HLSLFileCheck/pix/DbgValueToDbgDeclare_16bit_bitfields.hlsl`	Tests 16-bit bitfields.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+          // The register span for a dynamic write comes from the
+          // !pix-alloca-reg-write metadata the annotation pass attached to
+          // this instruction, so it always matches the virtual-register
+          // numbering the annotation pass assigned.
+          uint32_t MaxArraySize = std::max(1u, IandT->AllocaRegisterSize);


+      llvm::DIType *OtherTy =
+          OtherDbgValue->getVariable()->getType().resolve(EmptyMap);
+      if (OtherTy != nullptr && DITypePeelTypeAlias(OtherTy) == UnaliasedTy) {
+        return true;


+        uint64_t memberIndex = pStructMember->getLimitedValue();
+        if (memberIndex > pStructType->getStructNumElements()) {
+          return false;
+        }


+      for (auto *pAncestorGEP : AncestorGEPs) {
+        auto *pStructType = llvm::dyn_cast<llvm::StructType>(
+            pAncestorGEP->getPointerOperandType()->getPointerElementType());
+        if (pStructType == nullptr) {
+          continue;


Copilot

Copilot review overview

🔵 Needs a closer look

Array-of-aggregate spans and chained array GEP offsets remain incorrect, and same-typed variables can lose debug records.

Review effort: Balanced
Findings: 4

Open (4)

Walking the complete chain still silently ignores ancestor GEPs whose source type is an array (and…
The upper bound is exclusive. An index equal to getStructNumElements() is already out of range,…
Matching only the peeled composite type can discard a legitimate constant update for an unrelated…
This assumes the metadata contains the flattened register span, but AnnotateAlloca still records…

Copilot

Copilot review overview

🟡 Changes recommended

Ancestor offsets, composite-copy detection, bounds handling, and malformed-layout behavior remain incorrect.

Review effort: Balanced
Findings: 1 · 4

Open (5)

Prevent assertions for overlapping debug layout ranges · New
Walking the complete chain still silently ignores ancestor GEPs whose source type is an array (and…
The upper bound is exclusive. An index equal to getStructNumElements() is already out of range,…
Matching only the peeled composite type can discard a legitimate constant update for an unrelated…
This assumes the metadata contains the flattened register span, but AnnotateAlloca still records…

+    } else if (AlignedOffset < m_CurrentAlignedOffset) {
+      VALUE_TO_DECLARE_LOG("Refusing to move aligned offset back from %d to %d",
+                           m_CurrentAlignedOffset, AlignedOffset);
+      // Keep existing mappings monotonic when debug information overlaps.
+      return;


Copilot

Copilot review overview

🟡 Changes recommended

Ancestor offsets and composite-copy detection can still produce incorrect or order-dependent shadow mappings.

Review effort: Balanced
Findings: 1 · 5 · 1

Open (7)

Prevent assertions for overlapping debug layout ranges
Precompute dbg.value associations before erasing instructions · New
Walking the complete chain still silently ignores ancestor GEPs whose source type is an array (and…
The upper bound is exclusive. An index equal to getStructNumElements() is already out of range,…
Matching only the peeled composite type can discard a legitimate constant update for an unrelated…
This assumes the metadata contains the flattened register span, but AnnotateAlloca still records…
Add release notes for PIX debugger fixes · New

+  for (llvm::BasicBlock &Block : *DbgValue->getParent()->getParent()) {
+    for (llvm::Instruction &Instruction : Block) {
+      auto *OtherDbgValue = llvm::dyn_cast<llvm::DbgValueInst>(&Instruction);
+      if (OtherDbgValue == nullptr || OtherDbgValue == DbgValue ||
+          !llvm::isa<llvm::AllocaInst>(OtherDbgValue->getValue())) {


+          // !pix-alloca-reg-write metadata the annotation pass attached to
+          // this instruction, so it always matches the virtual-register
+          // numbering the annotation pass assigned.
+          uint32_t MaxArraySize = std::max(1u, IandT->AllocaRegisterSize);


Copilot

Copilot review overview

🔵 Needs a closer look

Deep non-struct GEPs can still receive incorrect metadata, and composite matching can suppress unrelated variables.

Review effort: Balanced
Findings: 1 · 5 · 1

Open (7)

Prevent assertions for overlapping debug layout ranges
Precompute dbg.value associations before erasing instructions
Walking the complete chain still silently ignores ancestor GEPs whose source type is an array (and…
The upper bound is exclusive. An index equal to getStructNumElements() is already out of range,…
Matching only the peeled composite type can discard a legitimate constant update for an unrelated…
This assumes the metadata contains the flattened register span, but AnnotateAlloca still records…
Add release notes for PIX debugger fixes

Copilot

Copilot review overview

🔵 Needs a closer look

Ancestor array offsets and distinct same-typed composite variables can still produce incorrect shadow-storage metadata.

Review effort: Balanced
Findings: 1 · 5 · 1

Open (7)

Prevent assertions for overlapping debug layout ranges
Precompute dbg.value associations before erasing instructions
Walking the complete chain still silently ignores ancestor GEPs whose source type is an array (and…
The upper bound is exclusive. An index equal to getStructNumElements() is already out of range,…
Matching only the peeled composite type can discard a legitimate constant update for an unrelated…
This assumes the metadata contains the flattened register span, but AnnotateAlloca still records…
Add release notes for PIX debugger fixes

Copilot

Copilot review overview

🔵 Needs a closer look

Nested array offsets, aggregate register spans, variable matching, and one bounds check remain incorrect.

Review effort: Balanced
Findings: 1 · 5 · 1

Open (7)

Prevent assertions for overlapping debug layout ranges
Precompute dbg.value associations before erasing instructions
Walking the complete chain still silently ignores ancestor GEPs whose source type is an array (and…
The upper bound is exclusive. An index equal to getStructNumElements() is already out of range,…
Matching only the peeled composite type can discard a legitimate constant update for an unrelated…
This assumes the metadata contains the flattened register span, but AnnotateAlloca still records…
Add release notes for PIX debugger fixes

Copilot

🔵 Needs a closer look

Nested GEP offsets, same-type variable matching, and an off-by-one bounds check can still produce incorrect metadata.

7 open findings

Prevent assertions for overlapping debug layout ranges
Precompute dbg.value associations before erasing instructions
Walking the complete chain still silently ignores ancestor GEPs whose source type is an array (and…
The upper bound is exclusive. An index equal to getStructNumElements() is already out of range,…
Matching only the peeled composite type can discard a legitimate constant update for an unrelated…
This assumes the metadata contains the flattened register span, but AnnotateAlloca still records…
Add release notes for PIX debugger fixes

🧠 Review effort: Balanced

Correct shadow-storage offsets for arrays, aggregate padding, bitfields and matrices; use the true span for dynamically indexed alloca writes; handle deep GEP chains safely in IsAllocaRegisterWrite; and find embedded-array storage even when DXC renames the flattened global. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 40dc9de3-617e-4caf-ab0d-fba0a033ed93

Copilot

🟡 Changes recommended

Dynamic aggregate spans and indices remain incorrect, and unbounded debug arrays can trigger excessive traversal.

8 open findings

🧠 Review effort: Balanced

+          TotalElementCount *= Subrange->getCount();
+          FoundSubrange = true;


github-project-automation Bot added this to HLSL Roadmap Aug 27, 2026

github-project-automation Bot moved this to New in HLSL Roadmap Aug 27, 2026

Damyan Pepper (damyanp) requested a review from Austin Kinross (austinkinross) August 27, 2026 23:45

Austin Kinross (austinkinross) approved these changes Aug 27, 2026

View reviewed changes

Damyan Pepper (damyanp) force-pushed the users/damyanp/pix-fixes-10 branch from 85d2877 to 8658126 Compare August 27, 2026 23:53

Damyan Pepper (damyanp) marked this pull request as ready for review August 27, 2026 23:54

Copilot AI balanced review requested due to automatic review settings August 27, 2026 23:54

Copilot started reviewing on behalf of Damyan Pepper (damyanp) August 27, 2026 23:55 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 29, 2026 03:29

Copilot started reviewing on behalf of Damyan Pepper (damyanp) September 29, 2026 03:30 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 29, 2026 16:31

Copilot started reviewing on behalf of Damyan Pepper (damyanp) September 29, 2026 16:32 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings September 30, 2026 00:08

Copilot started reviewing on behalf of Damyan Pepper (damyanp) September 30, 2026 00:09 View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 1, 2026 22:03

Damyan Pepper (damyanp) force-pushed the users/damyanp/pix-fixes-10 branch from 4c8dd48 to 2dbfd94 Compare October 1, 2026 22:03

Copilot started reviewing on behalf of Damyan Pepper (damyanp) October 1, 2026 22:04 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 1, 2026 23:11

Damyan Pepper (damyanp) force-pushed the users/damyanp/pix-fixes-10 branch from 2dbfd94 to 4220976 Compare October 1, 2026 23:11

Copilot started reviewing on behalf of Damyan Pepper (damyanp) October 1, 2026 23:12 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 6, 2026 21:00

Damyan Pepper (damyanp) force-pushed the users/damyanp/pix-fixes-10 branch from 4220976 to bb8531d Compare October 6, 2026 21:00

Copilot started reviewing on behalf of Damyan Pepper (damyanp) October 6, 2026 21:01 View session

Copilot AI reviewed Oct 6, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 7, 2026 18:21

Damyan Pepper (damyanp) force-pushed the users/damyanp/pix-fixes-10 branch from bb8531d to 3eff4d5 Compare October 7, 2026 18:21

Copilot started reviewing on behalf of Damyan Pepper (damyanp) October 7, 2026 18:21 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 9, 2026 17:06

Damyan Pepper (damyanp) force-pushed the users/damyanp/pix-fixes-10 branch from 3eff4d5 to 3d1b7f1 Compare October 9, 2026 17:06

Copilot started reviewing on behalf of Damyan Pepper (damyanp) October 9, 2026 17:07 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread lib/DxilPIXPasses/DxilDbgValueToDbgDeclare.cpp

Comment on lines +469 to +470

TotalElementCount *= Subrange->getCount();

FoundSubrange = true;

Navigation Menu

Repository navigation

[PIX] Fix debug variable storage layout - #8851

[PIX] Fix debug variable storage layout#8851
Damyan Pepper (damyanp) wants to merge 1 commit into
users/damyanp/pix-fixes-09from
users/damyanp/pix-fixes-10

Damyan Pepper (damyanp) commented Aug 27, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		TotalElementCount *= Subrange->getCount();
		FoundSubrange = true;

Conversation

Damyan Pepper (damyanp) commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Damyan Pepper (damyanp) commented Aug 27, 2026 •

edited

Loading

github-actions Bot commented Aug 27, 2026 •

edited

Loading