[Webkit-unassigned] [Bug 187147] [WSL] Adding MSL generation notes

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=187147

--- Comment #4 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 343990
  --> https://bugs.webkit.org/attachment.cgi?id=343990
Patch

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

> Tools/ChangeLog:408
> +2018-06-28  Thomas Denney  <tdenney at apple.com>

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?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180630/9714e7ac/attachment-0001.html>