ES31: Enabling skipped deqp atomic counter tests on D3D11

include/GLSLANG/ShaderLang.h

@@ -26,7 +26,7 @@
 
 // Version number for shader translation API.
 // It is incremented every time the API changes.
-#define ANGLE_SH_VERSION 205
+#define ANGLE_SH_VERSION 206
 
 enum ShShaderSpec
 {
@@ -274,6 +274,13 @@ const ShCompileOptions SH_EMULATE_GL_DRAW_ID = UINT64_C(1) << 40;
 // another webpage/application.
 const ShCompileOptions SH_INIT_SHARED_VARIABLES = UINT64_C(1) << 41;
 
+// Forces the value returned from an atomic operations to be always be resolved. This is targeted to
+// workaround a bug in NVIDIA D3D driver where the return value from
+// RWByteAddressBuffer.InterlockedAdd does not get resolved when used in the .yzw components of a
+// RWByteAddressBuffer.Store operation. Only has an effect on HLSL translation.
+// http://anglebug.com/3246
+const ShCompileOptions SH_FORCE_ATOMIC_VALUE_RESOLUTION = UINT64_C(1) << 42;
+
 // Defines alternate strategies for implementing array index clamping.
 enum ShArrayIndexClampingStrategy
 {

include/platform/WorkaroundsD3D.h

@@ -131,6 +131,13 @@ struct WorkaroundsD3D
     // This is targeted to work around a bug in NVIDIA D3D driver version 388.59 where in very
     // specific cases the driver would not handle constant register zero correctly.
     bool skipVSConstantRegisterZero = false;
+
+    // Forces the value returned from an atomic operations to be always be resolved. This is
+    // targeted to workaround a bug in NVIDIA D3D driver where the return value from
+    // RWByteAddressBuffer.InterlockedAdd does not get resolved when used in the .yzw components of
+    // a RWByteAddressBuffer.Store operation. Only has an effect on HLSL translation.
+    // http://anglebug.com/3246
+    bool forceAtomicValueResolution = false;
 };
 
 inline WorkaroundsD3D::WorkaroundsD3D()                            = default;

src/compiler/translator/AtomicCounterFunctionHLSL.cpp

@@ -25,6 +25,10 @@ constexpr ImmutableString kAtomicCounterDecrement("atomicCounterDecrement");
 constexpr ImmutableString kAtomicCounterBaseName("_acbase_");
 }  // namespace
 
+AtomicCounterFunctionHLSL::AtomicCounterFunctionHLSL(bool forceResolution)
+    : mForceResolution(forceResolution)
+{}
+
 ImmutableString AtomicCounterFunctionHLSL::useAtomicCounterFunction(const ImmutableString &name)
 {
     // The largest string that will be create created is "_acbase_increment" or "_acbase_decrement"
@@ -66,28 +70,35 @@ void AtomicCounterFunctionHLSL::atomicCounterFunctionHeader(TInfoSinkBase &out)
     {
         out << "uint " << atomicFunction.first
             << "(in RWByteAddressBuffer counter, int address)\n"
-               "{\n";
+               "{\n"
+               "    uint ret;\n";
+
         switch (atomicFunction.second)
         {
             case AtomicCounterFunction::INCREMENT:
+                out << "    counter.InterlockedAdd(address, 1u, ret);\n";
+                break;
             case AtomicCounterFunction::DECREMENT:
-                out << "    uint ret;\n"
-                       "    counter.InterlockedAdd(address, ";
-                if (atomicFunction.second == AtomicCounterFunction::DECREMENT)
-                {
-                    out << "0u - ";
-                }
-                out << "1u, ret);\n"
-                    << "    return ret;\n";
+                out << "    counter.InterlockedAdd(address, 0u - 1u, ret);\n"
+                       "    ret -= 1u;\n";  // atomicCounterDecrement is a post-decrement op
                 break;
             case AtomicCounterFunction::LOAD:
-                out << "    return counter.Load(address);\n";
+                out << "    ret = counter.Load(address);\n";
                 break;
             default:
                 UNREACHABLE();
                 break;
         }
-        out << "}\n\n";
+
+        if (mForceResolution && atomicFunction.second != AtomicCounterFunction::LOAD)
+        {
+            out << "    if (ret == 0) {\n"
+                   "        ret = 0 - ret;\n"
+                   "    }\n";
+        }
+
+        out << "    return ret;\n"
+               "}\n\n";
     }
 }
 

src/compiler/translator/AtomicCounterFunctionHLSL.h

