Bug 177998 - Emit SPIR-V from WSL compiler (Part 1)
Summary: Emit SPIR-V from WSL compiler (Part 1)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks: 176199
  Show dependency treegraph
 
Reported: 2017-10-05 23:46 PDT by Myles C. Maxfield
Modified: 2018-10-13 19:18 PDT (History)
6 users (show)

See Also:


Attachments
WIP (22.11 KB, patch)
2017-10-05 23:47 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
WIP (27.05 KB, patch)
2017-10-06 12:36 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
WIP (41.81 KB, patch)
2017-10-06 16:26 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
WIP (42.16 KB, patch)
2017-10-06 16:49 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (47.33 KB, patch)
2017-10-07 19:55 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2017-10-05 23:46:15 PDT
Emit SPIR-V from WSL compiler
Comment 1 Myles C. Maxfield 2017-10-05 23:47:34 PDT
Created attachment 322993 [details]
WIP
Comment 2 Myles C. Maxfield 2017-10-06 12:36:59 PDT
Created attachment 323038 [details]
WIP
Comment 3 Myles C. Maxfield 2017-10-06 16:26:24 PDT
Created attachment 323064 [details]
WIP
Comment 4 Myles C. Maxfield 2017-10-06 16:49:48 PDT
Created attachment 323065 [details]
WIP
Comment 5 Myles C. Maxfield 2017-10-07 19:55:50 PDT
Created attachment 323116 [details]
Patch
Comment 6 Filip Pizlo 2017-10-11 15:43:07 PDT
Comment on attachment 323116 [details]
Patch

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

> Tools/WebGPUShadingLanguageRI/SPIRVCodegen.js:104
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +

So much space!

> Tools/WebGPUShadingLanguageRI/SPIRVCodegen.js:120
> +                    switch (type[0]) {

It seems like you really want the TypeRef, not the instantiated type.  You should try to see how you can structure this code so that you get that.  Then, you can case on types more easily.

FWIW the inliner already leaves behind the uninstantiated types in other places where we already needed them.

I don't think that this concern should block landing, since the plan here is to land a WIP that we can iterate on.

> Tools/WebGPUShadingLanguageRI/SPIRVCodegen.js:190
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +

So much space!

> Tools/WebGPUShadingLanguageRI/SPIRVCodegen.js:244
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +

So space.

> Tools/WebGPUShadingLanguageRI/SPIRVTypeAnalyzer.js:102
> +    // FIXME: Using toString() in these functions is a hack. Instead, we should implement a proper type deduper.

Yeah, it's a hack.  But also, can you explain why you need a map at all?

> Tools/WebGPUShadingLanguageRI/SPIRVTypeAnalyzer.js:138
> +            this.typeMap.set(node.toString(), {id: id, fieldTypes: fieldTypes});

ES6 lets you say just {id, fieldTypes} in this case.

> Tools/WebGPUShadingLanguageRI/SPIRVVariableAnalyzer.js:141
> +        let builtInStructs = [
> +            "struct vec2<> { int32 x; int32 y }",
> +            "struct vec2<> { uint32 x; uint32 y; }",
> +            "struct vec2<> { float32 x; float32 y; }",
> +            "struct vec2<> { float64 x; float64 y; }",
> +            "struct vec3<> { int32 x; int32 y; int32 z; }",
> +            "struct vec3<> { uint32 x; uint32 y; uint32 z; }",
> +            "struct vec3<> { float32 x; float32 y; float32 z; }",
> +            "struct vec3<> { float64 x; float64 y; float64 z; }",
> +            "struct vec4<> { int32 x; int32 y; int32 z; int32 w; }",
> +            "struct vec4<> { uint32 x; uint32 y; uint32 z; uint32 w; }",
> +            "struct vec4<> { float32 x; float32 y; float32 z; float32 w; }",
> +            "struct vec4<> { float64 x; float64 y; float64 z; float64 w; }"
> +        ];
> +        for (let builtInStruct of builtInStructs) {
> +            if (node.toString() == builtInStruct) {
> +                this.result.push({ name: this.nameComponents.join(""), id: this._currentId++, type: this.typeMap.get(node.toString()).id, location: this._currentLocation++  });
> +                return;
> +            }
> +        }

I think you really want Intrinsics to stash fields in those types to describe their spirv behavior.  I think that the vec types should be native types so that this becomes easier.
Comment 7 WebKit Commit Bot 2017-10-12 11:45:08 PDT
Comment on attachment 323116 [details]
Patch

Clearing flags on attachment: 323116

Committed r223246: <https://trac.webkit.org/changeset/223246>
Comment 8 WebKit Commit Bot 2017-10-12 11:45:09 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2017-10-12 11:46:32 PDT
<rdar://problem/34959874>
Comment 10 Myles C. Maxfield 2018-10-13 19:18:44 PDT
Migrated to https://github.com/gpuweb/WHLSL/issues/164