Mark SetMeshOutputCounts as noduplicate#8438
Conversation
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
|
@tex3d, can you confirm my understanding that the |
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 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. |
|
The concern I have about doing this only on the HL opcode is that while that prevents jump threading from performing the invalid operation, but not other passes that we run after DXIL generation. SimplifyCFG for example may also duplicate function calls to collapse control flow, and we run that both before and after DXIL generation. I also worry that IHV drivers may do the wrong thing in their own optimizers based on the attributes being incorrect. Since I think all the driver vendors read DXIL with LLVM-based bitcode readers I have a hard time imagining this causing problems. I even know of game engines that are baking in custom bitcode abbreviations that aren't technically valid in DXIL (although the DXIL validator and any LLVM-based reader actually don't care). This may warrant some testing and outreach to IHVs to confirm. |
2 participants
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