[WHLSL] Checker sets wrong type for property access instruction with an ander
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Jul 2019 23:42:30 +0000 (23:42 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Jul 2019 23:42:30 +0000 (23:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200282

Reviewed by Myles C. Maxfield.

Source/WebCore:

We were assigning resulting type based on the base value instead of the ander
of the base value. For example, consider:
```
struct Point { float x; float y; }
compute main(device Point[] buffer) { buffer[0]; }
```

The local variable "buffer" is in the "thread" address space. So we would end up
trying to use the thread address space for "buffer[0]". This caused us to
generate invalid Metal code because we would call a "thread" ander with a
"device" pointer. The fix is to use the "device" address space, which is
the type of the ander we were already setting on this property access instruction.

Test: webgpu/whlsl/device-proper-type-checker.html

* Modules/webgpu/WHLSL/WHLSLChecker.cpp:
(WebCore::WHLSL::Checker::finishVisiting):

LayoutTests:

* webgpu/whlsl/device-proper-type-checker-expected.txt: Added.
* webgpu/whlsl/device-proper-type-checker.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248022 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/webgpu/whlsl/device-proper-type-checker-expected.txt [new file with mode: 0644]
LayoutTests/webgpu/whlsl/device-proper-type-checker.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp

index 532e7e9..9a75788 100644 (file)
@@ -1,3 +1,13 @@
+2019-07-30  Saam Barati  <sbarati@apple.com>
+
+        [WHLSL] Checker sets wrong type for property access instruction with an ander
+        https://bugs.webkit.org/show_bug.cgi?id=200282
+
+        Reviewed by Myles C. Maxfield.
+
+        * webgpu/whlsl/device-proper-type-checker-expected.txt: Added.
+        * webgpu/whlsl/device-proper-type-checker.html: Added.
+
 2019-07-30  Ryan Haddad  <ryanhaddad@apple.com>
 
         Add test expectations and baselines for iPad
diff --git a/LayoutTests/webgpu/whlsl/device-proper-type-checker-expected.txt b/LayoutTests/webgpu/whlsl/device-proper-type-checker-expected.txt
new file mode 100644 (file)
index 0000000..4882f90
--- /dev/null
@@ -0,0 +1,5 @@
+PASS 
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/webgpu/whlsl/device-proper-type-checker.html b/LayoutTests/webgpu/whlsl/device-proper-type-checker.html
new file mode 100644 (file)
index 0000000..cee8cf1
--- /dev/null
@@ -0,0 +1,90 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../js/webgpu-functions.js"></script>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+const shaderSource = `
+struct Point {
+    float x;
+    float y;
+}
+
+[numthreads(1, 1, 1)]
+compute void computeShader(device Point[] buffer : register(u0), float3 threadID : SV_DispatchThreadID) {
+    float x = buffer[0].x;
+    float y = buffer[0].y;
+    x += 1.0;
+    y += 1.0;
+    buffer[0].x = x;
+    buffer[0].y = y;
+}
+`;
+
+async function start(device) {
+    const shaderModule = device.createShaderModule({code: shaderSource, isWHLSL: true});
+    const computeStage = {module: shaderModule, entryPoint: "computeShader"};
+
+    const bindGroupLayoutDescriptor = {bindings: [{binding: 0, visibility: 7, type: "storage-buffer"}]};
+    const bindGroupLayout = device.createBindGroupLayout(bindGroupLayoutDescriptor);
+    const pipelineLayoutDescriptor = {bindGroupLayouts: [bindGroupLayout]};
+    const pipelineLayout = device.createPipelineLayout(pipelineLayoutDescriptor);
+
+    const computePipelineDescriptor = {computeStage, layout: pipelineLayout};
+    const computePipeline = device.createComputePipeline(computePipelineDescriptor);
+
+    const size = Float32Array.BYTES_PER_ELEMENT * 2;
+
+    const bufferDescriptor = {size, usage: GPUBufferUsage.MAP_WRITE | GPUBufferUsage.TRANSFER_SRC};
+    const buffer = device.createBuffer(bufferDescriptor);
+    const bufferArrayBuffer = await buffer.mapWriteAsync();
+    const bufferFloat32Array = new Float32Array(bufferArrayBuffer);
+    bufferFloat32Array[0] = 42.0;
+    bufferFloat32Array[1] = 43.0;
+    buffer.unmap();
+
+    const resultsBufferDescriptor = {size, usage: GPUBufferUsage.STORAGE | GPUBufferUsage.TRANSFER_DST | GPUBufferUsage.MAP_READ};
+    const resultsBuffer = device.createBuffer(resultsBufferDescriptor);
+
+    const bufferBinding = {buffer: resultsBuffer, size};
+    const bindGroupBinding = {binding: 0, resource: bufferBinding};
+    const bindGroupDescriptor = {layout: bindGroupLayout, bindings: [bindGroupBinding]};
+    const bindGroup = device.createBindGroup(bindGroupDescriptor);
+
+    const commandEncoder = device.createCommandEncoder(); // {}
+    commandEncoder.copyBufferToBuffer(buffer, 0, resultsBuffer, 0, size);
+    const computePassEncoder = commandEncoder.beginComputePass();
+    computePassEncoder.setPipeline(computePipeline);
+    computePassEncoder.setBindGroup(0, bindGroup);
+    computePassEncoder.dispatch(1, 1, 1);
+    computePassEncoder.endPass();
+    const commandBuffer = commandEncoder.finish();
+    device.getQueue().submit([commandBuffer]);
+
+    const resultsArrayBuffer = await resultsBuffer.mapReadAsync();
+    const resultsFloat32Array = new Float32Array(resultsArrayBuffer);
+    if (resultsFloat32Array[0] == 43.0
+        && resultsFloat32Array[1] == 44.0)
+        testPassed("");
+    else
+        testFailed("");
+    resultsBuffer.unmap();
+}
+window.jsTestIsAsync = true;
+getBasicDevice().then(function(device) {
+    start(device).then(function() {
+        finishJSTest();
+    }, function() {
+        testFailed("");
+        finishJSTest();
+    });
+}, function() {
+    testPassed("");
+    finishJSTest();
+});
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index 598ad3e..09ee2e2 100644 (file)
@@ -1,5 +1,30 @@
 2019-07-30  Saam Barati  <sbarati@apple.com>
 
+        [WHLSL] Checker sets wrong type for property access instruction with an ander
+        https://bugs.webkit.org/show_bug.cgi?id=200282
+
+        Reviewed by Myles C. Maxfield.
+
+        We were assigning resulting type based on the base value instead of the ander
+        of the base value. For example, consider:
+        ```
+        struct Point { float x; float y; }
+        compute main(device Point[] buffer) { buffer[0]; }
+        ```
+        
+        The local variable "buffer" is in the "thread" address space. So we would end up
+        trying to use the thread address space for "buffer[0]". This caused us to
+        generate invalid Metal code because we would call a "thread" ander with a
+        "device" pointer. The fix is to use the "device" address space, which is
+        the type of the ander we were already setting on this property access instruction.
+
+        Test: webgpu/whlsl/device-proper-type-checker.html
+
+        * Modules/webgpu/WHLSL/WHLSLChecker.cpp:
+        (WebCore::WHLSL::Checker::finishVisiting):
+
+2019-07-30  Saam Barati  <sbarati@apple.com>
+
         [WHLSL] Make ASTDumper dump types and address spaces
         https://bugs.webkit.org/show_bug.cgi?id=200281
 
index ae0d32e..9117405 100644 (file)
@@ -1058,7 +1058,7 @@ void Checker::finishVisiting(AST::PropertyAccessExpression& propertyAccessExpres
     AST::TypeAnnotation typeAnnotation = AST::RightValue();
     if (auto leftAddressSpace = baseInfo->typeAnnotation.leftAddressSpace()) {
         if (anderFunction)
-            typeAnnotation = AST::LeftValue { *leftAddressSpace };
+            typeAnnotation = AST::LeftValue { downcast<AST::ReferenceType>(anderFunction->type()).addressSpace() };
         else if (setterFunction)
             typeAnnotation = AST::AbstractLeftValue();
     } else if (!baseInfo->typeAnnotation.isRightValue() && (setterFunction || threadAnderFunction))