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

[hlsl-out] Incorrect behaviour with firstTrailingBit #1939

Closed
hasali19 opened this issue May 23, 2022 · 4 comments · Fixed by #2315
Closed

[hlsl-out] Incorrect behaviour with firstTrailingBit #1939

hasali19 opened this issue May 23, 2022 · 4 comments · Fixed by #2315
Labels
area: back-end Outputs of shader conversion kind: bug Something isn't working lang: HLSL High-Level Shading Language

Comments

@hasali19
Copy link
Contributor

This program incorrectly outputs -1 with DirectX. I think this is because of the use of firstbitlow, since it works correctly with tint which uses its own polyfill for firstbitlow - perhaps naga needs to do the same?

WGSL:

@group(0)
@binding(1)
var<storage, read_write> k: i32;

@compute
@workgroup_size(1)
fn main() {
    k = max(~0, firstTrailingBit(1));
}

HLSL:

RWByteAddressBuffer k : register(u1);

[numthreads(1, 1, 1)]
void main()
{
    k.Store(0, asuint(max(~0, firstbitlow(1))));
    return;
}
@teoxoy teoxoy added kind: bug Something isn't working area: back-end Outputs of shader conversion lang: HLSL High-Level Shading Language labels May 28, 2022
@teoxoy teoxoy added this to the WGSL Specification V1 milestone May 28, 2022
@evahop
Copy link
Contributor

evahop commented Apr 20, 2023

I currently don't have a method for testing hlsl, but this is perplexing. At first glance this would imply firstbitlow returns -1 for a bit at the 0 index. That seems very counterintuitive.

@teoxoy
Copy link
Member

teoxoy commented Apr 20, 2023

Looking at the generated DXBC via https://shader-playground.timjones.io/ it seems that the umax instruction is generated instead of imax. DXC seems to also be consistent here; generating DXIL with an evaluated -1.

It seems to me that the return type of firstbitlow is always an uint and doesn't depend on the argument contrary to what the HLSL docs suggest.

Wrapping the call with an int cast (i.e. int(firstbitlow(x))) gets FXC and DXC to output the expected instructions.

@teoxoy
Copy link
Member

teoxoy commented Apr 20, 2023

Looking at the src of DXC, firstbithigh might be in the same boat but I'm not sure what the meaning of the syntax in that file is.

@evahop
Copy link
Contributor

evahop commented Apr 24, 2023

I’m not sure of the syntax either but firstbithigh does appear to produce the same umax in the OP example. I will update this where needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: back-end Outputs of shader conversion kind: bug Something isn't working lang: HLSL High-Level Shading Language
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants