[WHLSL] The checker needs to resolve types for the anonymous variables in ReadModifyWrite expressions
https://bugs.webkit.org/show_bug.cgi?id=198988
Reviewed by Dean Jackson and Myles C. Maxfield.
Source/WebCore:
This patch makes it so that the Checker assigns types to the internal variables
in a read modify write expression. These were the only variables that didn't have
types ascribed to them.
This patch also does a fly by fix where we kept pointers to value types
in a HashMap in the checker. This is wrong precisely when the HashMap gets
resized. Instead, we now just store the value itself since we're just
dealing with a simple Variant that wraps either an empty struct or an
enum.
Test: webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables.html
* Modules/webgpu/WHLSL/AST/WHLSLVariableDeclaration.h:
(WebCore::WHLSL::AST::VariableDeclaration::setType):
(WebCore::WHLSL::AST::VariableDeclaration::type const):
* Modules/webgpu/WHLSL/WHLSLASTDumper.cpp: Make it obvious that read
modify write expressions are such by prefixing them with "RMW".
(WebCore::WHLSL::ASTDumper::visit):
* Modules/webgpu/WHLSL/WHLSLChecker.cpp:
(WebCore::WHLSL::Checker::visit):
LayoutTests:
* webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables-expected.txt: Added.
* webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@246625 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index c431591..5b88616 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2019-06-19 Saam Barati <sbarati@apple.com>
+
+ [WHLSL] The checker needs to resolve types for the anonymous variables in ReadModifyWrite expressions
+ https://bugs.webkit.org/show_bug.cgi?id=198988
+
+ Reviewed by Dean Jackson and Myles C. Maxfield.
+
+ * webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables-expected.txt: Added.
+ * webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables.html: Added.
+
2019-06-19 Nikita Vasilyev <nvasilyev@apple.com>
REGRESSION(r240946): Web Inspector: Styles: Pasting multiple properties has issues
diff --git a/LayoutTests/webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables-expected.txt b/LayoutTests/webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables-expected.txt
new file mode 100644
index 0000000..4660202
--- /dev/null
+++ b/LayoutTests/webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables-expected.txt
@@ -0,0 +1,5 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS resultsFloat32Array[0] is 42
+
diff --git a/LayoutTests/webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables.html b/LayoutTests/webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables.html
new file mode 100644
index 0000000..464e268
--- /dev/null
+++ b/LayoutTests/webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables.html
@@ -0,0 +1,78 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+const shaderSource = `
+[numthreads(1, 1, 1)]
+compute void computeShader(device float[] buffer : register(u0), float3 threadID : SV_DispatchThreadID) {
+ float4 vec;
+ vec[0] += 42.0;
+ buffer[uint(threadID.x)] = vec[0];
+}
+`;
+let resultsFloat32Array;
+async function start() {
+ const adapter = await navigator.gpu.requestAdapter();
+ const device = await adapter.requestDevice();
+
+ 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 * 1;
+
+ 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] = 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();
+ resultsFloat32Array = new Float32Array(resultsArrayBuffer);
+ shouldBe("resultsFloat32Array[0]", "42");
+ resultsBuffer.unmap();
+}
+if (window.testRunner)
+ testRunner.waitUntilDone();
+window.addEventListener("load", function() {
+ start().then(function() {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, function() {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ });
+});
+</script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index b7435ef..4861d5b 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,31 @@
+2019-06-19 Saam Barati <sbarati@apple.com>
+
+ [WHLSL] The checker needs to resolve types for the anonymous variables in ReadModifyWrite expressions
+ https://bugs.webkit.org/show_bug.cgi?id=198988
+
+ Reviewed by Dean Jackson and Myles C. Maxfield.
+
+ This patch makes it so that the Checker assigns types to the internal variables
+ in a read modify write expression. These were the only variables that didn't have
+ types ascribed to them.
+
+ This patch also does a fly by fix where we kept pointers to value types
+ in a HashMap in the checker. This is wrong precisely when the HashMap gets
+ resized. Instead, we now just store the value itself since we're just
+ dealing with a simple Variant that wraps either an empty struct or an
+ enum.
+
+ Test: webgpu/whlsl-checker-should-set-type-of-read-modify-write-variables.html
+
+ * Modules/webgpu/WHLSL/AST/WHLSLVariableDeclaration.h:
+ (WebCore::WHLSL::AST::VariableDeclaration::setType):
+ (WebCore::WHLSL::AST::VariableDeclaration::type const):
+ * Modules/webgpu/WHLSL/WHLSLASTDumper.cpp: Make it obvious that read
+ modify write expressions are such by prefixing them with "RMW".
+ (WebCore::WHLSL::ASTDumper::visit):
+ * Modules/webgpu/WHLSL/WHLSLChecker.cpp:
+ (WebCore::WHLSL::Checker::visit):
+
2019-06-19 Devin Rousso <drousso@apple.com>
Web Inspector: Network: replace CFNetwork SPI with new API where able
diff --git a/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLVariableDeclaration.h b/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLVariableDeclaration.h
index 9f8d99b..37ea386 100644
--- a/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLVariableDeclaration.h
+++ b/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLVariableDeclaration.h
@@ -64,7 +64,15 @@
String& name() { return m_name; }
- const Optional<UniqueRef<UnnamedType>>& type() const { return m_type; } // Anonymous variables inside ReadModifyWriteExpressions have their type set by the type checker.
+ // We use this for ReadModifyWrite expressions, since we don't know the type of their
+ // internal variables until the checker runs. All other variables should start life out
+ // with a type.
+ void setType(UniqueRef<UnnamedType> type)
+ {
+ ASSERT(!m_type);
+ m_type = WTFMove(type);
+ }
+ const Optional<UniqueRef<UnnamedType>>& type() const { return m_type; }
UnnamedType* type() { return m_type ? &*m_type : nullptr; }
Optional<Semantic>& semantic() { return m_semantic; }
Expression* initializer() { return m_initializer ? &*m_initializer : nullptr; }
diff --git a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp
index 2ebf862..5cfc8e2 100644
--- a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp
+++ b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.cpp
@@ -639,7 +639,7 @@
auto oldVariable = readModifyWriteExpression.oldVariableReference();
auto newVariable = readModifyWriteExpression.newVariableReference();
- m_out.print("(");
+ m_out.print("RMW(");
visit(oldVariable.get());
m_out.print(" = ");
visit(readModifyWriteExpression.leftValue());
diff --git a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp
index d894b92..c05c733 100644
--- a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp
+++ b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp
@@ -463,7 +463,7 @@
bool isBoolType(ResolvingType&);
struct RecurseInfo {
ResolvingType& resolvingType;
- AST::TypeAnnotation& typeAnnotation;
+ const AST::TypeAnnotation typeAnnotation;
};
Optional<RecurseInfo> recurseAndGetInfo(AST::Expression&, bool requiresLeftValue = false);
Optional<RecurseInfo> getInfo(AST::Expression&, bool requiresLeftValue = false);
@@ -859,13 +859,15 @@
if (!leftValueInfo)
return;
- // FIXME: https://bugs.webkit.org/show_bug.cgi?id=198166 Figure out what to do with the ReadModifyWriteExpression's AnonymousVariables.
+ readModifyWriteExpression.oldValue().setType(leftValueInfo->resolvingType.getUnnamedType()->clone());
auto newValueInfo = recurseAndGetInfo(readModifyWriteExpression.newValueExpression());
if (!newValueInfo)
return;
- if (!matchAndCommit(leftValueInfo->resolvingType, newValueInfo->resolvingType)) {
+ if (Optional<UniqueRef<AST::UnnamedType>> matchedType = matchAndCommit(leftValueInfo->resolvingType, newValueInfo->resolvingType))
+ readModifyWriteExpression.newValue().setType(WTFMove(matchedType.value()));
+ else {
setError();
return;
}
@@ -1131,8 +1133,7 @@
ASSERT(variableReference.variable()->type());
AST::TypeAnnotation typeAnnotation = AST::RightValue();
- if (!variableReference.variable()->isAnonymous()) // FIXME: https://bugs.webkit.org/show_bug.cgi?id=198166 This doesn't seem right.
- typeAnnotation = AST::LeftValue { AST::AddressSpace::Thread };
+ typeAnnotation = AST::LeftValue { AST::AddressSpace::Thread };
assignType(variableReference, variableReference.variable()->type()->clone(), WTFMove(typeAnnotation));
}