@@ -24,6 +24,8 @@ struct TLayoutQualifier;
 class AtomicCounterFunctionHLSL final : angle::NonCopyable
 {
   public:
+    AtomicCounterFunctionHLSL(bool forceResolution);
+
     ImmutableString useAtomicCounterFunction(const ImmutableString &name);
 
     void atomicCounterFunctionHeader(TInfoSinkBase &out);
@@ -38,6 +40,7 @@ class AtomicCounterFunctionHLSL final : angle::NonCopyable
     };
 
     std::map<ImmutableString, AtomicCounterFunction> mAtomicCounterFunctions;
+    bool mForceResolution;
 };
 
 ImmutableString getAtomicCounterNameForBinding(int binding);

src/compiler/translator/OutputHLSL.cpp

@@ -285,10 +285,11 @@ OutputHLSL::OutputHLSL(sh::GLenum shaderType,
 
     mExcessiveLoopIndex = nullptr;
 
-    mStructureHLSL             = new StructureHLSL;
-    mTextureFunctionHLSL       = new TextureFunctionHLSL;
-    mImageFunctionHLSL         = new ImageFunctionHLSL;
-    mAtomicCounterFunctionHLSL = new AtomicCounterFunctionHLSL;
+    mStructureHLSL       = new StructureHLSL;
+    mTextureFunctionHLSL = new TextureFunctionHLSL;
+    mImageFunctionHLSL   = new ImageFunctionHLSL;
+    mAtomicCounterFunctionHLSL =
+        new AtomicCounterFunctionHLSL((compileOptions & SH_FORCE_ATOMIC_VALUE_RESOLUTION) != 0);
 
     unsigned int firstUniformRegister =
         ((compileOptions & SH_SKIP_D3D_CONSTANT_REGISTER_ZERO) != 0) ? 1u : 0u;

src/compiler/translator/ParseContext.cpp

@@ -2366,11 +2366,6 @@ void TParseContext::checkAtomicCounterOffsetDoesNotOverlap(bool forceAppend,
                                                            const TSourceLoc &loc,
                                                            TType *type)
 {
-    if (!IsAtomicCounter(type->getBasicType()))
-    {
-        return;
-    }
-
     const size_t size = type->isArray() ? kAtomicCounterArrayStride * type->getArraySizeProduct()
                                         : kAtomicCounterSize;
     TLayoutQualifier layoutQualifier = type->getLayoutQualifier();
@@ -2393,6 +2388,17 @@ void TParseContext::checkAtomicCounterOffsetDoesNotOverlap(bool forceAppend,
     type->setLayoutQualifier(layoutQualifier);
 }
 
+void TParseContext::checkAtomicCounterOffsetAlignment(const TSourceLoc &location, const TType &type)
+{
+    TLayoutQualifier layoutQualifier = type.getLayoutQualifier();
+
+    // OpenGL ES 3.1 Table 6.5, Atomic counter offset must be a multiple of 4
+    if (layoutQualifier.offset % 4 != 0)
+    {
+        error(location, "Offset must be multiple of 4", "atomic counter");
+    }
+}
+
 void TParseContext::checkGeometryShaderInputAndSetArraySize(const TSourceLoc &location,
                                                             const ImmutableString &token,
                                                             TType *type)
@@ -2495,7 +2501,12 @@ TIntermDeclaration *TParseContext::parseSingleDeclaration(
 
         checkCanBeDeclaredWithoutInitializer(identifierOrTypeLocation, identifier, type);
 
-        checkAtomicCounterOffsetDoesNotOverlap(false, identifierOrTypeLocation, type);
+        if (IsAtomicCounter(type->getBasicType()))
+        {
+            checkAtomicCounterOffsetDoesNotOverlap(false, identifierOrTypeLocation, type);
+
+            checkAtomicCounterOffsetAlignment(identifierOrTypeLocation, *type);
+        }
 
         TVariable *variable = nullptr;
         if (declareVariable(identifierOrTypeLocation, identifier, type, &variable))
@@ -2537,7 +2548,12 @@ TIntermDeclaration *TParseContext::parseSingleArrayDeclaration(
 
     checkCanBeDeclaredWithoutInitializer(identifierLocation, identifier, arrayType);
 
-    checkAtomicCounterOffsetDoesNotOverlap(false, identifierLocation, arrayType);
+    if (IsAtomicCounter(arrayType->getBasicType()))
+    {
+        checkAtomicCounterOffsetDoesNotOverlap(false, identifierLocation, arrayType);
+
+        checkAtomicCounterOffsetAlignment(identifierLocation, *arrayType);
+    }
 
     TIntermDeclaration *declaration = new TIntermDeclaration();
     declaration->setLine(identifierLocation);
@@ -2695,7 +2711,12 @@ void TParseContext::parseDeclarator(TPublicType &publicType,
 
     checkCanBeDeclaredWithoutInitializer(identifierLocation, identifier, type);
 
-    checkAtomicCounterOffsetDoesNotOverlap(true, identifierLocation, type);
+    if (IsAtomicCounter(type->getBasicType()))
+    {
+        checkAtomicCounterOffsetDoesNotOverlap(true, identifierLocation, type);
+
+        checkAtomicCounterOffsetAlignment(identifierLocation, *type);
+    }
 
     TVariable *variable = nullptr;
     if (declareVariable(identifierLocation, identifier, type, &variable))
@@ -2732,7 +2753,12 @@ void TParseContext::parseArrayDeclarator(TPublicType &elementType,
 
         checkCanBeDeclaredWithoutInitializer(identifierLocation, identifier, arrayType);
 
-        checkAtomicCounterOffsetDoesNotOverlap(true, identifierLocation, arrayType);
+        if (IsAtomicCounter(arrayType->getBasicType()))
+        {
+            checkAtomicCounterOffsetDoesNotOverlap(true, identifierLocation, arrayType);
+
+            checkAtomicCounterOffsetAlignment(identifierLocation, *arrayType);
+        }
 
         TVariable *variable = nullptr;
         if (declareVariable(identifierLocation, identifier, arrayType, &variable))

src/compiler/translator/ParseContext.h

@@ -515,6 +515,8 @@ class TParseContext : angle::NonCopyable
     void checkAtomicCounterOffsetDoesNotOverlap(bool forceAppend,
                                                 const TSourceLoc &loc,
                                                 TType *type);
+    void checkAtomicCounterOffsetAlignment(const TSourceLoc &location, const TType &type);
+
     void checkIndexIsNotSpecified(const TSourceLoc &location, int index);
     void checkBindingIsValid(const TSourceLoc &identifierLocation, const TType &type);
     void checkBindingIsNotSpecified(const TSourceLoc &location, int binding);

src/compiler/translator/ResourcesHLSL.cpp

@@ -385,8 +385,9 @@ void ResourcesHLSL::outputAtomicCounterBuffer(TInfoSinkBase &out,
                                               const int binding,
                                               const unsigned int registerIndex)
 {
-    out << "uniform RWByteAddressBuffer " << getAtomicCounterNameForBinding(binding)
-        << " : register(u" << registerIndex << ");\n";
+    // Atomic counter memory access is not incoherent
+    out << "uniform globallycoherent RWByteAddressBuffer "
+        << getAtomicCounterNameForBinding(binding) << " : register(u" << registerIndex << ");\n";
 }
 
 void ResourcesHLSL::uniformsHeader(TInfoSinkBase &out,

src/libANGLE/renderer/d3d/ShaderD3D.cpp

@@ -118,6 +118,10 @@ ShaderD3D::ShaderD3D(const gl::ShaderState &data,
     {
         mAdditionalOptions |= SH_SKIP_D3D_CONSTANT_REGISTER_ZERO;
     }
+    if (workarounds.forceAtomicValueResolution)
+    {
+        mAdditionalOptions |= SH_FORCE_ATOMIC_VALUE_RESOLUTION;
+    }
     if (extensions.multiview)
     {
         mAdditionalOptions |= SH_INITIALIZE_BUILTINS_FOR_INSTANCED_MULTIVIEW;

src/libANGLE/renderer/d3d/d3d11/renderer11_utils.cpp

@@ -2383,6 +2383,7 @@ angle::WorkaroundsD3D GenerateWorkarounds(const Renderer11DeviceCaps &deviceCaps
     workarounds.flushAfterEndingTransformFeedback = IsNvidia(adapterDesc.VendorId);
     workarounds.getDimensionsIgnoresBaseLevel     = IsNvidia(adapterDesc.VendorId);
     workarounds.skipVSConstantRegisterZero        = IsNvidia(adapterDesc.VendorId);
+    workarounds.forceAtomicValueResolution        = IsNvidia(adapterDesc.VendorId);
 
     if (IsIntel(adapterDesc.VendorId))
     {

src/tests/deqp_support/deqp_gles31_test_expectations.txt

@@ -46,7 +46,6 @@
 // test expectations parser doesn't support having FAIL for GL and SKIP for D3D with the same test filter.
 1442 OPENGL D3D11 : dEQP-GLES31.functional.image_load_store.* = SKIP
 
-1729 D3D11 : dEQP-GLES31.functional.atomic_counter.* = SKIP
 1951 D3D11 : dEQP-GLES31.functional.layout_binding.ssbo.* = SKIP
 1951 D3D11 : dEQP-GLES31.functional.shaders.opaque_type_indexing.ssbo.* = SKIP
 1442 D3D11 : dEQP-GLES31.functional.program_interface_query.* = SKIP