[WSL] Adding MSL generation notes
Created attachment 343824 [details] Patch
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.
Created attachment 343990 [details] Patch
Comment on attachment 343990 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343990&action=review > Tools/ChangeLog:408 > +2018-06-28 Thomas Denney <tdenney@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?
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
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
Created attachment 344255 [details] Patch
Created attachment 344396 [details] Patch
Created attachment 344505 [details] Patch
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
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
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
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
Created attachment 344707 [details] Patch
Created attachment 344866 [details] Patch