Bug 186906 – [WSL] Flesh out some background concepts in the spec and describe some holes to fill in

• Component: New Bugs
• Status: RESOLVED FIXED

[Myles C. Maxfield]

[WSL] Flesh out some background concepts in the spec and describe some holes to fill in

[Myles C. Maxfield]

Created attachment 343303 [details]
WIP

[Myles C. Maxfield]

Created attachment 343493 [details]
Patch

[Myles C. Maxfield]

Created attachment 343494 [details]
Patch

[EWS Watchlist]

Comment on attachment 343494 [details]
Patch

Attachment 343494 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8328000

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html

[EWS Watchlist]

Created attachment 343495 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Robin Morisset]

Comment on attachment 343494 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343494&action=review

LGTM (I am not yet an official webkit reviewer so cannot r=me).

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:23
> +affect error reporting.

Very minor nitpick: I don't think I would specifically mention one optimization such as dead code removal.
Maybe we should include some variation on the "as-if" rule: https://en.cppreference.com/w/cpp/language/as_if

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:48
> +FIXME: Describe the three shader types

I am using the "..todo::" directive for this purpose. I do not have any preference for one style over the other.

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:50
> +A shader of a particular type may describe three

Incomplete sentence

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:54
> +#. Tokenization

I like this word better than "Lexical analysis" that I've been using. I will update my side of the document to use it.

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:68
> +Graphics Pipeline

Thank you for this section! It made things a lot clearer to me.

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:108
> +units at the entire shader level or the instruction level (or any level in between). Therefore,

I see what you mean, but I believe that races may actually exhibit behaviours that show intra-instruction concurrency (such as load tearing).
I am sadly not sure how to better word it.

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:233
> +All entry points must begin with the keyword "vertex", "fragment", or "compute", and the keyword

OK, I will add the "compute" keyword to the syntax. I don't think it is currently supported in the interpreter.

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:235
> +type of shader stage. Entry points may not have any generic arguments (e.g. between the <>

We have temporarily removed all generics from the language (except for types in the standard library), so this sentence will have to go.

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:248
> +must be appropriate for the shader stage type associated with this entry point.

I am curious what happens if the names of the fields conflict during this flattening of the structures.

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:262
> +The return type of an entry points are flattened through structs - that is, each member of any struct

grammar nitpick: "an entry points"

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:291
> +following it.

I have added this in my "Whitespace and comments" subsection

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:294
> +"""""""""""""""""

Similarly, this will likely conflict with my "Keywords and punctuation" subsection.

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:504
> +#. We should figure out how we want to handle atomics.

I was planning to put atomics in the "Memory model" section

> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:519
> +Some of these (like barriers) require uniform control flow. We probably can't enforce dynamically uniform expressions, so without a strong use case, we should not include that concept.

We can fairly easily formalize dynamically uniform expressions (see first part of my last email). Not sure what you mean by "enforce".

[Myles C. Maxfield]

Comment on attachment 343494 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343494&action=review

>> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:108
>> +units at the entire shader level or the instruction level (or any level in between). Therefore,
> 
> I see what you mean, but I believe that races may actually exhibit behaviours that show intra-instruction concurrency (such as load tearing).
> I am sadly not sure how to better word it.

Yes, load tearing is totally possible. I’ll try to make this more clear.

>> Tools/WebGPUShadingLanguageRI/SpecWork/source/index.rst:248
>> +must be appropriate for the shader stage type associated with this entry point.
> 
> I am curious what happens if the names of the fields conflict during this flattening of the structures.

Good point.

[Myles C. Maxfield]

https://github.com/gpuweb/WHLSL/commit/42cffb8f564054991e51fe12f04719c85481e094

[Alex Christensen]

Comment on attachment 343494 [details]
Patch

This has been requesting review for more than one year.  If this is still needed, please rebase and re-request review.