Skip to content

[SPIR-V] Block semantic annotation reuse for input#6396

Merged
Keenuts merged 1 commit intomicrosoft:mainfrom
Keenuts:fix-3737
Mar 8, 2024
Merged

[SPIR-V] Block semantic annotation reuse for input#6396
Keenuts merged 1 commit intomicrosoft:mainfrom
Keenuts:fix-3737

Conversation

@Keenuts
Copy link
Collaborator

@Keenuts Keenuts commented Mar 7, 2024

The MSDN spec is not very clear regarding input parameter aliasing, BUT DXIL & MS agrees (See #3737) using the same semantic annotation twice should be disallowed.

DXIL currently disallows it for most, and due to a bug, allows some compute related semantics to be aliased.
SPIR-V did allowed aliasing, but it was a implemented as a hack causing typing issues if the type changed between the 2 parameters using the same semantic annotation.

To align more closely with DXIL, and the "spec", we are now disallowing all semantic attribute reuse for input parameters.

Fixes #3737

The MSDN spec is not very clear regarding input parameter
aliasing, BUT DXIL & MS agrees (See microsoft#3737) using the same
semantic annotation twice should be disallowed.

DXIL currently disallows it for most, and due to a bug, allows
some compute related semantics to be aliased.
SPIR-V did allowed aliasing, but it was a implemented as a hack causing
typing issues if the type changed between the 2 parameters using the
same semantic annotation.

To align more closely with DXIL, and the "spec", we are not disallowing
all semantic attribute reuse for input parameters.

Fixes microsoft#3737

Signed-off-by: Nathan Gauër <brioche@google.com>
Copy link
Collaborator

@sudonatalie sudonatalie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though it's a bit breaking, this seems reasonable to me. Thanks for all the background work looking into this.

To align more closely with DXIL, and the "spec", we are not disallowing all semantic attribute reuse for input parameters.

s/not/now?

@Keenuts
Copy link
Collaborator Author

Keenuts commented Mar 7, 2024

Even though it's a bit breaking, this seems reasonable to me. Thanks for all the background work looking into this.

If users complain, maybe we could limit that to some specific shader model like 6+ (as the SPIR-V doc does says we support all shader model < 5..)

To align more closely with DXIL, and the "spec", we are not disallowing all semantic attribute reuse for input parameters.

s/not/now?

Ah yes thanks!

@Keenuts Keenuts merged commit 1f162e2 into microsoft:main Mar 8, 2024
@Keenuts Keenuts deleted the fix-3737 branch March 8, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[SPIR-V] Reapeated SV_DomainLocation semantics don't produce an error and can crash the compiler (works with the DXIL backend)

3 participants