ES31: Support atomic functions on D3D11 - Part II

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',

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 @@ void TranslatorHLSL::translate(TIntermBlock *root,
         sh::RewriteUnaryMinusOperatorInt(root);
     }
 
+    if (getShaderVersion() >= 310)
+    {
+        sh::RewriteAtomicFunctionExpressions(root, &getSymbolTable());
+    }
+
     sh::OutputHLSL outputHLSL(getShaderType(), getShaderVersion(), getExtensionBehavior(),
                               getSourcePath(), getOutputType(), numRenderTargets, getUniforms(),
                               compileOptions, getComputeShaderLocalSize(), &getSymbolTable(),

src/compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.cpp

+//
+// 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

src/compiler/translator/tree_ops/RewriteAtomicFunctionExpressions.h

+//
+// 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

src/tests/gl_tests/ComputeShaderTest.cpp

@@ -1446,7 +1446,6 @@ TEST_P(ComputeShaderTest, StructArrayAsSharedVariable)
 }
 
 // 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 @@ TEST_P(ComputeShaderTest, AtomicFunctionsNoReturnValue)
 
     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 @@ TEST_P(ComputeShaderTest, AtomicFunctionsNoReturnValue)
         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 @@ TEST_P(ComputeShaderTest, AtomicFunctionsNoReturnValue)
             {
                 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 @@ TEST_P(ComputeShaderTest, AtomicFunctionsNoReturnValue)
             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 @@ TEST_P(ComputeShaderTest, AtomicFunctionsNoReturnValue)
                        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);
 }