From 6d765b07f86f189db4164be0171e36fd7ddf5f3a Mon Sep 17 00:00:00 2001 From: Qin Jiajia Date: Fri, 28 Sep 2018 14:16:06 +0800 Subject: [PATCH] ES31: Fix some bugs in ShaderStorageBlockOutputHLSL When EOpIndexDirect/EOpIndexIndirect/EOpIndexDirectStruct/TIntermSwizzle appear in [] in ssbo access chain, we should transfer the process of them to OutputHLSL. For example: instance.v[gl_GlobalInvocationID.x] = data; // becomes float_Store(dx_instance, 0 + 16 * gl_GlobalInvocationID.x, _data); instance.v[s.index[0].x] = data; // becomes float_Store(dx_instance, 0 + 16 * _s.index[0].x, _data); Bug: angleproject:1951 Change-Id: I333e238400a10a799a6294f8759cf9c4ef2451c8 Reviewed-on: https://chromium-review.googlesource.com/c/1250661 Reviewed-by: Corentin Wallez Reviewed-by: Geoff Lang Commit-Queue: Jiajia Qin --- .../ShaderStorageBlockOutputHLSL.cpp | 27 ++++++- src/compiler/translator/util.cpp | 5 ++ src/tests/gl_tests/ComputeShaderTest.cpp | 76 +++++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) diff --git a/src/compiler/translator/ShaderStorageBlockOutputHLSL.cpp b/src/compiler/translator/ShaderStorageBlockOutputHLSL.cpp index 28b7ac898..6a346cacf 100644 --- a/src/compiler/translator/ShaderStorageBlockOutputHLSL.cpp +++ b/src/compiler/translator/ShaderStorageBlockOutputHLSL.cpp @@ -227,9 +227,14 @@ void ShaderStorageBlockOutputHLSL::visitConstantUnion(TIntermConstantUnion *node bool ShaderStorageBlockOutputHLSL::visitSwizzle(Visit visit, TIntermSwizzle *node) { - TInfoSinkBase &out = mOutputHLSL->getInfoSink(); if (visit == PostVisit) { + if (!IsInShaderStorageBlock(node)) + { + return mOutputHLSL->visitSwizzle(visit, node); + } + + TInfoSinkBase &out = mOutputHLSL->getInfoSink(); // TODO(jiajia.qin@intel.com): add swizzle process. if (mIsLoadFunctionCall) { @@ -247,6 +252,11 @@ bool ShaderStorageBlockOutputHLSL::visitBinary(Visit visit, TIntermBinary *node) { case EOpIndexDirect: { + if (!IsInShaderStorageBlock(node->getLeft())) + { + return mOutputHLSL->visitBinary(visit, node); + } + const TType &leftType = node->getLeft()->getType(); if (leftType.isInterfaceBlock()) { @@ -276,21 +286,36 @@ bool ShaderStorageBlockOutputHLSL::visitBinary(Visit visit, TIntermBinary *node) break; } case EOpIndexIndirect: + { + if (!IsInShaderStorageBlock(node->getLeft())) + { + return mOutputHLSL->visitBinary(visit, node); + } + // We do not currently support indirect references to interface blocks ASSERT(node->getLeft()->getBasicType() != EbtInterfaceBlock); writeEOpIndexDirectOrIndirectOutput(out, visit, node); break; + } case EOpIndexDirectStruct: + { + if (!IsInShaderStorageBlock(node->getLeft())) + { + return mOutputHLSL->visitBinary(visit, node); + } + if (visit == InVisit) { ASSERT(IsInShaderStorageBlock(node->getLeft())); const TStructure *structure = node->getLeft()->getType().getStruct(); const TIntermConstantUnion *index = node->getRight()->getAsConstantUnion(); const TField *field = structure->fields()[index->getIConst(0)]; + out << " + "; writeDotOperatorOutput(out, field); return false; } break; + } case EOpIndexDirectInterfaceBlock: if (visit == InVisit) { diff --git a/src/compiler/translator/util.cpp b/src/compiler/translator/util.cpp index 37bf48e92..72ac655a1 100644 --- a/src/compiler/translator/util.cpp +++ b/src/compiler/translator/util.cpp @@ -762,6 +762,11 @@ bool IsInShaderStorageBlock(TIntermTyped *node) { return IsInShaderStorageBlock(binaryNode->getLeft()); } + TIntermSymbol *symbolNode = swizzleNode->getOperand()->getAsSymbolNode(); + if (symbolNode) + { + return symbolNode->getQualifier() == EvqBuffer; + } } binaryNode = node->getAsBinaryNode(); diff --git a/src/tests/gl_tests/ComputeShaderTest.cpp b/src/tests/gl_tests/ComputeShaderTest.cpp index 679b75337..b49e85dc6 100644 --- a/src/tests/gl_tests/ComputeShaderTest.cpp +++ b/src/tests/gl_tests/ComputeShaderTest.cpp @@ -2190,6 +2190,82 @@ TEST_P(ComputeShaderTest, ShaderStorageBlocksWithUnsizedArray) EXPECT_GL_NO_ERROR(); } +// Test that EOpIndexDirect/EOpIndexIndirect/EOpIndexDirectStruct nodes in ssbo EOpIndexInDirect +// don't need to calculate the offset and should be translated by OutputHLSL directly. +TEST_P(ComputeShaderTest, IndexAndDotOperatorsInSSBOIndexIndirectOperator) +{ + constexpr char kComputeShaderSource[] = + R"(#version 310 es + layout(local_size_x=1) in; + layout(std140, binding = 0) buffer blockA { + float v[4]; + }; + layout(std140, binding = 1) buffer blockB { + float v[4]; + } instanceB[1]; + struct S + { + uvec4 index[2]; + } s; + void main() + { + s.index[0] = uvec4(0u, 1u, 2u, 3u); + float data = v[s.index[0].y]; + instanceB[0].v[s.index[0].x] = data; + } + )"; + + ANGLE_GL_COMPUTE_PROGRAM(program, kComputeShaderSource); + EXPECT_GL_NO_ERROR(); +} + +// Test that swizzle node in non-SSBO symbol works well. +TEST_P(ComputeShaderTest, ShaderStorageBlocksWithNonSSBOSwizzle) +{ + constexpr char kComputeShaderSource[] = + R"(#version 310 es + layout(local_size_x=8) in; + layout(std140, binding = 0) buffer blockA { + float v[8]; + }; + layout(std140, binding = 1) buffer blockB { + float v[8]; + } instanceB[1]; + + void main() + { + float data = v[gl_GlobalInvocationID.x]; + instanceB[0].v[gl_GlobalInvocationID.x] = data; + } + )"; + + ANGLE_GL_COMPUTE_PROGRAM(program, kComputeShaderSource); + EXPECT_GL_NO_ERROR(); +} + +// Test that swizzle node in SSBO symbol works well. +TEST_P(ComputeShaderTest, ShaderStorageBlocksWithSSBOSwizzle) +{ + constexpr char kComputeShaderSource[] = + R"(#version 310 es + layout(local_size_x=1) in; + layout(std140, binding = 0) buffer blockA { + vec2 v; + }; + layout(std140, binding = 1) buffer blockB { + float v; + } instanceB[1]; + + void main() + { + instanceB[0].v = v.x; + } + )"; + + ANGLE_GL_COMPUTE_PROGRAM(program, kComputeShaderSource); + EXPECT_GL_NO_ERROR(); +} + // Check that it is not possible to create a compute shader when the context does not support ES // 3.10 TEST_P(ComputeShaderTestES3, NotSupported)