Always consider type arrayness for atomic counters
Atomic counter arrays may be declared with various different syntax -
the array size may be declared as a part of the type or as a part of
the declarator. Take this into account when determining whether atomic
counter offsets overlap.
BUG=angleproject:1729
TEST=angle_unittests
Change-Id: I7435ded9401c4c1caab22c22d83fd2ad301df768
Reviewed-on: https://chromium-review.googlesource.com/738140
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 13f9354..3f0ce5a 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -2248,30 +2248,35 @@
// Make sure there is no offset overlapping, and store the newly assigned offset to "type" in
// intermediate tree.
-void TParseContext::checkAtomicCounterOffsetIsNotOverlapped(TPublicType &publicType,
- size_t size,
- bool forceAppend,
- const TSourceLoc &loc,
- TType &type)
+void TParseContext::checkAtomicCounterOffsetDoesNotOverlap(bool forceAppend,
+ const TSourceLoc &loc,
+ TType *type)
{
- auto &bindingState = mAtomicCounterBindingStates[publicType.layoutQualifier.binding];
+ if (!IsAtomicCounter(type->getBasicType()))
+ {
+ return;
+ }
+
+ const size_t size = type->isArray() ? kAtomicCounterArrayStride * type->getArraySizeProduct()
+ : kAtomicCounterSize;
+ TLayoutQualifier layoutQualifier = type->getLayoutQualifier();
+ auto &bindingState = mAtomicCounterBindingStates[layoutQualifier.binding];
int offset;
- if (publicType.layoutQualifier.offset == -1 || forceAppend)
+ if (layoutQualifier.offset == -1 || forceAppend)
{
offset = bindingState.appendSpan(size);
}
else
{
- offset = bindingState.insertSpan(publicType.layoutQualifier.offset, size);
+ offset = bindingState.insertSpan(layoutQualifier.offset, size);
}
if (offset == -1)
{
error(loc, "Offset overlapping", "atomic counter");
return;
}
- TLayoutQualifier qualifier = type.getLayoutQualifier();
- qualifier.offset = offset;
- type.setLayoutQualifier(qualifier);
+ layoutQualifier.offset = offset;
+ type->setLayoutQualifier(layoutQualifier);
}
void TParseContext::checkGeometryShaderInputAndSetArraySize(const TSourceLoc &location,
@@ -2373,12 +2378,7 @@
checkCanBeDeclaredWithoutInitializer(identifierOrTypeLocation, identifier, &type);
- if (IsAtomicCounter(publicType.getBasicType()))
- {
-
- checkAtomicCounterOffsetIsNotOverlapped(publicType, kAtomicCounterSize, false,
- identifierOrTypeLocation, type);
- }
+ checkAtomicCounterOffsetDoesNotOverlap(false, identifierOrTypeLocation, &type);
TVariable *variable = nullptr;
declareVariable(identifierOrTypeLocation, identifier, type, &variable);
@@ -2421,12 +2421,7 @@
checkCanBeDeclaredWithoutInitializer(identifierLocation, identifier, &arrayType);
- if (IsAtomicCounter(elementType.getBasicType()))
- {
- checkAtomicCounterOffsetIsNotOverlapped(
- elementType, kAtomicCounterArrayStride * arrayType.getArraySizeProduct(), false,
- identifierLocation, arrayType);
- }
+ checkAtomicCounterOffsetDoesNotOverlap(false, identifierLocation, &arrayType);
TVariable *variable = nullptr;
declareVariable(identifierLocation, identifier, arrayType, &variable);
@@ -2587,11 +2582,8 @@
checkCanBeDeclaredWithoutInitializer(identifierLocation, identifier, &type);
- if (IsAtomicCounter(publicType.getBasicType()))
- {
- checkAtomicCounterOffsetIsNotOverlapped(publicType, kAtomicCounterSize, true,
- identifierLocation, type);
- }
+ checkAtomicCounterOffsetDoesNotOverlap(true, identifierLocation, &type);
+
declareVariable(identifierLocation, identifier, type, &variable);
if (variable)
@@ -2628,12 +2620,7 @@
checkCanBeDeclaredWithoutInitializer(identifierLocation, identifier, &arrayType);
- if (IsAtomicCounter(elementType.getBasicType()))
- {
- checkAtomicCounterOffsetIsNotOverlapped(
- elementType, kAtomicCounterArrayStride * arrayType.getArraySizeProduct(), true,
- identifierLocation, arrayType);
- }
+ checkAtomicCounterOffsetDoesNotOverlap(true, identifierLocation, &arrayType);
TVariable *variable = nullptr;
declareVariable(identifierLocation, identifier, arrayType, &variable);
diff --git a/src/compiler/translator/ParseContext.h b/src/compiler/translator/ParseContext.h
index 27eb0cc..f423b83 100644
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -441,6 +441,7 @@
// we treat it as always 4 in favour of the original interpretation in
// "ARB_shader_atomic_counters".
// TODO(jie.a.chen@intel.com): Double check this once the spec vagueness is resolved.
+ // Note that there may be tests in AtomicCounter_test that will need to be updated as well.
constexpr static size_t kAtomicCounterArrayStride = 4;
// Returns a clamped index. If it prints out an error message, the token is "[]".
@@ -480,11 +481,9 @@
TLayoutImageInternalFormat internalFormat);
void checkMemoryQualifierIsNotSpecified(const TMemoryQualifier &memoryQualifier,
const TSourceLoc &location);
- void checkAtomicCounterOffsetIsNotOverlapped(TPublicType &publicType,
- size_t size,
- bool forceAppend,
- const TSourceLoc &loc,
- TType &type);
+ void checkAtomicCounterOffsetDoesNotOverlap(bool forceAppend,
+ const TSourceLoc &loc,
+ TType *type);
void checkBindingIsValid(const TSourceLoc &identifierLocation, const TType &type);
void checkBindingIsNotSpecified(const TSourceLoc &location, int binding);
void checkOffsetIsNotSpecified(const TSourceLoc &location, int offset);