Skip to content

Mark SetMeshOutputCounts as noduplicate#8438

Open
llvm-beanz wants to merge 1 commit into
microsoft:mainfrom
llvm-beanz:cbieneman/8104
Open

Mark SetMeshOutputCounts as noduplicate#8438
llvm-beanz wants to merge 1 commit into
microsoft:mainfrom
llvm-beanz:cbieneman/8104

Conversation

@llvm-beanz
Copy link
Copy Markdown
Collaborator

The SetMeshOutputCounts function can only appear once in generated DXIL, so it should not be legal for the compiler to duplicate the function through any optimization pass.

In the reproduction case attached to the issue, the JumpThreading pass duplicates calls to SetMeshOutputCounts, this can be prevented by marking the HL and DXIL operations with noduplicate, which is a trivial change.

Fixes #8104

The SetMeshOutputCounts function can only appear once in generated
DXIL, so it should not be legal for the compiler to duplicate the
function through any optimization pass.

In the reproduction case attached to the issue, the JumpThreading pass
duplicates calls to SetMeshOutputCounts, this can be prevented by
marking the HL and DXIL operations with noduplicate, which is a trivial
change.

Fixes microsoft#8104
@llvm-beanz
Copy link
Copy Markdown
Collaborator Author

@tex3d, can you confirm my understanding that the noduplicate attribute is valid in DXIL so this change should be safe even tough we didn't previously attach the attribute to this operation?

@tex3d
Copy link
Copy Markdown
Contributor

tex3d commented May 11, 2026

@tex3d, can you confirm my understanding that the noduplicate attribute is valid in DXIL so this change should be safe even tough we didn't previously attach the attribute to this operation?

Yes, it's a valid attribute, however it will change the function signature for existing DXIL consumers, which would be a problem I think.

We could potentially mitigate that by stripping the attribute at the end during the DxilFinalizeModule pass in DxilPreparePasses.cpp for DXIL 1.9 and below. Validator version gating would also be necessary.

But this might run into further internal conflicts (when loaded by a different version), since the DxilOperation system isn't designed to handle ops that may be defined in two different ways like this.

Is the attribute on the HL op sufficient to prevent DXC from introducing the problem? Or is it required on the DXIL op as well? I see that we run the JumpThreading pass on HL code, not DXIL, so it seems like it should be, however your JumpThreading test is run on the DXIL op instead.

My recommended fix is to only add it to the HL op, test the HL op instead of the DXIL op for the JumpThreading pass, and we can note this limitation (that the DXIL op lacks the attribute) for our output. We could probably rev the OP to add the attribute somehow in the future, but I think that will require some more careful changes and testing work.

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.

DXC can sink and duplicate SetMeshOutputCounts if the inputs are computed within a scalar branch

2 participants