Commit ba0bd785 by Qin Jiajia Committed by Commit Bot

Fix the assert error and inbalence parens for SSBO

This PR strengthens below two cases: 1. Support calculate unsized array size for any type. Previously, it reported error when the type was a structure. 2. Correctly add the right paren for store function when any load/length functions are nested in it. Bug: angleproject:5734 Change-Id: I3428fa35f1481275e1d193094ceddb011f747cf1 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2762655Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Commit-Queue: Jiajia Qin <jiajia.qin@intel.com>
parent b717952e
...@@ -311,7 +311,6 @@ ShaderStorageBlockOutputHLSL::ShaderStorageBlockOutputHLSL( ...@@ -311,7 +311,6 @@ ShaderStorageBlockOutputHLSL::ShaderStorageBlockOutputHLSL(
: TIntermTraverser(true, true, true, symbolTable), : TIntermTraverser(true, true, true, symbolTable),
mMatrixStride(0), mMatrixStride(0),
mRowMajor(false), mRowMajor(false),
mLocationAsTheLastArgument(false),
mOutputHLSL(outputHLSL), mOutputHLSL(outputHLSL),
mResourcesHLSL(resourcesHLSL), mResourcesHLSL(resourcesHLSL),
mShaderStorageBlocks(shaderStorageBlocks) mShaderStorageBlocks(shaderStorageBlocks)
...@@ -326,51 +325,57 @@ ShaderStorageBlockOutputHLSL::~ShaderStorageBlockOutputHLSL() ...@@ -326,51 +325,57 @@ ShaderStorageBlockOutputHLSL::~ShaderStorageBlockOutputHLSL()
void ShaderStorageBlockOutputHLSL::outputStoreFunctionCallPrefix(TIntermTyped *node) void ShaderStorageBlockOutputHLSL::outputStoreFunctionCallPrefix(TIntermTyped *node)
{ {
mLocationAsTheLastArgument = false; mMethodTypeStack.push(SSBOMethod::STORE);
traverseSSBOAccess(node, SSBOMethod::STORE); traverseSSBOAccess(node, SSBOMethod::STORE);
} }
void ShaderStorageBlockOutputHLSL::outputLoadFunctionCall(TIntermTyped *node) void ShaderStorageBlockOutputHLSL::outputLoadFunctionCall(TIntermTyped *node)
{ {
mLocationAsTheLastArgument = true; mMethodTypeStack.push(SSBOMethod::LOAD);
traverseSSBOAccess(node, SSBOMethod::LOAD); traverseSSBOAccess(node, SSBOMethod::LOAD);
} }
void ShaderStorageBlockOutputHLSL::outputLengthFunctionCall(TIntermTyped *node) void ShaderStorageBlockOutputHLSL::outputLengthFunctionCall(TIntermTyped *node)
{ {
mLocationAsTheLastArgument = true; mMethodTypeStack.push(SSBOMethod::LENGTH);
traverseSSBOAccess(node, SSBOMethod::LENGTH); traverseSSBOAccess(node, SSBOMethod::LENGTH);
} }
void ShaderStorageBlockOutputHLSL::outputAtomicMemoryFunctionCallPrefix(TIntermTyped *node, void ShaderStorageBlockOutputHLSL::outputAtomicMemoryFunctionCallPrefix(TIntermTyped *node,
TOperator op) TOperator op)
{ {
mLocationAsTheLastArgument = false;
switch (op) switch (op)
{ {
case EOpAtomicAdd: case EOpAtomicAdd:
mMethodTypeStack.push(SSBOMethod::ATOMIC_ADD);
traverseSSBOAccess(node, SSBOMethod::ATOMIC_ADD); traverseSSBOAccess(node, SSBOMethod::ATOMIC_ADD);
break; break;
case EOpAtomicMin: case EOpAtomicMin:
mMethodTypeStack.push(SSBOMethod::ATOMIC_MIN);
traverseSSBOAccess(node, SSBOMethod::ATOMIC_MIN); traverseSSBOAccess(node, SSBOMethod::ATOMIC_MIN);
break; break;
case EOpAtomicMax: case EOpAtomicMax:
mMethodTypeStack.push(SSBOMethod::ATOMIC_MAX);
traverseSSBOAccess(node, SSBOMethod::ATOMIC_MAX); traverseSSBOAccess(node, SSBOMethod::ATOMIC_MAX);
break; break;
case EOpAtomicAnd: case EOpAtomicAnd:
mMethodTypeStack.push(SSBOMethod::ATOMIC_AND);
traverseSSBOAccess(node, SSBOMethod::ATOMIC_AND); traverseSSBOAccess(node, SSBOMethod::ATOMIC_AND);
break; break;
case EOpAtomicOr: case EOpAtomicOr:
mMethodTypeStack.push(SSBOMethod::ATOMIC_OR);
traverseSSBOAccess(node, SSBOMethod::ATOMIC_OR); traverseSSBOAccess(node, SSBOMethod::ATOMIC_OR);
break; break;
case EOpAtomicXor: case EOpAtomicXor:
mMethodTypeStack.push(SSBOMethod::ATOMIC_XOR);
traverseSSBOAccess(node, SSBOMethod::ATOMIC_XOR); traverseSSBOAccess(node, SSBOMethod::ATOMIC_XOR);
break; break;
case EOpAtomicExchange: case EOpAtomicExchange:
mMethodTypeStack.push(SSBOMethod::ATOMIC_EXCHANGE);
traverseSSBOAccess(node, SSBOMethod::ATOMIC_EXCHANGE); traverseSSBOAccess(node, SSBOMethod::ATOMIC_EXCHANGE);
break; break;
case EOpAtomicCompSwap: case EOpAtomicCompSwap:
mMethodTypeStack.push(SSBOMethod::ATOMIC_COMPSWAP);
traverseSSBOAccess(node, SSBOMethod::ATOMIC_COMPSWAP); traverseSSBOAccess(node, SSBOMethod::ATOMIC_COMPSWAP);
break; break;
default: default:
...@@ -411,8 +416,55 @@ void ShaderStorageBlockOutputHLSL::setMatrixStride(TIntermTyped *node, ...@@ -411,8 +416,55 @@ void ShaderStorageBlockOutputHLSL::setMatrixStride(TIntermTyped *node,
} }
} }
void ShaderStorageBlockOutputHLSL::collectShaderStorageBlocks(TIntermTyped *node)
{
TIntermSwizzle *swizzleNode = node->getAsSwizzleNode();
if (swizzleNode)
{
return collectShaderStorageBlocks(swizzleNode->getOperand());
}
TIntermBinary *binaryNode = node->getAsBinaryNode();
if (binaryNode)
{
switch (binaryNode->getOp())
{
case EOpIndexDirectInterfaceBlock:
case EOpIndexIndirect:
case EOpIndexDirect:
case EOpIndexDirectStruct:
return collectShaderStorageBlocks(binaryNode->getLeft());
default:
UNREACHABLE();
return;
}
}
const TIntermSymbol *symbolNode = node->getAsSymbolNode();
const TType &type = symbolNode->getType();
ASSERT(type.getQualifier() == EvqBuffer);
const TVariable &variable = symbolNode->variable();
const TInterfaceBlock *interfaceBlock = type.getInterfaceBlock();
ASSERT(interfaceBlock);
if (mReferencedShaderStorageBlocks.count(interfaceBlock->uniqueId().get()) == 0)
{
const TVariable *instanceVariable = nullptr;
if (type.isInterfaceBlock())
{
instanceVariable = &variable;
}
mReferencedShaderStorageBlocks[interfaceBlock->uniqueId().get()] =
new TReferencedBlock(interfaceBlock, instanceVariable);
GetShaderStorageBlockMembersInfo(interfaceBlock, mShaderStorageBlocks,
&mBlockMemberInfoMap);
}
}
void ShaderStorageBlockOutputHLSL::traverseSSBOAccess(TIntermTyped *node, SSBOMethod method) void ShaderStorageBlockOutputHLSL::traverseSSBOAccess(TIntermTyped *node, SSBOMethod method)
{ {
// TODO: Merge collectShaderStorageBlocks and GetBlockLayoutInfo to simplify the code.
collectShaderStorageBlocks(node);
mMatrixStride = 0; mMatrixStride = 0;
mRowMajor = false; mRowMajor = false;
...@@ -425,8 +477,31 @@ void ShaderStorageBlockOutputHLSL::traverseSSBOAccess(TIntermTyped *node, SSBOMe ...@@ -425,8 +477,31 @@ void ShaderStorageBlockOutputHLSL::traverseSSBOAccess(TIntermTyped *node, SSBOMe
int unsizedArrayStride = 0; int unsizedArrayStride = 0;
if (node->getType().isUnsizedArray()) if (node->getType().isUnsizedArray())
{ {
unsizedArrayStride = // The unsized array member must be the last member of a shader storage block.
GetBlockMemberInfoByType(node->getType(), storage, rowMajor).arrayStride; TIntermBinary *binaryNode = node->getAsBinaryNode();
if (binaryNode)
{
const TInterfaceBlock *interfaceBlock =
binaryNode->getLeft()->getType().getInterfaceBlock();
ASSERT(interfaceBlock);
const TIntermConstantUnion *index = binaryNode->getRight()->getAsConstantUnion();
const TField *field = interfaceBlock->fields()[index->getIConst(0)];
auto fieldInfoIter = mBlockMemberInfoMap.find(field);
ASSERT(fieldInfoIter != mBlockMemberInfoMap.end());
unsizedArrayStride = fieldInfoIter->second.arrayStride;
}
else
{
const TIntermSymbol *symbolNode = node->getAsSymbolNode();
const TVariable &variable = symbolNode->variable();
const TInterfaceBlock *interfaceBlock = symbolNode->getType().getInterfaceBlock();
ASSERT(interfaceBlock);
const TField *field =
GetFieldMemberInShaderStorageBlock(interfaceBlock, variable.name());
auto fieldInfoIter = mBlockMemberInfoMap.find(field);
ASSERT(fieldInfoIter != mBlockMemberInfoMap.end());
unsizedArrayStride = fieldInfoIter->second.arrayStride;
}
} }
setMatrixStride(node, storage, rowMajor); setMatrixStride(node, storage, rowMajor);
...@@ -488,18 +563,6 @@ void ShaderStorageBlockOutputHLSL::visitSymbol(TIntermSymbol *node) ...@@ -488,18 +563,6 @@ void ShaderStorageBlockOutputHLSL::visitSymbol(TIntermSymbol *node)
const TType &variableType = variable.getType(); const TType &variableType = variable.getType();
const TInterfaceBlock *interfaceBlock = variableType.getInterfaceBlock(); const TInterfaceBlock *interfaceBlock = variableType.getInterfaceBlock();
ASSERT(interfaceBlock); ASSERT(interfaceBlock);
if (mReferencedShaderStorageBlocks.count(interfaceBlock->uniqueId().get()) == 0)
{
const TVariable *instanceVariable = nullptr;
if (variableType.isInterfaceBlock())
{
instanceVariable = &variable;
}
mReferencedShaderStorageBlocks[interfaceBlock->uniqueId().get()] =
new TReferencedBlock(interfaceBlock, instanceVariable);
GetShaderStorageBlockMembersInfo(interfaceBlock, mShaderStorageBlocks,
&mBlockMemberInfoMap);
}
if (variableType.isInterfaceBlock()) if (variableType.isInterfaceBlock())
{ {
out << DecorateVariableIfNeeded(variable); out << DecorateVariableIfNeeded(variable);
...@@ -552,10 +615,16 @@ bool ShaderStorageBlockOutputHLSL::visitSwizzle(Visit visit, TIntermSwizzle *nod ...@@ -552,10 +615,16 @@ bool ShaderStorageBlockOutputHLSL::visitSwizzle(Visit visit, TIntermSwizzle *nod
TInfoSinkBase &out = mOutputHLSL->getInfoSink(); TInfoSinkBase &out = mOutputHLSL->getInfoSink();
// TODO(jiajia.qin@intel.com): add swizzle process if the swizzle node is not the last node // TODO(jiajia.qin@intel.com): add swizzle process if the swizzle node is not the last node
// of ssbo access chain. Such as, data.xy[0] // of ssbo access chain. Such as, data.xy[0]
if (mLocationAsTheLastArgument && isEndOfSSBOAccessChain()) if (isEndOfSSBOAccessChain())
{
ASSERT(!mMethodTypeStack.empty());
SSBOMethod curMethod = mMethodTypeStack.top();
if (curMethod == SSBOMethod::LENGTH || curMethod == SSBOMethod::LOAD)
{ {
out << ")"; out << ")";
} }
mMethodTypeStack.pop();
}
} }
return true; return true;
} }
...@@ -580,15 +649,6 @@ bool ShaderStorageBlockOutputHLSL::visitBinary(Visit visit, TIntermBinary *node) ...@@ -580,15 +649,6 @@ bool ShaderStorageBlockOutputHLSL::visitBinary(Visit visit, TIntermBinary *node)
{ {
ASSERT(leftType.getQualifier() == EvqBuffer); ASSERT(leftType.getQualifier() == EvqBuffer);
TIntermSymbol *instanceArraySymbol = node->getLeft()->getAsSymbolNode(); TIntermSymbol *instanceArraySymbol = node->getLeft()->getAsSymbolNode();
const TInterfaceBlock *interfaceBlock = leftType.getInterfaceBlock();
if (mReferencedShaderStorageBlocks.count(interfaceBlock->uniqueId().get()) == 0)
{
mReferencedShaderStorageBlocks[interfaceBlock->uniqueId().get()] =
new TReferencedBlock(interfaceBlock, &instanceArraySymbol->variable());
GetShaderStorageBlockMembersInfo(interfaceBlock, mShaderStorageBlocks,
&mBlockMemberInfoMap);
}
const int arrayIndex = node->getRight()->getAsConstantUnion()->getIConst(0); const int arrayIndex = node->getRight()->getAsConstantUnion()->getIConst(0);
out << mResourcesHLSL->InterfaceBlockInstanceString( out << mResourcesHLSL->InterfaceBlockInstanceString(
...@@ -727,10 +787,16 @@ void ShaderStorageBlockOutputHLSL::writeEOpIndexDirectOrIndirectOutput(TInfoSink ...@@ -727,10 +787,16 @@ void ShaderStorageBlockOutputHLSL::writeEOpIndexDirectOrIndirectOutput(TInfoSink
{ {
out << ")"; out << ")";
} }
if (mLocationAsTheLastArgument && isEndOfSSBOAccessChain()) if (isEndOfSSBOAccessChain())
{
ASSERT(!mMethodTypeStack.empty());
SSBOMethod curMethod = mMethodTypeStack.top();
if (curMethod == SSBOMethod::LENGTH || curMethod == SSBOMethod::LOAD)
{ {
out << ")"; out << ")";
} }
mMethodTypeStack.pop();
}
} }
} }
...@@ -753,10 +819,16 @@ void ShaderStorageBlockOutputHLSL::writeDotOperatorOutput(TInfoSinkBase &out, co ...@@ -753,10 +819,16 @@ void ShaderStorageBlockOutputHLSL::writeDotOperatorOutput(TInfoSinkBase &out, co
out << " * ("; out << " * (";
} }
} }
if (mLocationAsTheLastArgument && isEndOfSSBOAccessChain()) if (isEndOfSSBOAccessChain())
{
ASSERT(!mMethodTypeStack.empty());
SSBOMethod curMethod = mMethodTypeStack.top();
if (curMethod == SSBOMethod::LENGTH || curMethod == SSBOMethod::LOAD)
{ {
out << ")"; out << ")";
} }
mMethodTypeStack.pop();
}
} }
} // namespace sh } // namespace sh
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#ifndef COMPILER_TRANSLATOR_SHADERSTORAGEBLOCKOUTPUTHLSL_H_ #ifndef COMPILER_TRANSLATOR_SHADERSTORAGEBLOCKOUTPUTHLSL_H_
#define COMPILER_TRANSLATOR_SHADERSTORAGEBLOCKOUTPUTHLSL_H_ #define COMPILER_TRANSLATOR_SHADERSTORAGEBLOCKOUTPUTHLSL_H_
#include <stack>
#include "compiler/translator/ShaderStorageBlockFunctionHLSL.h" #include "compiler/translator/ShaderStorageBlockFunctionHLSL.h"
#include "compiler/translator/blocklayout.h" #include "compiler/translator/blocklayout.h"
#include "compiler/translator/tree_util/IntermTraverse.h" #include "compiler/translator/tree_util/IntermTraverse.h"
...@@ -75,10 +76,11 @@ class ShaderStorageBlockOutputHLSL : public TIntermTraverser ...@@ -75,10 +76,11 @@ class ShaderStorageBlockOutputHLSL : public TIntermTraverser
void writeEOpIndexDirectOrIndirectOutput(TInfoSinkBase &out, Visit visit, TIntermBinary *node); void writeEOpIndexDirectOrIndirectOutput(TInfoSinkBase &out, Visit visit, TIntermBinary *node);
// Common part in dot operations. // Common part in dot operations.
void writeDotOperatorOutput(TInfoSinkBase &out, const TField *field); void writeDotOperatorOutput(TInfoSinkBase &out, const TField *field);
void collectShaderStorageBlocks(TIntermTyped *node);
int mMatrixStride; int mMatrixStride;
bool mRowMajor; bool mRowMajor;
bool mLocationAsTheLastArgument; std::stack<SSBOMethod> mMethodTypeStack;
OutputHLSL *mOutputHLSL; OutputHLSL *mOutputHLSL;
ShaderStorageBlockFunctionHLSL *mSSBOFunctionHLSL; ShaderStorageBlockFunctionHLSL *mSSBOFunctionHLSL;
ResourcesHLSL *mResourcesHLSL; ResourcesHLSL *mResourcesHLSL;
......
...@@ -2640,6 +2640,37 @@ void main() ...@@ -2640,6 +2640,37 @@ void main()
EXPECT_EQ(100u, outputValue); EXPECT_EQ(100u, outputValue);
} }
// Test that the length of a struct buffer variable is supported.
TEST_P(ComputeShaderTest, ShaderStorageBlocksStructLength)
{
const char kCSSource[] = R"(#version 310 es
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
struct Particle
{
int len;
};
layout(binding = 0, std430) readonly buffer Buf1
{
Particle particlesRead[];
};
layout(binding = 1, std430) buffer Buf2
{
Particle particlesWrite[];
};
void main()
{
int index = int(gl_GlobalInvocationID.x);
particlesWrite[index].len = particlesRead.length();
})";
ANGLE_GL_COMPUTE_PROGRAM(program, kCSSource);
EXPECT_GL_NO_ERROR();
}
// Test that scalar buffer variables are supported. // Test that scalar buffer variables are supported.
TEST_P(ComputeShaderTest, ShaderStorageBlocksScalar) TEST_P(ComputeShaderTest, ShaderStorageBlocksScalar)
{ {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment