Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load/Store intrinsics for Raw and Typed buffers are inconsistent in allowing vec1 indexing #7079

Open
pow2clk opened this issue Jan 22, 2025 · 1 comment · May be fixed by #7114 or #7115
Open

Load/Store intrinsics for Raw and Typed buffers are inconsistent in allowing vec1 indexing #7079

pow2clk opened this issue Jan 22, 2025 · 1 comment · May be fixed by #7114 or #7115
Assignees
Labels
bug Bug, regression, crash

Comments

@pow2clk
Copy link
Member

pow2clk commented Jan 22, 2025

Description
Equivalent intrinsics on different buffers and sometimes within the same buffer kind will accept or reject single-element vectors as indices with no consistency.

Since the clang implementation will implicitly truncate such vectors to scalars in all cases, the most forward compatible solution is to allow these in all the cases.

Steps to Reproduce
https://godbolt.org/z/Y7cYo5vPs includes a long list of buffers with their full list of load/store operations used with int1 indices, which can be toggled to scalars with the change of a single #if 0 on line 9.

These are pretty simple to reproduce though, any shader that passes in a vec1, however trivial to eliminate will fail if the method in question isn't among those that happen to be supported.

Actual Behavior
Structured buffers and typed buffers accept vec1s for subscripts whether used for retrieval or assignment. ByteAddress and Structured buffers both reject them for any Load or Store method.
Typed buffers accept vec1s in all cases except for RWBuffer Loads. This is simply because of how they are defined in gen_intrin_main.txt.

Environment

  • DXC version 1.8.2407
  • Host Operating System MacOs 14.7.2 Also Linux something(dot)something. Yes, that sounds right.
@pow2clk pow2clk added bug Bug, regression, crash needs-triage Awaiting triage labels Jan 22, 2025
@pow2clk pow2clk added this to the Release 1.8.2502 milestone Jan 22, 2025
@pow2clk pow2clk self-assigned this Jan 22, 2025
@pow2clk
Copy link
Member Author

pow2clk commented Jan 22, 2025

Sidenote, this was an obstacle to getting sensible testing for long vectors or it wouldn't have been important enough to pay attention to in DXC.

@pow2clk pow2clk moved this to Active in HLSL Support Jan 22, 2025
@damyanp damyanp moved this to Triaged in HLSL Triage Jan 22, 2025
@damyanp damyanp removed the needs-triage Awaiting triage label Jan 22, 2025
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 3, 2025
Allow truncations when matching arguments for intrinsic overloads. This eliminates the need for explicit scalar extractions from vectors for arguments that are scalar by nature. This encompasses any vectors passed for scalars, allowing the truncation, but emitting a warning the same as is done for other assignments of vectors to scalars.

This maintains splats as the preferred transformations and promotes perfect matches to be preferred over that. This has the effect of removing the need to carefully order intrinsics to ensure that the right variant gets matched first before another one incorrectly takes its place with a faulty cast.

Allowing truncations causes a problems with a small subset of intrinsics that have explicit overloads for various matrix,vector, scalar combinations. Namely the mul overloads. These could be simplified to accept a new range of template types except the dimensions need to be matched in unconventional ways.

For these, the notion of uncastable or "ONLY" variants of the template/layout types are introduced. These are indicated with a trailing "!" after the parameter typename in gen_intrin_main, which directs them to an array that contains a NOCAST enum that, when encountered, will skip the attempts to splat or truncate.

Fixes microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 3, 2025
Load operations previously split out the addr into separate components in individual locations where they were required. This was error prone as it had a lot of places that were missing. This changes the contents of resload and atomic helpers to contain the broken down coordinates instead of the individual Value. This catches a number of new locations, but it requires the intrinsic overloads to be defined as vec1s to handle that case and it still involves a lot of fiddly places for the code that don't produce truncation warnings when longer vectors are used. Not recommended, but functional.

Fixes microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 3, 2025
Allow truncations when matching arguments for intrinsic overloads. This eliminates the need for explicit scalar extractions from vectors for arguments that are scalar by nature. This encompasses any vectors passed for scalars, allowing the truncation, but emitting a warning the same as is done for other assignments of vectors to scalars.

This maintains splats as the preferred transformations and promotes perfect matches to be preferred over that. This has the effect of removing the need to carefully order intrinsics to ensure that the right variant gets matched first before another one incorrectly takes its place with a faulty cast.

