Skip to content

Conversation

@dneto0
Copy link
Contributor

@dneto0 dneto0 commented Mar 31, 2021

This captures the case of pipeline-overridable constants as well.

TODO: modify this when the 'const' -> 'let' change lands.

@dneto0 dneto0 requested review from alan-baker, kvark and litherum March 31, 2021 16:26
@dneto0 dneto0 mentioned this pull request Mar 31, 2021
@dneto0 dneto0 changed the title Add section for "const identifier expression" Add section for "Constant Identifier Expression" Mar 31, 2021
dneto0 added a commit to dneto0/gpuweb that referenced this pull request Mar 31, 2021
dneto0 added a commit to dneto0/gpuweb that referenced this pull request Mar 31, 2021
Copy link
Contributor

@kvark kvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this looks good overall! Just a few notes.

@dneto0 dneto0 added the wgsl WebGPU Shading Language Issues label Apr 1, 2021
@dneto0 dneto0 added this to the MVP milestone Apr 1, 2021
@dneto0 dneto0 requested a review from kvark April 1, 2021 21:58
Copy link
Contributor

@alan-baker alan-baker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit about undefined values.

Comment on lines +3428 to +3443
<td>If pipeline creation specified a value for the [=pipeline constant ID|constant ID=],
then the result is that value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the spec constant is not specified at pipeline creation and the constant has not initializer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion in the meeting: Add a note for the "no value supplied" case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New commit added, after rebase. PTAL.

dneto0 added 4 commits April 6, 2021 15:07
This captures the case of pipeline-overridable constants as well.

TODO: modify this when the 'const' -> 'let' change lands.
Also, add cross-reference to "pipeline constant ID"
It never happens because pipeline creation fails.
@dneto0 dneto0 merged commit cda3661 into gpuweb:main Apr 6, 2021
@kdashg
Copy link
Contributor

kdashg commented Apr 7, 2021

WGSL meeting minutes 2021-04-06
  • DN: Replied to feedback from DM.
  • AB: What about undefined case? When you don’t specify something
  • DN: Pipeline creation fails
  • AB: What if it has an initializer, do we need to supply it?
  • DN: That would be ok.
  • MM: This is values in the global scope?
  • DN: First row is pipeline overridable constant, <details>
  • DN: To JB’s point, could say that program is invalid in that case?
  • DN: Will add note for AB, will check with AB before merging

dneto0 added a commit that referenced this pull request Apr 7, 2021
* WGSL: describe ref ptr

- Add "memory view types" section
  - Defines reference type and pointer type
  - gives use cases and examples
  - Compares to other lanaguages
- Adds:
  - composite reference sub-expressions
  - address-of expression (unary &)
  - indirection expression (unary *)
  - variable identifier expression
  - const identifier expression
  - formal parmaeter identifier expression

TODO: grammar update for unary * and unary &

Alternate to #1368
Fixes: #1456

* Remove -> void from the newer examples

* Apply review feedback

Addressed feedback from @alan-baker and @ben-clayton

Fixed typos, examples, links, and added an example showing
what happens when the expression in a return statement is a reference.

* Fix word

* Fix types in comments in examples

* Add grammar rule for unary &, unary *; address feedback

- Added the grammar rule for address-of and indirection
- Fixed algorithm name text (was copypasta)
- Fixed example in the assignment-statement section.

* Fix typo in the add_one example

* Make address-of, indirection defined terms

Improve cross-linking.

* Add missing access qualifier in example

* One more rename of "composite reference component expression"

* Fix caption on structure-member reference expression

* Convert ptr syntax examples to use a function declaration

Applies feedback from @kvark review.
Using a complete and valid function definition allows the example
to be valid WGSL.

- changed the pointer-to-uniform example to pointer-to-function,
  because pointer-to-uniform arguments won't be valid. (Sorry, that's a
  restriction from baseline SPIR-V in Vulkan restriction.)

* Remove "Const identifier expression"

That was pulled out to #1586

* For assignment statement, spell out memory *locations*

* Add cross-reference to function-param expression
ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this pull request Sep 6, 2022
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.

4 participants