Bug 187147 – [WSL] Adding MSL generation notes

• Status: RESOLVED
• Resolution: INVALID
• Component: New Bugs

[Thomas Denney]

[WSL] Adding MSL generation notes

[Thomas Denney]

Created attachment 343824 [details]
Patch

[EWS Watchlist]

Attachment 343824 [details] did not pass style-queue:


ERROR: Tools/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Thomas Denney]

Created attachment 343990 [details]
Patch

[Myles C. Maxfield]

Comment on attachment 343990 [details]
Patch

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

> Tools/ChangeLog:408
> [phone]  Thomas Denney  <[e-mail]>

Whoops! No need for two entries.

> Tools/WebGPUShadingLanguageRI/Metal.html:4
> +    <title>WebSL Metal Compiler</title>

We should figure out a better way to do this. Aren’t JavaScript modules supposed to solve this problem?

> Tools/WebGPUShadingLanguageRI/Metal.html:213
> +// Most of the code below is copy-pasted from index.html, the only significant difference is in |doCompilePresentShaderSource|

Doesn’t this mean there should be some code sharing?

> Tools/WebGPUShadingLanguageRI/MetalCodeGenNotes.md:9
> +* Handling of special WSL attribtues, e.g. `wsl_Position`, `wsl_Color`, `wsl_vertexID` and others

We should figure out what the exhaustive list of these should be. Is coming up with a proposal something you would be interested in doing?

> Tools/WebGPUShadingLanguageRI/MetalCodeGenNotes.md:10
> +* Translation of reference types, including array references

There’s something we should discuss about this. SPIR-V has a lot of optional behavior, and one of the options is to use “logical addressing mode.” This is a mode which disallows pointers (at least in the way that you and I think of when we think of pointers). The Vulkan specification says that all SPIR-V programs accepted by the Vulkan API must be in logical addressing mode. MSL has pointers, and DXIL has pointers (I think?). However, because we need to limit ourselves to the intersection of the three APIs, the portable form of WHLSL needs to not be more expressive than SPIR-V Logical. However, this restriction is completely artificial; WHLSL in all its natural glory need not limit itself to this restriction. Therefore, we came up with the idea that WHLSL has a profile with a few restrictions, which limit its expressibility and allow any WHLSL program in this profile to be expressable in SPIR-V Logical. You can see these restrictions at the very bottom of https://trac.webkit.org/browser/webkit/trunk/Tools/WebGPUShadingLanguageRI/WSL.md.

I’m sure you’re aware that this language isn’t “done” yet - one of the things we have to figure out is if this restricted version of the language is the only version of the language, or if there are “profiles” My opinion is that the restricted version should be the only version, but we should have wider discussion about it.

> Tools/WebGPUShadingLanguageRI/MetalCodeGenNotes.md:19
> +* Enum types are also translated in the obvious way --- **recurison in enum declarations is not checked due to a bug in the existing parser**

Patches welcome.

> Tools/WebGPUShadingLanguageRI/MetalCodegen.js:27
> +template<typename T>

Templates shouldn’t be necessary in the generated code.

> Tools/WebGPUShadingLanguageRI/MetalCodegen.js:66
> +    compile() 

Seems badly named.

> Tools/WebGPUShadingLanguageRI/MetalCodegen.js:238
> +/// Ciurrently this filters out cast operation functions, but there are certainly other operations that could be excluded.

Typo

> Tools/WebGPUShadingLanguageRI/MetalCodegen.js:306
> +class MSLFunctionForwardDeclaration extends MSLFunctionDeclaration 

Is this really the correct abstraction for this?

> Tools/WebGPUShadingLanguageRI/MetalCodegen.js:328
> +// TODO SSA

🤣

> Tools/WebGPUShadingLanguageRI/MetalCodegen.js:392
> +/// Replaces equivalent (but not identical) type instances with identical instances. The SPIR-V compiler depended on toString and allowed for TypeRef instances in the the Type tree. This class replaces each type that it discovers with a unique equivalent instance and collapses TypeRef instances by instantiating them where necessary. In order to determine equivalence it uses a separate stringification algorithm (so that we don't depend on the implementation of toString).

Line breaks, please

> Tools/WebGPUShadingLanguageRI/MetalCodegen.js:395
> +class TypeUnifier extends Rewriter {

I thought we were going with the approach of not de-duping types, but instead only emitting deduped types?

> Tools/WebGPUShadingLanguageRI/Rewriter.js:30
> +class Rewriter extends Visitor {

What is this for?

[EWS Watchlist]

Comment on attachment 343990 [details]
Patch

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

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

[EWS Watchlist]

Created attachment 343997 [details]
Archive of layout-test-results from ews202 for win-future

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

[Thomas Denney]

Created attachment 344255 [details]
Patch

[Thomas Denney]

Created attachment 344396 [details]
Patch

[Thomas Denney]

Created attachment 344505 [details]
Patch

[EWS Watchlist]

Comment on attachment 344505 [details]
Patch

Attachment 344505 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8463792

New failing tests:
media/media-fullscreen-return-to-inline.html

[EWS Watchlist]

Created attachment 344512 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 344505 [details]
Patch

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

New failing tests:
http/tests/security/canvas-remote-read-remote-video-redirect.html
http/tests/preload/onload_event.html

[EWS Watchlist]

Created attachment 344522 [details]
Archive of layout-test-results from ews206 for win-future

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

[Thomas Denney]

Created attachment 344707 [details]
Patch

[Thomas Denney]

Created attachment 344866 [details]
Patch