Allowing truncations causes a problems with a small subset of intrinsics that have explicit overloads for various matrix,vector, scalar combinations. Namely the mul overloads. These could be simplified to accept a new range of template types except the dimensions need to be matched in unconventional ways.

For these, the notion of uncastable or "ONLY" variants of the template/layout types are introduced. These are indicated with a trailing "!" after the parameter typename in gen_intrin_main, which directs them to an array that contains a NOCAST enum that, when encountered, will skip the attempts to splat or truncate.

Fixes microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 3, 2025
It was producing a validation error previously because they are represented as u32 and i1 is not valid for output resources. This preserves the cmp conversions for values read from the resource, but satisfies the validation requirement. Performs some incidental consolidation of types that are represented as u32.

Related to microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 3, 2025
It was producing a validation error previously because they are represented as u32 and i1 is not valid for output resources. This preserves the cmp conversions for values read from the resource, but satisfies the validation requirement. Performs some incidental consolidation of types that are represented as u32.

Related to microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 6, 2025
Allow truncations when matching arguments for intrinsic overloads. This eliminates the need for explicit scalar extractions from vectors for arguments that are scalar by nature. This encompasses any vectors passed for scalars, allowing the truncation, but emitting a warning the same as is done for other assignments of vectors to scalars.

This maintains splats as the preferred transformations and promotes perfect matches to be preferred over that. This has the effect of removing the need to carefully order intrinsics to ensure that the right variant gets matched first before another one incorrectly takes its place with a faulty cast.

Allowing truncations causes a problems with a small subset of intrinsics that have explicit overloads for various matrix,vector, scalar combinations. Namely the mul overloads. These could be simplified to accept a new range of template types except the dimensions need to be matched in unconventional ways.

For these, the notion of uncastable or "ONLY" variants of the template/layout types are introduced. These are indicated with a trailing "!" after the parameter typename in gen_intrin_main, which directs them to an array that contains a NOCAST enum that, when encountered, will skip the attempts to splat or truncate.

Fixes microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 9, 2025
Allow truncations when matching arguments for intrinsic overloads. This eliminates the need for explicit scalar extractions from vectors for arguments that are scalar by nature. This encompasses any vectors passed for scalars, allowing the truncation, but emitting a warning the same as is done for other assignments of vectors to scalars.

This maintains splats as the preferred transformations and promotes perfect matches to be preferred over that. This has the effect of removing the need to carefully order intrinsics to ensure that the right variant gets matched first before another one incorrectly takes its place with a faulty cast.

Allowing truncations causes a problems with a small subset of intrinsics that have explicit overloads for various matrix,vector, scalar combinations. Namely the mul overloads. These could be simplified to accept a new range of template types except the dimensions need to be matched in unconventional ways.

For these, the notion of uncastable or "ONLY" variants of the template/layout types are introduced. These are indicated with a trailing "!" after the parameter typename in gen_intrin_main, which directs them to an array that contains a NOCAST enum that, when encountered, will skip the attempts to splat or truncate.

Fixes microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 9, 2025
It was producing a validation error previously because they are represented as u32 and i1 is not valid for output resources. This preserves the cmp conversions for values read from the resource, but satisfies the validation requirement. Performs some incidental consolidation of types that are represented as u32.

Related to microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 9, 2025
Allow truncations when matching arguments for intrinsic overloads. This eliminates the need for explicit scalar extractions from vectors for arguments that are scalar by nature. This encompasses any vectors passed for scalars, allowing the truncation, but emitting a warning the same as is done for other assignments of vectors to scalars.

This maintains splats as the preferred transformations and promotes perfect matches to be preferred over that. This has the effect of removing the need to carefully order intrinsics to ensure that the right variant gets matched first before another one incorrectly takes its place with a faulty cast.

Allowing truncations causes a problems with a small subset of intrinsics that have explicit overloads for various matrix,vector, scalar combinations. Namely the mul overloads. These could be simplified to accept a new range of template types except the dimensions need to be matched in unconventional ways.

For these, the notion of uncastable or "ONLY" variants of the template/layout types are introduced. These are indicated with a trailing "!" after the parameter typename in gen_intrin_main, which directs them to an array that contains a NOCAST enum that, when encountered, will skip the attempts to splat or truncate.

