Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Emit SPIR-V from WSL compiler (Part 1) #164

Closed
litherum opened this issue Oct 14, 2018 · 9 comments
Closed

Emit SPIR-V from WSL compiler (Part 1) #164

litherum opened this issue Oct 14, 2018 · 9 comments

Comments

@litherum
Copy link
Contributor

Migrated from https://bugs.webkit.org/show_bug.cgi?id=177998:

At 2017-10-06T06:46:15Z, mmaxfield@apple.com wrote:
Emit SPIR-V from WSL compiler

@litherum
Copy link
Contributor Author

At 2017-10-06T06:47:34Z, mmaxfield@apple.com wrote:
Created attachment 322993
WIP

@litherum
Copy link
Contributor Author

At 2017-10-06T19:36:59Z, mmaxfield@apple.com wrote:
Created attachment 323038
WIP

@litherum
Copy link
Contributor Author

At 2017-10-06T23:26:24Z, mmaxfield@apple.com wrote:
Created attachment 323064
WIP

@litherum
Copy link
Contributor Author

At 2017-10-06T23:49:48Z, mmaxfield@apple.com wrote:
Created attachment 323065
WIP

@litherum
Copy link
Contributor Author

At 2017-10-08T02:55:50Z, mmaxfield@apple.com wrote:
Created attachment 323116
Patch

@litherum
Copy link
Contributor Author

At 2017-10-11T22:43:07Z, fpizlo@apple.com wrote:
Comment on attachment 323116
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.

@litherum
Copy link
Contributor Author

At 2017-10-12T18:45:08Z, commit-queue@webkit.org wrote:
Comment on attachment 323116
Patch

Clearing flags on attachment: 323116

Committed r223246: https://trac.webkit.org/changeset/223246

@litherum
Copy link
Contributor Author

At 2017-10-12T18:45:09Z, commit-queue@webkit.org wrote:
All reviewed patches have been landed. Closing bug.

@litherum
Copy link
Contributor Author

At 2017-10-12T18:46:32Z, webkit-bug-importer@group.apple.com wrote:
rdar://problem/34959874

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant