- Revision
- 245973
- Author
- [email protected]
- Date
- 2019-05-31 11:16:45 -0700 (Fri, 31 May 2019)
Log Message
[WHLSL] Make sure we properly emit code for "&*x"
https://bugs.webkit.org/show_bug.cgi?id=198198
Reviewed by Myles C. Maxfield.
Source/WebCore:
I ran into this when trying to test zero-filling code, so let's just fix it.
The issue is that the property resolver ends up emitting code that looks like
"&*x". The semantics of this are such that it should result in just x.
However, we emitted Metal code in such a way where we'd end up with a pointer
to a temporary value. To fix this, DereferenceExpression will emit code that results
in a reference type. Then, MakePointerExpression will correctly return the
pointer backing that reference type.
Because of this, we also no longer need to pattern match the lhs of assignment
expressions since we will now be assigning to a reference type.
Test: webgpu/whlsl-store-to-property-updates-properly.html
* Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:
(WebCore::WHLSL::Metal::FunctionDefinitionWriter::visit):
LayoutTests:
* webgpu/whlsl-store-to-property-updates-properly-expected.html: Added.
* webgpu/whlsl-store-to-property-updates-properly.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (245972 => 245973)
--- trunk/LayoutTests/ChangeLog 2019-05-31 17:11:58 UTC (rev 245972)
+++ trunk/LayoutTests/ChangeLog 2019-05-31 18:16:45 UTC (rev 245973)
@@ -1,3 +1,13 @@
+2019-05-31 Saam Barati <[email protected]>
+
+ [WHLSL] Make sure we properly emit code for "&*x"
+ https://bugs.webkit.org/show_bug.cgi?id=198198
+
+ Reviewed by Myles C. Maxfield.
+
+ * webgpu/whlsl-store-to-property-updates-properly-expected.html: Added.
+ * webgpu/whlsl-store-to-property-updates-properly.html: Added.
+
2019-05-31 Ryan Haddad <[email protected]>
Unreviewed, rolling out r245946.
Added: trunk/LayoutTests/webgpu/whlsl-store-to-property-updates-properly-expected.html (0 => 245973)
--- trunk/LayoutTests/webgpu/whlsl-store-to-property-updates-properly-expected.html (rev 0)
+++ trunk/LayoutTests/webgpu/whlsl-store-to-property-updates-properly-expected.html 2019-05-31 18:16:45 UTC (rev 245973)
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<canvas id="canvas" width="400" height="400"></canvas>
+<script>
+async function start() {
+ const canvas = document.getElementById("canvas");
+ const context = canvas.getContext("2d");
+ context.fillStyle = "blue";
+ context.fillRect(0, 0, 400, 400);
+ context.fillStyle = "white";
+ context.fillRect(100, 100, 200, 200);
+}
+window.addEventListener("load", start);
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/webgpu/whlsl-store-to-property-updates-properly.html (0 => 245973)
--- trunk/LayoutTests/webgpu/whlsl-store-to-property-updates-properly.html (rev 0)
+++ trunk/LayoutTests/webgpu/whlsl-store-to-property-updates-properly.html 2019-05-31 18:16:45 UTC (rev 245973)
@@ -0,0 +1,146 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<canvas id="canvas" width="400" height="400"></canvas>
+<script>
+const shaderSource = `
+struct VertexOut {
+ float4 position : SV_Position;
+ float shade : attribute(0);
+}
+
+struct Bar {
+ int x;
+ float y;
+ int z;
+}
+
+struct Foo {
+ Bar bar;
+ Bar bar2;
+}
+
+thread Foo* ptr() {
+ Foo local;
+ return &local;
+}
+
+vertex VertexOut vertexShader(float4 position : attribute(0), float shade : attribute(1)) {
+ VertexOut result;
+
+ thread Foo* p = ptr();
+ p->bar2.y = 1.0;
+ float value = p->bar2.y;
+
+ result.position = position;
+ result.shade = value;
+
+ return result;
+}
+
+fragment float4 fragmentShader(float shade : attribute(0)) : SV_Target 0 {
+ return float4(shade, shade, shade, 1.0);
+}
+`;
+async function start() {
+ const adapter = await navigator.gpu.requestAdapter();
+ const device = await adapter.requestDevice();
+
+ const shaderModule = device.createShaderModule({code: shaderSource, isWHLSL: true});
+ const vertexStage = {module: shaderModule, entryPoint: "vertexShader"};
+ const fragmentStage = {module: shaderModule, entryPoint: "fragmentShader"};
+ const primitiveTopology = "triangle-strip";
+ const rasterizationState = {frontFace: "cw", cullMode: "none"};
+ const alphaBlend = {};
+ const colorBlend = {};
+ const colorStates = [{format: "rgba8unorm", alphaBlend, colorBlend, writeMask: 15}]; // GPUColorWriteBits.ALL
+ const depthStencilState = null;
+
+ const attribute0 = {shaderLocation: 0, format: "float4", offset: 0};
+ const attribute1 = {shaderLocation: 1, format: "float", offset: 16};
+ const attributes = [attribute0, attribute1];
+ const input0 = {stride: 20, attributeSet: attributes};
+ const inputs = [input0];
+ const vertexInput = {vertexBuffers: inputs};
+
+ const bindGroupLayoutDescriptor = {bindings: [{binding: 0, visibility: 7, type: "uniform-buffer"}]};
+ const bindGroupLayout = device.createBindGroupLayout(bindGroupLayoutDescriptor);
+ const pipelineLayoutDescriptor = {bindGroupLayouts: [bindGroupLayout]};
+ const pipelineLayout = device.createPipelineLayout(pipelineLayoutDescriptor);
+
+ const renderPipelineDescriptor = {vertexStage, fragmentStage, primitiveTopology, rasterizationState, colorStates, depthStencilState, vertexInput, sampleCount: 1, layout: pipelineLayout};
+ const renderPipeline = device.createRenderPipeline(renderPipelineDescriptor);
+
+ const vertexBuffer0Descriptor = {size: Float32Array.BYTES_PER_ELEMENT * 5 * 4, usage: GPUBufferUsage.VERTEX | GPUBufferUsage.MAP_WRITE};
+ const vertexBuffer0 = device.createBuffer(vertexBuffer0Descriptor);
+ const vertexBuffer0ArrayBuffer = await vertexBuffer0.mapWriteAsync();
+ const vertexBuffer0Float32Array = new Float32Array(vertexBuffer0ArrayBuffer);
+ vertexBuffer0Float32Array[0] = -0.5;
+ vertexBuffer0Float32Array[1] = -0.5;
+ vertexBuffer0Float32Array[2] = 1.0;
+ vertexBuffer0Float32Array[3] = 1.0;
+ vertexBuffer0Float32Array[4] = 1.0;
+
+ vertexBuffer0Float32Array[5] = -0.5;
+ vertexBuffer0Float32Array[6] = 0.5;
+ vertexBuffer0Float32Array[7] = 1.0;
+ vertexBuffer0Float32Array[8] = 1.0;
+ vertexBuffer0Float32Array[9] = 1.0;
+
+ vertexBuffer0Float32Array[10] = 0.5;
+ vertexBuffer0Float32Array[11] = -0.5;
+ vertexBuffer0Float32Array[12] = 1.0;
+ vertexBuffer0Float32Array[13] = 1.0;
+ vertexBuffer0Float32Array[14] = 1.0;
+
+ vertexBuffer0Float32Array[15] = 0.5;
+ vertexBuffer0Float32Array[16] = 0.5;
+ vertexBuffer0Float32Array[17] = 1.0;
+ vertexBuffer0Float32Array[18] = 1.0;
+ vertexBuffer0Float32Array[19] = 1.0;
+ vertexBuffer0.unmap();
+
+ const resourceBufferDescriptor = {size: Float32Array.BYTES_PER_ELEMENT, usage: GPUBufferUsage.UNIFORM | GPUBufferUsage.MAP_WRITE};
+ const resourceBuffer = device.createBuffer(resourceBufferDescriptor);
+ const resourceBufferArrayBuffer = await resourceBuffer.mapWriteAsync();
+ const resourceBufferFloat32Array = new Float32Array(resourceBufferArrayBuffer);
+ resourceBufferFloat32Array[0] = 1;
+ resourceBuffer.unmap();
+
+ const bufferBinding = {buffer: resourceBuffer, size: 4};
+ const bindGroupBinding = {binding: 0, resource: bufferBinding};
+ const bindGroupDescriptor = {layout: bindGroupLayout, bindings: [bindGroupBinding]};
+ const bindGroup = device.createBindGroup(bindGroupDescriptor);
+
+ const canvas = document.getElementById("canvas");
+ const context = canvas.getContext("gpu");
+ const swapChainDescriptor = {device, format: "bgra8unorm"};
+ const swapChain = context.configureSwapChain(swapChainDescriptor);
+ const outputTexture = swapChain.getCurrentTexture();
+ const outputTextureView = outputTexture.createDefaultView();
+
+ const commandEncoder = device.createCommandEncoder(); // {}
+ const red = {r: 0, g: 0, b: 1, a: 1};
+ const colorAttachments = [{attachment: outputTextureView, resolveTarget: null, loadOp: "clear", storeOp: "store", clearColor: red}];
+ const depthStencilAttachment = null;
+ const renderPassDescriptor = {colorAttachments, depthStencilAttachment};
+ const renderPassEncoder = commandEncoder.beginRenderPass(renderPassDescriptor);
+ renderPassEncoder.setPipeline(renderPipeline);
+ renderPassEncoder.setBindGroup(0, bindGroup);
+ renderPassEncoder.setVertexBuffers(0, [vertexBuffer0], [0]);
+ renderPassEncoder.draw(4, 1, 0, 0);
+ renderPassEncoder.endPass();
+ const commandBuffer = commandEncoder.finish();
+ device.getQueue().submit([commandBuffer]);
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+}
+if (window.testRunner)
+ testRunner.waitUntilDone();
+window.addEventListener("load", start);
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (245972 => 245973)
--- trunk/Source/WebCore/ChangeLog 2019-05-31 17:11:58 UTC (rev 245972)
+++ trunk/Source/WebCore/ChangeLog 2019-05-31 18:16:45 UTC (rev 245973)
@@ -1,3 +1,26 @@
+2019-05-31 Saam Barati <[email protected]>
+
+ [WHLSL] Make sure we properly emit code for "&*x"
+ https://bugs.webkit.org/show_bug.cgi?id=198198
+
+ Reviewed by Myles C. Maxfield.
+
+ I ran into this when trying to test zero-filling code, so let's just fix it.
+ The issue is that the property resolver ends up emitting code that looks like
+ "&*x". The semantics of this are such that it should result in just x.
+ However, we emitted Metal code in such a way where we'd end up with a pointer
+ to a temporary value. To fix this, DereferenceExpression will emit code that results
+ in a reference type. Then, MakePointerExpression will correctly return the
+ pointer backing that reference type.
+
+ Because of this, we also no longer need to pattern match the lhs of assignment
+ expressions since we will now be assigning to a reference type.
+
+ Test: webgpu/whlsl-store-to-property-updates-properly.html
+
+ * Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:
+ (WebCore::WHLSL::Metal::FunctionDefinitionWriter::visit):
+
2019-05-31 Geoffrey Garen <[email protected]>
Some WeakPtr cleanup
Modified: trunk/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp (245972 => 245973)
--- trunk/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp 2019-05-31 17:11:58 UTC (rev 245972)
+++ trunk/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp 2019-05-31 18:16:45 UTC (rev 245973)
@@ -457,15 +457,6 @@
void FunctionDefinitionWriter::visit(AST::AssignmentExpression& assignmentExpression)
{
- if (is<AST::DereferenceExpression>(assignmentExpression.left())) {
- checkErrorAndVisit(downcast<AST::DereferenceExpression>(assignmentExpression.left()).pointer());
- auto leftName = m_stack.takeLast();
- checkErrorAndVisit(assignmentExpression.right());
- auto rightName = m_stack.takeLast();
- m_stringBuilder.append(makeString('*', leftName, " = ", rightName, ";\n"));
- m_stack.append(rightName);
- return;
- }
checkErrorAndVisit(assignmentExpression.left());
auto leftName = m_stack.takeLast();
checkErrorAndVisit(assignmentExpression.right());
@@ -510,7 +501,7 @@
checkErrorAndVisit(dereferenceExpression.pointer());
auto right = m_stack.takeLast();
auto variableName = generateNextVariableName();
- m_stringBuilder.append(makeString(m_typeNamer.mangledNameForType(dereferenceExpression.resolvedType()), ' ', variableName, " = *", right, ";\n"));
+ m_stringBuilder.append(makeString(AST::toString(*dereferenceExpression.typeAnnotation().leftAddressSpace()), ' ', m_typeNamer.mangledNameForType(dereferenceExpression.resolvedType()), "& ", variableName, " = *", right, ";\n"));
m_stack.append(variableName);
}
