microsoft / microsoft/DirectXShaderCompiler
Atomics on RWTexture2DMS result in silent UB or ICE
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 3.7k
- Forks
- 900
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 44
Description
Description
The Shader Model 6.7 spec does not indicate whether atomics are supposed to work on multisampled textures when WritableMSAATexturesSupported is true. DXC does compile atomics when not explicitly specifying the sample index (e.g. InterlockedMax(tex[uv], ...)). It does not when you do specify it (e.g. InterlockedMax(tex.sample[s][uv], ...)), producing an internal compiler error.
After some investigation, the DXIL op textureStoreSample for non-atomic access was added in SM6.7 to supply the sample index (note: the parameters are not documented in DXIL.rst but it's easy enough to compare to plain textureStore), but atomicBinOp did not receive an equivalent additional variant. Thus, atomics on MS textures are currently impossible. The implicit sample 0 case that DXC does compile actually results in undefined behavior, where the resulting DXIL pushed through RGA will use an uninitialized register for the sample index instead of 0 (the hardware can actually do atomics on MS textures).
Steps to Reproduce
Godbolt link: https://hlsl.godbolt.org/z/jqG7GYfW5
Code in case it goes down (-T ps_6_7 -E PSMain):
struct PSInput {
uint2 uv : UV;
uint s : SAMPLE;
};
RWTexture2DMS<uint, 2> tex;
uint PSMain(PSInput input) : SV_Target0
{
uint value = 0xDEADBEEF;
uint old_val;
// atomicBinOp cannot pass a sample index. RGA passes an uninitialized register (v5)!
// This should be an error!
// Note that for a non-MS texture RGA only passes two registers for coordinates,
// so the hardware can actually do MS atomics (see Image Opcodes with No Sampler in ISA docs),
// but would require a new Shader Model since atomicBinOp can't.
InterlockedMax(tex[input.uv], value, old_val);
// error: cast<X>() argument of incompatible type!
// This should be a more user friendly error message.
// InterlockedMax(tex.sample[input.s][input.uv], value, old_val);
// --- Consistency checks, no bugs ---
// uses textureStoreSample with sample index 0 (correct)
tex[input.uv] = value;
// uses textureStoreSample with sample index s (correct)
tex.sample[input.s][input.uv] = value;
return old_val;
}
Desired Outcome
With current DXIL these atomics are not possible, so DXC should reject them with a clear error message instead of either silent UB or an internal compiler error. (Note: Do not forget about RWTexture2DMSArray)
Environment
- DXC version: 1.10.2605.24
- Host Operating System: Windows 11
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reproducing the PSMain examples from the Godbolt link with the stated Shader Model 6.7 target, then read DXIL.rst and compare the documented textureStore and textureStoreSample operations. Trace handling of InterlockedMax for RWTexture2DMS and RWTexture2DMSArray; done means both unsupported atomic forms are rejected with clear diagnostics rather than producing undefined behavior or an internal compiler error.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100