Skip to content

Add numthreads to disassembly output#4484

Merged
bob80905 merged 6 commits intomicrosoft:mainfrom
bob80905:addNumthreads
May 31, 2022
Merged

Add numthreads to disassembly output#4484
bob80905 merged 6 commits intomicrosoft:mainfrom
bob80905:addNumthreads

Conversation

@bob80905
Copy link
Copy Markdown
Collaborator

Previously, for CS, MS, and AS shaders, the metadata at the top of the dxil disassembly would not mention anything about numthreads attributes for the respective shader, and there would be an obscure instruction in the dxil reflecting the numthreads configuration.
Now, numthreads configurations per shader (that supports it) will be seen in the disassembly spew at the top of the dxil output.

@bob80905 bob80905 requested review from gracejennings and pow2clk May 27, 2022 22:50
@bob80905 bob80905 requested a review from tex3d May 27, 2022 23:43
@AppVeyorBot
Copy link
Copy Markdown

@AppVeyorBot
Copy link
Copy Markdown

@bob80905 bob80905 changed the title Add numthreads Add numthreads to disassembly output May 31, 2022
Copy link
Copy Markdown
Collaborator

@pow2clk pow2clk left a comment

Choose a reason for hiding this comment

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

Thanks for updating the description!

@AppVeyorBot
Copy link
Copy Markdown

@bob80905
Copy link
Copy Markdown
Collaborator Author

This PR is for #4447

@bob80905 bob80905 merged commit 6962799 into microsoft:main May 31, 2022

const unsigned offset = sizeof(unsigned);
const PSVRuntimeInfo0 *pInfo = (const PSVRuntimeInfo0 *)(pBuffer + offset);
const PSVRuntimeInfo2 *pInfo2 = (const PSVRuntimeInfo2 *)(pBuffer + offset);
Copy link
Copy Markdown
Contributor

@tex3d tex3d May 31, 2022

Choose a reason for hiding this comment

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

The first DWORD that is skipped here by offset = sizeof(unsigned) is the size of the PSVRuntimeInfo structure. Only if it is >= sizeof(PSVRuntimeInfo2) should you be casting it to that.

In fact, this should use DxilPipelineStateValidation, since it handles all this. Here's how:

DxilPipelineStateValidation PSV;
PSV.InitFromPSV0(pBuffer, uBufferSize);
const PSVRuntimeInfo0 *pInfo0 = PSV.GetPSVRuntimeInfo0();
const PSVRuntimeInfo0 *pInfo1 = PSV.GetPSVRuntimeInfo1();
const PSVRuntimeInfo2 *pInfo2 = PSV.GetPSVRuntimeInfo2();
...
if (pInfo2) {
  OS << comment << " Numthreads=(" << pInfo2->NumThreadsX << "," << pInfo2->NumThreadsY << "," << pInfo2->NumThreadsZ << ")\n";
}
...
// Example - Mesh Shader output topology:
if (pInfo1) {
  OS << comment << " MeshOutputTopology=";
  DXIL::MeshOutputTopology topology = static_cast<DXIL::MeshOutputTopology>(pInfo1->MS1.MeshOutputTopology);
  switch (topology) {
  case DXIL::MeshOutputTopology::Undefined:
    OS << "undefined\n";
    break;
  case DXIL::MeshOutputTopology::Line:
    OS << "line\n";
    break;
  case DXIL::MeshOutputTopology::Triangle:
    OS << "triangle\n";
    break;
  default:
    OS << "invalid\n";
    break;
  }
}

This requires that size be passed in here, which should be available from the part size when PSV0 part was looked up.

edited to add MeshOutputTopology example and required pInfo1.
edited to add switch for enum and change style to Name=<value> to match the rest of the disassembler here.
Note: it would be nice to create nice enum to string functions instead of switch statements inline here, but this follows the pattern of the rest of the PSV decoding in the existing code.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants