Skip to content

abs only works on signed integer scalar or vector #1282

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

Merged
merged 1 commit into from
Jan 19, 2021
Merged

Conversation

dneto0
Copy link
Contributor

@dneto0 dneto0 commented Dec 8, 2020

It doesn't make sense for it to apply to unsigned integer scalar or
vector. It's a no-op in that case.

@dneto0 dneto0 requested a review from dj2 December 8, 2020 17:07
@dneto0
Copy link
Contributor Author

dneto0 commented Dec 8, 2020

abs on unsigned, if it exists, ought to be a no-op.
But currently the SPIR-V description is to use the SAbs extended instruction, which is signed-absolute value.

Either remove the abs for unsigned argument (this PR), or modify the description.

This may require discussion

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2020

Previews, as seen at the time of posting this comment:
WebGPU | IDL
WGSL
ee5b1c7

@kvark
Copy link
Contributor

kvark commented Dec 9, 2020

It would be nice to reach some guidelines for consistency of WGSL with regards to no-ops. For example, I recall that we allow casting the type to itself as a no-op, just because it will be easier to write generic code. Should the same argument apply to abs()? And if not, why?

@dj2
Copy link
Member

dj2 commented Dec 9, 2020

My position is we should keep this as allowed and spec it as a no-op.

@kvark
Copy link
Contributor

kvark commented Dec 9, 2020

Mini-investigation:

  • SPIR-V requires signed
  • HLSL seem to require signed
  • MSL documents that available on unsigned, but we may need to try it out and see

In the unsigned case it's the identity function.
@dneto0
Copy link
Contributor Author

dneto0 commented Dec 9, 2020

I changed to PR to keep the unsigned case, but split it out into its own rows and describe it as the identity function.

I'm ok with either path, as long as we eliminate the implication of 'abs' on unsigned as mapping to GLSLstd450SAbs, because that's a bug.

@Kangz Kangz added the wgsl WebGPU Shading Language Issues label Dec 11, 2020
@kdashg kdashg merged commit eae6358 into gpuweb:main Jan 19, 2021
@kdashg
Copy link
Contributor

kdashg commented Jan 19, 2021

ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this pull request Sep 6, 2022
…1417)

This PR adds unimplmented stubs for the read-write-modify atomic operations.

 * `atomicAdd`
 * `atomicAnd`
 * `atomicCompareExchangeWeak`
 * `atomicExchange`
 * `atomicMax`
 * `atomicMin`
 * `atomicOr`
 * `atomicSub`
 * `atomicXor`

Issue gpuweb#1275, gpuweb#1276, gpuweb#1277, gpuweb#1278, gpuweb#1279, gpuweb#1280, gpuweb#1281, gpuweb#1282, gpuweb#1283
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
None yet
Development

Successfully merging this pull request may close these issues.

5 participants