Skip to content

abs only works on signed integer scalar or vector#1282

Merged
kdashg merged 1 commit intogpuweb:mainfrom
dneto0:uabs
Jan 19, 2021
Merged

abs only works on signed integer scalar or vector#1282
kdashg merged 1 commit intogpuweb:mainfrom
dneto0:uabs

Conversation

@dneto0
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

6 participants