Skip to content

[0027] MaybeReorderThread: Use NOP and numCoherenceBits == 0 for HLSL overloads #485

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

Closed
simoll opened this issue Apr 11, 2025 · 0 comments · Fixed by #487
Closed

[0027] MaybeReorderThread: Use NOP and numCoherenceBits == 0 for HLSL overloads #485

simoll opened this issue Apr 11, 2025 · 0 comments · Fixed by #487
Assignees

Comments

@simoll
Copy link

simoll commented Apr 11, 2025

Which proposal does this relate to?

https://github.com/microsoft/hlsl-specs/blob/main/proposals/0027-shader-execution-reordering.md#maybereorderthread

Describe the issue or outstanding question.

This is an implementation issue that needs a spec change. Quoting #479:

One problem is that an undef value could arise from a coding error or optimization problem, and end up looking like a legitimate call to the intrinsic without the HitObject, rather than being caught by the validator as an undef value when a valid value should have been provided.

Another issue is that undef values are replaced with poison values in latest LLVM, and these are supposed to indicate a bad value that should not be used. We shouldn't be determining whether or not an operation uses a value (and does something different) on the basis of whether or not the value in question is poison.

Solution

Rephrasing to something like the below. This is the consensus solution for #479.

Operation that reorders the current thread based on the supplied hints and HitObject. The HLSL overload without HitObject is lowered by passing a NOP HitObject (HitObject_MakeNop). The HLSL overload without coherence hit is lowered by passing 0 for num of coherence hint bits from LSB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Triaged
Development

Successfully merging a pull request may close this issue.

2 participants