Imported a few ES3 fixes from Angle
Imported some of the more trivial bug fixes:
- Added a few missing interface block cases
- Added checks for varying structs
- Added checks for unsized arrays
- Added first-class array check for pre ES3 compilation
- Added check that ES3 functions do not use builtin names (ES3 only)
- Added more binary operator checks
Change-Id: I3d75453f17e1123478ef7da0998e869970a7fb7d
Reviewed-on: https://swiftshader-review.googlesource.com/8289
Tested-by: Alexis Hétu <sugoi@google.com>
Reviewed-by: Nicolas Capens <capn@google.com>
diff --git a/src/OpenGL/compiler/ParseHelper.cpp b/src/OpenGL/compiler/ParseHelper.cpp
index 7bd7e64..83f58ce 100644
--- a/src/OpenGL/compiler/ParseHelper.cpp
+++ b/src/OpenGL/compiler/ParseHelper.cpp
@@ -28,6 +28,77 @@
//
////////////////////////////////////////////////////////////////////////
+namespace
+{
+ bool IsVaryingOut(TQualifier qualifier)
+ {
+ switch(qualifier)
+ {
+ case EvqVaryingOut:
+ case EvqSmoothOut:
+ case EvqFlatOut:
+ case EvqCentroidOut:
+ case EvqVertexOut:
+ return true;
+
+ default: break;
+ }
+
+ return false;
+ }
+
+ bool IsVaryingIn(TQualifier qualifier)
+ {
+ switch(qualifier)
+ {
+ case EvqVaryingIn:
+ case EvqSmoothIn:
+ case EvqFlatIn:
+ case EvqCentroidIn:
+ case EvqFragmentIn:
+ return true;
+
+ default: break;
+ }
+
+ return false;
+ }
+
+ bool IsVarying(TQualifier qualifier)
+ {
+ return IsVaryingIn(qualifier) || IsVaryingOut(qualifier);
+ }
+
+ bool IsAssignment(TOperator op)
+ {
+ switch(op)
+ {
+ case EOpPostIncrement:
+ case EOpPostDecrement:
+ case EOpPreIncrement:
+ case EOpPreDecrement:
+ case EOpAssign:
+ case EOpAddAssign:
+ case EOpSubAssign:
+ case EOpMulAssign:
+ case EOpVectorTimesMatrixAssign:
+ case EOpVectorTimesScalarAssign:
+ case EOpMatrixTimesScalarAssign:
+ case EOpMatrixTimesMatrixAssign:
+ case EOpDivAssign:
+ case EOpIModAssign:
+ case EOpBitShiftLeftAssign:
+ case EOpBitShiftRightAssign:
+ case EOpBitwiseAndAssign:
+ case EOpBitwiseXorAssign:
+ case EOpBitwiseOrAssign:
+ return true;
+ default:
+ return false;
+ }
+ }
+}
+
//
// Look at a '.' field selector string and change it into offsets
// for a vector.
@@ -282,6 +353,7 @@
case EOpIndexDirect:
case EOpIndexIndirect:
case EOpIndexDirectStruct:
+ case EOpIndexDirectInterfaceBlock:
return lValueErrorCheck(line, op, binaryNode->getLeft());
case EOpVectorSwizzle:
errorReturn = lValueErrorCheck(line, op, binaryNode->getLeft());
@@ -756,7 +828,7 @@
if (IsSampler(type.getBasicType()))
return true;
- if (type.getBasicType() == EbtStruct) {
+ if (type.getBasicType() == EbtStruct || type.isInterfaceBlock()) {
const TFieldList& fields = type.getStruct()->fields();
for(unsigned int i = 0; i < fields.size(); ++i) {
if (containsSampler(*fields[i]->type()))
@@ -776,7 +848,7 @@
{
TIntermConstantUnion* constant = expr->getAsConstantUnion();
- if (constant == 0 || !constant->isScalarInt())
+ if (expr->getQualifier() != EvqConstExpr || constant == 0 || !constant->isScalarInt())
{
error(line, "array size must be a constant integer expression", "");
return true;
@@ -798,14 +870,20 @@
{
size = constant->getIConst(0);
- if (size <= 0)
+ if (size < 0)
{
- error(line, "array size must be a positive integer", "");
+ error(line, "array size must be non-negative", "");
size = 1;
return true;
}
}
+ if(size == 0)
+ {
+ error(line, "array size must be greater than zero", "");
+ return true;
+ }
+
return false;
}
@@ -839,6 +917,15 @@
return true;
}
+ // In ESSL1.00 shaders, structs cannot be varying (section 4.3.5). This is checked elsewhere.
+ // In ESSL3.00 shaders, struct inputs/outputs are allowed but not arrays of structs (section 4.3.4).
+ if(mShaderVersion >= 300 && type.type == EbtStruct && IsVarying(type.qualifier))
+ {
+ error(line, "cannot declare arrays of structs of this qualifier",
+ TType(type).getCompleteString().c_str());
+ return true;
+ }
+
return false;
}
@@ -1191,9 +1278,19 @@
ASSERT(intermNode != nullptr);
TType type = TType(pType);
- if(type.isArray() && (type.getArraySize() == 0))
+ if(type.isUnsizedArray())
{
- type.setArraySize(initializer->getArraySize());
+ // We have not checked yet whether the initializer actually is an array or not.
+ if(initializer->isArray())
+ {
+ type.setArraySize(initializer->getArraySize());
+ }
+ else
+ {
+ // Having a non-array initializer for an unsized array will result in an error later,
+ // so we don't generate an error message here.
+ type.setArraySize(1u);
+ }
}
TVariable *variable = nullptr;
@@ -1285,6 +1382,12 @@
if(mShaderVersion < 300)
{
+ if(typeSpecifier.array)
+ {
+ error(typeSpecifier.line, "not supported", "first-class array");
+ returnType.clearArrayness();
+ }
+
if(qualifier == EvqAttribute && (typeSpecifier.type == EbtBool || typeSpecifier.type == EbtInt))
{
error(typeSpecifier.line, "cannot be bool or int", getQualifierString(qualifier));
@@ -1300,44 +1403,87 @@
}
else
{
- switch(qualifier)
+ if(!returnType.layoutQualifier.isEmpty())
{
- case EvqSmoothIn:
- case EvqSmoothOut:
- case EvqVertexOut:
- case EvqFragmentIn:
- case EvqCentroidOut:
- case EvqCentroidIn:
- if(typeSpecifier.type == EbtBool)
- {
- error(typeSpecifier.line, "cannot be bool", getQualifierString(qualifier));
- recover();
- }
- if(typeSpecifier.type == EbtInt || typeSpecifier.type == EbtUInt)
- {
- error(typeSpecifier.line, "must use 'flat' interpolation here", getQualifierString(qualifier));
- recover();
- }
- break;
+ globalErrorCheck(typeSpecifier.line, symbolTable.atGlobalLevel(), "layout");
+ }
- case EvqVertexIn:
- case EvqFragmentOut:
- case EvqFlatIn:
- case EvqFlatOut:
- if(typeSpecifier.type == EbtBool)
- {
- error(typeSpecifier.line, "cannot be bool", getQualifierString(qualifier));
- recover();
- }
- break;
-
- default: break;
+ if(IsVarying(returnType.qualifier) || returnType.qualifier == EvqVertexIn || returnType.qualifier == EvqFragmentOut)
+ {
+ checkInputOutputTypeIsValidES3(returnType.qualifier, typeSpecifier, typeSpecifier.line);
}
}
return returnType;
}
+void TParseContext::checkInputOutputTypeIsValidES3(const TQualifier qualifier,
+ const TPublicType &type,
+ const TSourceLoc &qualifierLocation)
+{
+ // An input/output variable can never be bool or a sampler. Samplers are checked elsewhere.
+ if(type.type == EbtBool)
+ {
+ error(qualifierLocation, "cannot be bool", getQualifierString(qualifier));
+ }
+
+ // Specific restrictions apply for vertex shader inputs and fragment shader outputs.
+ switch(qualifier)
+ {
+ case EvqVertexIn:
+ // ESSL 3.00 section 4.3.4
+ if(type.array)
+ {
+ error(qualifierLocation, "cannot be array", getQualifierString(qualifier));
+ }
+ // Vertex inputs with a struct type are disallowed in singleDeclarationErrorCheck
+ return;
+ case EvqFragmentOut:
+ // ESSL 3.00 section 4.3.6
+ if(type.isMatrix())
+ {
+ error(qualifierLocation, "cannot be matrix", getQualifierString(qualifier));
+ }
+ // Fragment outputs with a struct type are disallowed in singleDeclarationErrorCheck
+ return;
+ default:
+ break;
+ }
+
+ // Vertex shader outputs / fragment shader inputs have a different, slightly more lenient set of
+ // restrictions.
+ bool typeContainsIntegers = (type.type == EbtInt || type.type == EbtUInt ||
+ type.isStructureContainingType(EbtInt) ||
+ type.isStructureContainingType(EbtUInt));
+ if(typeContainsIntegers && qualifier != EvqFlatIn && qualifier != EvqFlatOut)
+ {
+ error(qualifierLocation, "must use 'flat' interpolation here", getQualifierString(qualifier));
+ }
+
+ if(type.type == EbtStruct)
+ {
+ // ESSL 3.00 sections 4.3.4 and 4.3.6.
+ // These restrictions are only implied by the ESSL 3.00 spec, but
+ // the ESSL 3.10 spec lists these restrictions explicitly.
+ if(type.array)
+ {
+ error(qualifierLocation, "cannot be an array of structures", getQualifierString(qualifier));
+ }
+ if(type.isStructureContainingArrays())
+ {
+ error(qualifierLocation, "cannot be a structure containing an array", getQualifierString(qualifier));
+ }
+ if(type.isStructureContainingType(EbtStruct))
+ {
+ error(qualifierLocation, "cannot be a structure containing a structure", getQualifierString(qualifier));
+ }
+ if(type.isStructureContainingType(EbtBool))
+ {
+ error(qualifierLocation, "cannot be a structure containing a bool", getQualifierString(qualifier));
+ }
+ }
+}
+
TIntermAggregate *TParseContext::parseSingleDeclaration(TPublicType &publicType,
const TSourceLoc &identifierOrTypeLocation,
const TString &identifier)
@@ -1915,7 +2061,13 @@
// here.
//
TFunction *prevDec = static_cast<TFunction *>(symbolTable.find(function->getMangledName(), getShaderVersion()));
- if(prevDec)
+ if(getShaderVersion() >= 300 && symbolTable.hasUnmangledBuiltIn(function->getName().c_str()))
+ {
+ // With ESSL 3.00, names of built-in functions cannot be redeclared as functions.
+ // Therefore overloading or redefining builtin functions is an error.
+ error(location, "Name of a built-in function cannot be redeclared as function", function->getName().c_str());
+ }
+ else if(prevDec)
{
if(prevDec->getReturnType() != function->getReturnType())
{
@@ -1969,84 +2121,7 @@
}
else
{
- switch(publicType.type)
- {
- case EbtFloat:
- if(publicType.isMatrix())
- {
- switch(publicType.getCols())
- {
- case 2:
- switch(publicType.getRows())
- {
- case 2: op = EOpConstructMat2; break;
- case 3: op = EOpConstructMat2x3; break;
- case 4: op = EOpConstructMat2x4; break;
- }
- break;
- case 3:
- switch(publicType.getRows())
- {
- case 2: op = EOpConstructMat3x2; break;
- case 3: op = EOpConstructMat3; break;
- case 4: op = EOpConstructMat3x4; break;
- }
- break;
- case 4:
- switch(publicType.getRows())
- {
- case 2: op = EOpConstructMat4x2; break;
- case 3: op = EOpConstructMat4x3; break;
- case 4: op = EOpConstructMat4; break;
- }
- break;
- }
- }
- else
- {
- switch(publicType.getNominalSize())
- {
- case 1: op = EOpConstructFloat; break;
- case 2: op = EOpConstructVec2; break;
- case 3: op = EOpConstructVec3; break;
- case 4: op = EOpConstructVec4; break;
- }
- }
- break;
-
- case EbtInt:
- switch(publicType.getNominalSize())
- {
- case 1: op = EOpConstructInt; break;
- case 2: op = EOpConstructIVec2; break;
- case 3: op = EOpConstructIVec3; break;
- case 4: op = EOpConstructIVec4; break;
- }
- break;
-
- case EbtUInt:
- switch(publicType.getNominalSize())
- {
- case 1: op = EOpConstructUInt; break;
- case 2: op = EOpConstructUVec2; break;
- case 3: op = EOpConstructUVec3; break;
- case 4: op = EOpConstructUVec4; break;
- }
- break;
-
- case EbtBool:
- switch(publicType.getNominalSize())
- {
- case 1: op = EOpConstructBool; break;
- case 2: op = EOpConstructBVec2; break;
- case 3: op = EOpConstructBVec3; break;
- case 4: op = EOpConstructBVec4; break;
- }
- break;
-
- default: break;
- }
-
+ op = TypeToConstructorOperator(TType(publicType));
if(op == EOpNull)
{
error(publicType.line, "cannot construct this type", getBasicString(publicType.type));
@@ -2717,9 +2792,8 @@
}
else
{
- ConstantUnion *unionArray = new ConstantUnion[1];
- unionArray->setIConst(i);
- TIntermTyped *index = intermediate.addConstantUnion(unionArray, *fields[i]->type(), fieldLocation);
+ TIntermTyped *index = TIntermTyped::CreateIndexNode(i);
+ index->setLine(fieldLocation);
indexedExpression = intermediate.addIndex(EOpIndexDirectStruct, baseExpression, index, dotLocation);
indexedExpression->setType(*fields[i]->type());
}
@@ -3237,6 +3311,46 @@
{
return false;
}
+ break;
+ case EOpAdd:
+ case EOpSub:
+ case EOpDiv:
+ case EOpIMod:
+ case EOpBitShiftLeft:
+ case EOpBitShiftRight:
+ case EOpBitwiseAnd:
+ case EOpBitwiseXor:
+ case EOpBitwiseOr:
+ case EOpAddAssign:
+ case EOpSubAssign:
+ case EOpDivAssign:
+ case EOpIModAssign:
+ case EOpBitShiftLeftAssign:
+ case EOpBitShiftRightAssign:
+ case EOpBitwiseAndAssign:
+ case EOpBitwiseXorAssign:
+ case EOpBitwiseOrAssign:
+ if((left->isMatrix() && right->isVector()) || (left->isVector() && right->isMatrix()))
+ {
+ return false;
+ }
+
+ // Are the sizes compatible?
+ if(left->getNominalSize() != right->getNominalSize() || left->getSecondarySize() != right->getSecondarySize())
+ {
+ // If the nominal sizes of operands do not match:
+ // One of them must be a scalar.
+ if(!left->isScalar() && !right->isScalar())
+ return false;
+
+ // In the case of compound assignment other than multiply-assign,
+ // the right side needs to be a scalar. Otherwise a vector/matrix
+ // would be assigned to a scalar. A scalar can't be shifted by a
+ // vector either.
+ if(!right->isScalar() && (IsAssignment(op) || op == EOpBitShiftLeft || op == EOpBitShiftRight))
+ return false;
+ }
+ break;
default:
break;
}