ES31: Support atomic functions on D3D11 - Part II
This patch adds the support of translating atomicExchange and
atomicCompSwap without return value on D3D11 back-ends.
As the last parameter of the HLSL intrinsic functions
InterlockedExchange and InterlockedCompareExchange is not optional,
when there is a call of either atomicExchange or atomicCompSwap
without return value, we add a temporary variable for it, so that
we can directly translate all of such calls in outputHLSL.cpp.
BUG=angleproject:2682
TEST=angle_end2end_tests
Change-Id: I7e9c6d3c7d1846c865909b2f5a26592846c82582
Reviewed-on: https://chromium-review.googlesource.com/1161744
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
diff --git a/src/compiler.gypi b/src/compiler.gypi
index 9bbeff0..0c631c9 100644
--- a/src/compiler.gypi
+++ b/src/compiler.gypi
@@ -155,6 +155,8 @@
'compiler/translator/tree_ops/RemovePow.h',
'compiler/translator/tree_ops/RemoveUnreferencedVariables.cpp',
'compiler/translator/tree_ops/RemoveUnreferencedVariables.h',
+ 'compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.cpp',
+ 'compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.h',
'compiler/translator/tree_ops/RewriteDoWhile.cpp',
'compiler/translator/tree_ops/RewriteDoWhile.h',
'compiler/translator/tree_ops/RewriteStructSamplers.cpp',
diff --git a/src/compiler/translator/TranslatorHLSL.cpp b/src/compiler/translator/TranslatorHLSL.cpp
index 4990622..13efd25 100644
--- a/src/compiler/translator/TranslatorHLSL.cpp
+++ b/src/compiler/translator/TranslatorHLSL.cpp
@@ -14,6 +14,7 @@
#include "compiler/translator/tree_ops/ExpandIntegerPowExpressions.h"
#include "compiler/translator/tree_ops/PruneEmptyCases.h"
#include "compiler/translator/tree_ops/RemoveDynamicIndexing.h"
+#include "compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.h"
#include "compiler/translator/tree_ops/RewriteElseBlocks.h"
#include "compiler/translator/tree_ops/RewriteTexelFetchOffset.h"
#include "compiler/translator/tree_ops/RewriteUnaryMinusOperatorInt.h"
@@ -126,6 +127,11 @@
sh::RewriteUnaryMinusOperatorInt(root);
}
+ if (getShaderVersion() >= 310)
+ {
+ sh::RewriteAtomicFunctionExpressions(root, &getSymbolTable());
+ }
+
sh::OutputHLSL outputHLSL(getShaderType(), getShaderVersion(), getExtensionBehavior(),
getSourcePath(), getOutputType(), numRenderTargets, getUniforms(),
compileOptions, getComputeShaderLocalSize(), &getSymbolTable(),
diff --git a/src/compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.cpp b/src/compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.cpp
new file mode 100644
index 0000000..4253b5c
--- /dev/null
+++ b/src/compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.cpp
@@ -0,0 +1,89 @@
+//
+// Copyright (c) 2018 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Implementation of the function RewriteAtomicFunctionExpressions.
+// See the header for more details.
+
+#include "RewriteAtomicFunctionExpressions.h"
+
+#include "compiler/translator/tree_util/IntermNodePatternMatcher.h"
+#include "compiler/translator/tree_util/IntermNode_util.h"
+#include "compiler/translator/tree_util/IntermTraverse.h"
+
+namespace sh
+{
+namespace
+{
+// Traverser that simplifies all the atomic function expressions into the ones that can be directly
+// translated into HLSL.
+class RewriteAtomicFunctionExpressionsTraverser : public TIntermTraverser
+{
+ public:
+ RewriteAtomicFunctionExpressionsTraverser(TSymbolTable *symbolTable);
+
+ bool visitAggregate(Visit visit, TIntermAggregate *node) override;
+
+ private:
+ static bool IsAtomicExchangeOrCompSwapNoReturnValue(TIntermAggregate *node,
+ TIntermNode *parentNode);
+
+ void separateAtomicFunctionCallNode(TIntermAggregate *oldAtomicFunctionNode);
+};
+
+RewriteAtomicFunctionExpressionsTraverser::RewriteAtomicFunctionExpressionsTraverser(
+ TSymbolTable *symbolTable)
+ : TIntermTraverser(true, false, false, symbolTable)
+{
+}
+
+void RewriteAtomicFunctionExpressionsTraverser::separateAtomicFunctionCallNode(
+ TIntermAggregate *oldAtomicFunctionNode)
+{
+ ASSERT(oldAtomicFunctionNode);
+
+ TIntermSequence insertions;
+
+ // Declare a temporary variable
+ TIntermDeclaration *returnVariableDeclaration;
+ TVariable *returnVariable = DeclareTempVariable(mSymbolTable, &oldAtomicFunctionNode->getType(),
+ EvqTemporary, &returnVariableDeclaration);
+ insertions.push_back(returnVariableDeclaration);
+
+ // Use this variable as the return value of the atomic function call.
+ TIntermBinary *atomicFunctionAssignment = new TIntermBinary(
+ TOperator::EOpAssign, CreateTempSymbolNode(returnVariable), oldAtomicFunctionNode);
+
+ insertStatementsInParentBlock(insertions);
+ queueReplacement(atomicFunctionAssignment, OriginalNode::IS_DROPPED);
+}
+
+bool RewriteAtomicFunctionExpressionsTraverser::IsAtomicExchangeOrCompSwapNoReturnValue(
+ TIntermAggregate *node,
+ TIntermNode *parentNode)
+{
+ ASSERT(node);
+ return (node->getOp() == EOpAtomicExchange || node->getOp() == EOpAtomicCompSwap) &&
+ parentNode && parentNode->getAsBlock();
+}
+
+bool RewriteAtomicFunctionExpressionsTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
+{
+ if (IsAtomicExchangeOrCompSwapNoReturnValue(node, getParentNode()))
+ {
+ separateAtomicFunctionCallNode(node);
+ return false;
+ }
+
+ return true;
+}
+} // anonymous namespace
+
+void RewriteAtomicFunctionExpressions(TIntermNode *root, TSymbolTable *symbolTable)
+{
+ RewriteAtomicFunctionExpressionsTraverser traverser(symbolTable);
+ traverser.traverse(root);
+ traverser.updateTree();
+}
+} // namespace sh
\ No newline at end of file
diff --git a/src/compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.h b/src/compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.h
new file mode 100644
index 0000000..b33cf3e
--- /dev/null
+++ b/src/compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.h
@@ -0,0 +1,36 @@
+//
+// Copyright (c) 2018 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// RewriteAtomicFunctionExpressions rewrites the expressions that contain
+// atomic function calls and cannot be directly translated into HLSL into
+// several simple ones that can be easily handled in the HLSL translator.
+//
+// We need to rewite these expressions because:
+// 1. All GLSL atomic functions have return values, which all represent the
+// original value of the shared or ssbo variable; while all HLSL atomic
+// functions don't, and the original value can be stored in the last
+// parameter of the function call.
+// 2. For HLSL atomic functions, the last parameter that stores the original
+// value is optional except for InterlockedExchange and
+// InterlockedCompareExchange. Missing original_value in the call of
+// InterlockedExchange or InterlockedCompareExchange results in a compile
+// error from HLSL compiler.
+//
+// RewriteAtomicFunctionExpressions is a function that can modify the AST
+// to ensure all the expressions that contain atomic function calls can be
+// directly translated into HLSL expressions.
+
+#ifndef COMPILER_TRANSLATOR_TREEOPS_REWRITE_ATOMIC_FUNCTION_EXPRESSIONS_H_
+#define COMPILER_TRANSLATOR_TREEOPS_REWRITE_ATOMIC_FUNCTION_EXPRESSIONS_H_
+
+namespace sh
+{
+class TIntermNode;
+class TSymbolTable;
+
+void RewriteAtomicFunctionExpressions(TIntermNode *root, TSymbolTable *symbolTable);
+} // namespace sh
+
+#endif // COMPILER_TRANSLATOR_TREEOPS_REWRITE_ATOMIC_FUNCTION_EXPRESSIONS_H_
\ No newline at end of file
diff --git a/src/tests/gl_tests/ComputeShaderTest.cpp b/src/tests/gl_tests/ComputeShaderTest.cpp
index a22eaad..aa0d90e 100644
--- a/src/tests/gl_tests/ComputeShaderTest.cpp
+++ b/src/tests/gl_tests/ComputeShaderTest.cpp
@@ -1446,7 +1446,6 @@
}
// Verify using atomic functions without return value can work correctly.
-// TODO(jiawei.shao@intel.com): add test on atomicExchange and atomicCompSwap.
TEST_P(ComputeShaderTest, AtomicFunctionsNoReturnValue)
{
// TODO(jiawei.shao@intel.com): find out why this shader causes a link error on Android Nexus 5
@@ -1455,7 +1454,7 @@
const char kCSShader[] =
R"(#version 310 es
- layout (local_size_x = 6, local_size_y = 1, local_size_z = 1) in;
+ layout (local_size_x = 8, local_size_y = 1, local_size_z = 1) in;
layout (r32ui, binding = 0) readonly uniform highp uimage2D srcImage;
layout (r32ui, binding = 1) writeonly uniform highp uimage2D dstImage;
@@ -1465,8 +1464,10 @@
const uint kOrIndex = 3u;
const uint kAndIndex = 4u;
const uint kXorIndex = 5u;
+ const uint kExchangeIndex = 6u;
+ const uint kCompSwapIndex = 7u;
- shared highp uint results[6];
+ shared highp uint results[8];
void main()
{
@@ -1474,6 +1475,10 @@
{
results[gl_LocalInvocationID.x] = 0xFFFFu;
}
+ else if (gl_LocalInvocationID.x == kCompSwapIndex)
+ {
+ results[gl_LocalInvocationID.x] = 1u;
+ }
else
{
results[gl_LocalInvocationID.x] = 0u;
@@ -1488,6 +1493,8 @@
atomicOr(results[kOrIndex], value);
atomicAnd(results[kAndIndex], value);
atomicXor(results[kXorIndex], value);
+ atomicExchange(results[kExchangeIndex], value);
+ atomicCompSwap(results[kCompSwapIndex], value, 256u);
memoryBarrierShared();
barrier();
@@ -1495,9 +1502,9 @@
uvec4(results[gl_LocalInvocationID.x]));
})";
- const std::array<GLuint, 6> inputData = {{1, 2, 4, 8, 16, 32}};
- const std::array<GLuint, 6> expectedValues = {{63, 1, 32, 63, 0, 63}};
- runSharedMemoryTest<GLuint, 6, 1>(kCSShader, GL_R32UI, GL_UNSIGNED_INT, inputData,
+ const std::array<GLuint, 8> inputData = {{1, 2, 4, 8, 16, 32, 64, 128}};
+ const std::array<GLuint, 8> expectedValues = {{255, 1, 128, 255, 0, 255, 128, 256}};
+ runSharedMemoryTest<GLuint, 8, 1>(kCSShader, GL_R32UI, GL_UNSIGNED_INT, inputData,
expectedValues);
}