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

Dot products on unsigned integer vectors do not use umad #7058

Open
pow2clk opened this issue Jan 13, 2025 · 0 comments · May be fixed by #7059
Open

Dot products on unsigned integer vectors do not use umad #7058

pow2clk opened this issue Jan 13, 2025 · 0 comments · May be fixed by #7059
Assignees
Labels
bug Bug, regression, crash needs-triage Awaiting triage

Comments

@pow2clk
Copy link
Member

pow2clk commented Jan 13, 2025

Description
When using the dot intrinsic to calculate a dot product, the generated code will use a signed mad operator instead of the umad operator that the mul intrinsic uses when it amounts to a dot product.

Steps to Reproduce

Just pass unsigned integer vectors of 2 or more elements to the dot intrinsic.

Switching the defined FUNC between dot and mul in this godbolt link will show the difference in output: https://godbolt.org/z/Pz6a5a38r

Actual Behavior

You can see that it will change between calling IMad with opcode 48 and UMad with opcode 49.

Environment

  • DXC version 1.8.2407
  • Host Operating System macos 14.7.2
@pow2clk pow2clk added bug Bug, regression, crash needs-triage Awaiting triage labels Jan 13, 2025
@pow2clk pow2clk self-assigned this Jan 13, 2025
pow2clk added a commit to pow2clk/DirectXShaderCompiler that referenced this issue Jan 13, 2025
Dot products on uint vectors were not using the unsigned mad operations when they were formed using the dot intrinsic though they did when it was the effect of the mul intrinsic. While such operations aren't very common and less common still are the cases where the unsigned variant makes a difference, this was an internal inconsistency as well as with the clang implementation. This corrects the problem using a dummy op for udot similar to how other intrinsics address similar problems to pass the unsigned information to operation lowering.

Incidentally adds unsigned notations to all the mul intrinsic entries. This is a non-functional change as any one overload that specifies an unsigned variant will apply for all of them, but it is less confusing than having half of them do it and the other half not.

The test verifies the output of a few dot operations with sint, float, and uint types for both dot and mul intrinsics

Fixes microsoft#7058
@pow2clk pow2clk linked a pull request Jan 13, 2025 that will close this issue
@pow2clk pow2clk moved this to Triaged in HLSL Triage Jan 13, 2025
@pow2clk pow2clk moved this from New to In review in HLSL Roadmap Jan 13, 2025
@pow2clk pow2clk added this to the Release 1.8.2502 milestone Jan 13, 2025
@pow2clk pow2clk moved this to Needs Review in HLSL Support Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, regression, crash needs-triage Awaiting triage
Projects
Status: In review
Status: Triaged
Development

Successfully merging a pull request may close this issue.

1 participant