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

wgsl: bit shift width is always u32 or vector of u32 #3516

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

dneto0
Copy link
Contributor

@dneto0 dneto0 commented Oct 7, 2022

  • Allows (1<<2u) to remain abstract
  • Remove the rule that the shift width must be non-negative, since it has to successfully concreteize to an unsigned value.

Fixes: #3511

@dneto0 dneto0 added the wgsl WebGPU Shading Language Issues label Oct 7, 2022
@dneto0 dneto0 added this to the V1.0 milestone Oct 7, 2022
@dneto0
Copy link
Contributor Author

dneto0 commented Oct 7, 2022

Editorially, it was much simpler to keep concrete vs. abstract for each of shift left and shift right.

The key thing this PR does is decouples the type of the shift width from the type of the value being shifted.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2022

Previews, as seen when this build job started (09c0730):
WebGPU webgpu.idl | Explainer | Correspondence Reference
WGSL grammar.js | wgsl.lalr.txt

* Allows (1<<2u) to remain abstract
* Remove the rule that the shift width must be non-negative, since
  it has to successfully concreteize to an unsigned value.

Fixes: gpuweb#3511
@dneto0
Copy link
Contributor Author

dneto0 commented Oct 11, 2022

Group agreed to land this in the 2022-10-11 meeting.

@kdashg
Copy link
Contributor

kdashg commented Oct 13, 2022

WGSL 2022-10-11 Minutes
  • (Fixes: wgsl: (1 << 2u) should be 4 (an abstract int) #3511)
  • DN: Weird issue BC noticed. When doing shift the type of result should be type of LHS (number shifting). The way abstract int case had both concrete or both abstract. So, could have cases like in the title where it's just weird. Making change makes it more nature. Seemed like a nicer way to keep extra behaviour without forcing shifted number to be concrete
  • MM: Was confused 2u isn't' abstract. Literally said u, but was surprising 1 << 2u that we haven't had this problem. Is there a way to have unsigned abstract?
  • DN: You use no suffix, if the value is non-negative then it's non-negative. Can only have an abstract when you know the value.
  • KG: So, instead of 1 << 2 would that work?
  • DN: That would work, you get abstract 4.
  • MM: Before this patch if either of the 2 params were concrete then the spec was written such that both had to be concrete. The suffix list value would get concretized right there. This patch says they can be distinct so LHS abstract and RHS concrete which is good, just surprised it was a problem in the first place.
  • KG: Title was confusing, but sgtm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wgsl WebGPU Shading Language Issues
Projects
WGSL
Awaiting triage
Development

Successfully merging this pull request may close these issues.

wgsl: (1 << 2u) should be 4 (an abstract int)
3 participants