Fixes microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 9, 2025
It was producing a validation error previously because they are represented as u32 and i1 is not valid for output resources. This preserves the cmp conversions for values read from the resource, but satisfies the validation requirement. Performs some incidental consolidation of types that are represented as u32.

Related to microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 12, 2025
Allow truncations when matching arguments for intrinsic overloads. This eliminates the need for explicit scalar extractions from vectors for arguments that are scalar by nature. This encompasses any vectors passed for scalars, allowing the truncation, but emitting a warning the same as is done for other assignments of vectors to scalars.

This maintains splats as the preferred transformations and promotes perfect matches to be preferred over that. This has the effect of removing the need to carefully order intrinsics to ensure that the right variant gets matched first before another one incorrectly takes its place with a faulty cast.

Allowing truncations causes a problems with a small subset of intrinsics that have explicit overloads for various matrix,vector, scalar combinations. Namely the mul overloads. These could be simplified to accept a new range of template types except the dimensions need to be matched in unconventional ways.

For these, the notion of uncastable or "ONLY" variants of the template/layout types are introduced. These are indicated with a trailing "!" after the parameter typename in gen_intrin_main, which directs them to an array that contains a NOCAST enum that, when encountered, will skip the attempts to splat or truncate.

Fixes microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 12, 2025
It was producing a validation error previously because they are represented as u32 and i1 is not valid for output resources. This preserves the cmp conversions for values read from the resource, but satisfies the validation requirement. Performs some incidental consolidation of types that are represented as u32.

Related to microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 13, 2025
Allow truncations when matching arguments for intrinsic overloads. This eliminates the need for explicit scalar extractions from vectors for arguments that are scalar by nature. This encompasses any vectors passed for scalars, allowing the truncation, but emitting a warning the same as is done for other assignments of vectors to scalars.

This maintains splats as the preferred transformations and promotes perfect matches to be preferred over that. This has the effect of removing the need to carefully order intrinsics to ensure that the right variant gets matched first before another one incorrectly takes its place with a faulty cast.

Allowing truncations causes a problems with a small subset of intrinsics that have explicit overloads for various matrix,vector, scalar combinations. Namely the mul overloads. These could be simplified to accept a new range of template types except the dimensions need to be matched in unconventional ways.

For these, the notion of uncastable or "ONLY" variants of the template/layout types are introduced. These are indicated with a trailing "!" after the parameter typename in gen_intrin_main, which directs them to an array that contains a NOCAST enum that, when encountered, will skip the attempts to splat or truncate.

Fixes microsoft#7079
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Feb 13, 2025
It was producing a validation error previously because they are represented as u32 and i1 is not valid for output resources. This preserves the cmp conversions for values read from the resource, but satisfies the validation requirement. Performs some incidental consolidation of types that are represented as u32.

Related to microsoft#7079

remove dead array handing code

The array type checks were for a type extracted from a vector. Arrays cannot be elements of vectors. This is leftover from when that was not the case. The code shows as never executed in coverage
pow2clk added a commit that referenced this issue Feb 25, 2025
It was producing a validation error previously because they are
represented as u32 and i1 is not valid for output resources. This
preserves the cmp conversions for values read from the resource, but
satisfies the validation requirement. Performs some incidental
consolidation of types that are represented as u32.

Includes a couple incidental NFC fixes as well:

Moves lit filecheck tests into the correct directory. I got CodeGenHLSL
and CodeGenDXIL mixed up.

Remove dead array handing code. The array type checks were for a type
extracted from a vector. Arrays cannot be elements of vectors. This is
leftover from when the type came from more sources. The code shows as
never executed in coverage.

Related to #7079
bob80905 pushed a commit to bob80905/DirectXShaderCompiler that referenced this issue Feb 26, 2025
It was producing a validation error previously because they are
represented as u32 and i1 is not valid for output resources. This
preserves the cmp conversions for values read from the resource, but
satisfies the validation requirement. Performs some incidental
consolidation of types that are represented as u32.

Includes a couple incidental NFC fixes as well:

Moves lit filecheck tests into the correct directory. I got CodeGenHLSL
and CodeGenDXIL mixed up.

Remove dead array handing code. The array type checks were for a type
extracted from a vector. Arrays cannot be elements of vectors. This is
leftover from when the type came from more sources. The code shows as
never executed in coverage.

Related to microsoft#7079
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, regression, crash
Projects
Status: Triaged
2 participants