Skip to content

[PIX] Fix render-state visualization passes - #8850

Open
Damyan Pepper (damyanp) wants to merge 1 commit into
users/damyanp/pix-fixes-08from
users/damyanp/pix-fixes-09
Open

Damyan Pepper (damyanp) wants to merge 1 commit into
users/damyanp/pix-fixes-08from
users/damyanp/pix-fixes-09

Conversation

@damyanp

@damyanp Damyan Pepper (damyanp) commented Aug 27, 2026 •

Copy link
Copy Markdown
Member

Part 9 of 14 in the PIX instrumentation stack. It targets users/damyanp/pix-fixes-08. Its content depends on PR 2 for the declaration cleanup helper.

dx.op.storeOutput has four legal overloads: f16, f32, i16, and i32. DxilOutputColorBecomesConstant looks only for f32 and i32. A shader that writes SV_Target0 as min16float, min16int, half, or int16_t gets no constant-color instrumentation. The pass then asserts that the outputs are not mixed, and that assertion cannot tell "both types present" from "neither type recognised".

The pass builds a constant buffer for the colour, but it does not give that buffer a layout ValidateCBuffer can reach. The validator therefore refuses the instrumented module.

DxilReduceMSAAToSingleSample does not look through annotateHandle, and it does not accept a 16-bit texture load.

Only one overload can write SV_Target0, so there is no mixed case to resolve. PIX always uploads four 32-bit components, so the shader narrows them when the output is 16 bits. The upload format does not change.

Assisted-by: Copilot

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


Stack created with GitHub Stacks CLI • Give Feedback 💬

@damyanp
Damyan Pepper (damyanp) marked this pull request as ready for review August 27, 2026 23:45
Copilot AI balanced review requested due to automatic review settings August 27, 2026 23:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes PIX render-state visualization for 16-bit outputs, valid constant-buffer metadata, and modern MSAA handles.

Changes:

  • Supports all legal storeOutput overloads and 16-bit narrowing.
  • Adds valid constant-buffer layout annotations.
  • Handles SM 6.6 annotated resources and f16 MSAA loads.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp Adds 16-bit output support and valid buffer metadata.
lib/DxilPIXPasses/DxilReduceMSAAToSingleSample.cpp Recognizes annotated handles and all load overloads.
tools/clang/unittests/HLSL/PixTest.cpp Adds validation and MSAA regression tests.
tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionMRT.hlsl Tests min-precision MRT output.
tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionint.hlsl Tests min-precision integer output.
tools/clang/test/HLSLFileCheck/pix/constantcolorminprecisionFromCB.hlsl Tests min-precision buffer narrowing.
tools/clang/test/HLSLFileCheck/pix/constantcolorminprecision.hlsl Tests min-precision float literals.
tools/clang/test/HLSLFileCheck/pix/constantcolorint16.hlsl Tests native 16-bit integer output.
tools/clang/test/HLSLFileCheck/pix/constantcolorhalfMRTOnRTV1.hlsl Tests half output on RTV1.
tools/clang/test/HLSLFileCheck/pix/constantcolorhalfMRT.hlsl Tests half output on RTV0.
tools/clang/test/HLSLFileCheck/pix/constantcolorhalfFromCB.hlsl Tests native-half buffer narrowing.
tools/clang/test/HLSLFileCheck/pix/constantcolorhalf.hlsl Tests native-half literal replacement.

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

Comment on lines +271 to +273
: Builder.CreateTrunc(ReplacementColors[ChannelIndex],
OutputValueType,
NarrowedNames[ChannelIndex]);

unsigned RangeId =
cast<ConstantInt>(CreateHandle.get_rangeId())->getLimitedValue();
auto Resource = DM.GetSRV(RangeId);
Copilot AI review requested due to automatic review settings September 29, 2026 03:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Cleanup does not report all module mutations, and the integer constant-buffer narrowing branch lacks regression coverage.

Review effort: Balanced
Findings: 2 Medium severity · 5 Low severity

Open (7)

std::array<Function *, 4> &OutputFunctions;
~EraseUnusedOutputFunctionsOnExit() {
for (Function *OutputFunction : OutputFunctions) {
PIXPassHelpers::eraseIfUnused(DM, OutputFunction);
pCBuf->SetSize(4);
pCBuf->SetSize(ConstantColorCBufferSizeInBytes);

auto *StructAnnotation = DM.GetTypeSystem().GetStructAnnotation(CBStructTy);
bool DxilReduceMSAAToSingleSample::runOnModule(Module &M) {
DxilModule &DM = M.GetOrCreateDxilModule();
static bool IsMultisampledSRVHandle(Value *TextureHandle, DxilModule &DM) {
auto *TextureHandleInst = dyn_cast<CallInst>(TextureHandle);
Comment on lines +4040 to +4041
auto compiled = Compile(m_dllSupport, source, L"ps_6_0", {L"-Od"});
auto output = runSinglePass(compiled, L"-hlsl-dxil-constantColor,mod-mode=1");
Comment on lines +4060 to +4062
auto lines = Tokenize(Disassemble(pNewContainer).c_str(), "\n");
bool foundConstantColorCBuffer = false;
for (auto const &line : lines) {
Copilot AI review requested due to automatic review settings September 29, 2026 16:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +210 to +211
auto &FieldAnnotation =
StructAnnotation->GetFieldAnnotation(ComponentIndex);
Comment on lines 92 to +93
auto &FunctionUse = *FI++;
auto FunctionUser = FunctionUse.getUser();
auto instruction = cast<Instruction>(FunctionUser);
DxilInst_TextureLoad LoadInstruction(instruction);
auto TextureHandle = LoadInstruction.get_srv();
auto TextureHandleInst = cast<CallInst>(TextureHandle);
DxilInst_CreateHandle createHandle(TextureHandleInst);
// Dynamic rangeId is not supported
if (isa<ConstantInt>(createHandle.get_rangeId())) {
unsigned rangeId =
cast<ConstantInt>(createHandle.get_rangeId())->getLimitedValue();
if (static_cast<DXIL::ResourceClass>(
createHandle.get_resourceClass_val()) ==
DXIL::ResourceClass::SRV) {
auto Resource = DM.GetSRV(rangeId);
if (Resource.GetKind() == DXIL::ResourceKind::Texture2DMS ||
Resource.GetKind() == DXIL::ResourceKind::Texture2DMSArray) {
// "2" is the mip-level/sample-index operand index:
// https://github.com/Microsoft/DirectXShaderCompiler/blob/master/docs/DXIL.rst#textureload
instruction->setOperand(2, HlslOP->GetI32Const(0));
Modified = true;
}
}
auto *InstructionUser = dyn_cast<Instruction>(FunctionUse.getUser());
const char *textureLoadOverload,
const char *originalSampleIndex) {
bool foundTextureLoad = false;
for (auto const &line : lines) {
Copilot AI balanced review requested due to automatic review settings October 1, 2026 22:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DxilOutputColorBecomesConstant handles all storeOutput overloads (including 16-bit), gives its constant buffer a layout the validator accepts, and DxilReduceMSAAToSingleSample looks through annotateHandle and accepts 16-bit texture loads.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 40dc9de3-617e-4caf-ab0d-fba0a033ed93
Copilot AI balanced review requested due to automatic review settings October 1, 2026 23:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

3 participants