DirectXShaderCompiler icon indicating copy to clipboard operation
DirectXShaderCompiler copied to clipboard

SM6.7 MSAA UAVs only include sample count info in the DXIL in annotateHandle instruction args

Open jenatali opened this issue 3 years ago • 7 comments

Repro: https://godbolt.org/z/YahTzfb8h

This shader has 2 different MSAA UAV types, one with an implicit sample count of 0, and one with an explicit sample count of 4. Accessing these UAVs produces different annotateHandle instructions:

%6 = call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle %5, %dx.types.ResourceProperties { i32 4099, i32 777 })  ; AnnotateHandle(res,props)  resource: RWTexture2DMS<3xF32>
%15 = call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle %14, %dx.types.ResourceProperties { i32 4099, i32 262921 })  ; AnnotateHandle(res,props)  resource: RWTexture2DMS<3xF32>

But, the global resource metadata in !dx.resources is identical for the two:

!5 = !{i32 0, [100 x %"class.RWTexture2DMS<vector<float, 3>, 0>"]* undef, !"", i32 1, i32 5, i32 100, i32 3, i1 false, i1 false, i1 false, !6}
!6 = !{i32 0, i32 9}
!7 = !{i32 1, [100 x %"class.RWTexture2DMS<vector<float, 3>, 4>"]* undef, !"", i32 0, i32 5, i32 100, i32 3, i1 false, i1 false, i1 false, !6}

The 1 -> 0 difference is just the binding space. I would've expected to see a 0 -> 4, or perhaps instead of !6 the 4xMSAA resource would've pointed to a different metadata entry which includes the sample count in a tag pair, like:

!8 = !{i32 0, i32 9, i32 2, i32 4}

I only noticed because I was looking to synthesize the annotateHandle arguments by reading the global resource metadata, and saw that this particular bit of data isn't available there for SM6.7 MSAA UAVs.

jenatali avatar Aug 11 '22 19:08 jenatali

@jenatali - we'll need to schedule some time to investigate this further. Is there any urgency on this?

damyanp avatar Apr 29 '24 17:04 damyanp

No, not urgent, just seemed like an oversight.

jenatali avatar Apr 29 '24 17:04 jenatali

The metadata for UAVs doesn't have a field for sample count (it's an "SRV specific field" in https://github.com/microsoft/DirectXShaderCompiler/blob/main/docs/DXIL.rst#metadata-resource-records), so there isn't anywhere that could be updated. IIUC since SM6.6+ uses annotateHandle for all resources that should be considered the source of truth for this bit.

(note that writeable MSAA textures are new in SM6.7)

bogner avatar Apr 29 '24 18:04 bogner

It could (and probably should) be added as a UAV-specific tag pair.

jenatali avatar Apr 29 '24 19:04 jenatali

The annotate handle was meant to be a replacement for the metadata for the driver, but never quite got that far. I believe the issue is one of restoring global resources upon module load. Without this information captured in the metadata, these resources will not restore their sample count.

Since DXIL has already shipped in this form, we can't really change the metadata without bumping the DXIL version.

If you're trying to get correct sample counts for the global resources upon module load, we might instead have to run some code to reconstruct these from the annotate handle ops by matching resource handle creation operations to the global resources. This is unfortunate, but potentially the only way to fix the state in DxilModule's global resources when loading from a serialized module.

I'm curious why you're trying to synthesize annoateHandle arguments for a feature requiring a shader model where these should always already exist.

tex3d avatar Apr 29 '24 19:04 tex3d

Sorry, I wasn't clear. The specific use case here was in the Mesa DXIL writer. Given an input shader in a different intermediate form, I produce global resource metadata first, and then walk through the set of instructions in the shader. When producing handles to resources, I add in the annotation by referencing the global resource metadata to look up the info I need for the annotation. For MSAA UAVs, this information isn't present in the global resource metadata, and so needs to be looked up through a different mechanism. This is the only piece of information with this problem.

If the intention is to stop adding info to the global resource metadata, then that's fine. It just seemed like an oversight, which is why I filed the issue.

jenatali avatar Apr 29 '24 19:04 jenatali

This is something we could add in a future DXIL version, which might be a solution to your scenario. If it's added in a future version, you could add it to your intermediate version of the metadata before removing it or re-emitting metadata without it depending on the shader model target.

Though, since this is for intermediate use, you could alternatively add some other temporary metadata to preserve the information elsewhere if you don't want to have to rewrite the DXIL metadata later (just remove the temporary internal metadata when done).

Having a form independent from DXIL metadata to preserve anything you need is an approach DXC uses for HL metadata and upstream Clang uses for all information before lowering to DXIL. It makes sense when shader model and validator versions can constrain the DXIL metadata in ways that might drop information you need.

tex3d avatar Apr 29 '24 19:04 tex3d

@jenatali, @tex3d, @bogner - are we at all likely to consider this important to do before SM 7.0? If not, is there any reason to keep this issue open since it only really applies to DXIL?

damyanp avatar Jan 22 '25 21:01 damyanp

@jenatali, @tex3d, @bogner - are we at all likely to consider this important to do before SM 7.0? If not, is there any reason to keep this issue open since it only really applies to DXIL?

No, I don't think so.

tex3d avatar Jan 22 '25 23:01 tex3d