Fixed NxM matrix construction and minor issues Fixed NxM matrix construction by properly checking for these types in TIntermOperator::isConstructor. Also fixed a few areas of the code where the secondary size wasn't properly taken into account. Change-Id: I646a41e37460255316f5712f1d744c3a06d8a64d Reviewed-on: https://swiftshader-review.googlesource.com/3195 Tested-by: Alexis Hétu <sugoi@google.com> Reviewed-by: Nicolas Capens <capn@google.com>
diff --git a/src/OpenGL/compiler/Intermediate.cpp b/src/OpenGL/compiler/Intermediate.cpp index fad11c6..b1d2526 100644 --- a/src/OpenGL/compiler/Intermediate.cpp +++ b/src/OpenGL/compiler/Intermediate.cpp
@@ -21,6 +21,38 @@ return left > right ? left : right; } +static bool ValidateMultiplication(TOperator op, const TType &left, const TType &right) +{ + switch(op) + { + case EOpMul: + case EOpMulAssign: + return left.getNominalSize() == right.getNominalSize() && + left.getSecondarySize() == right.getSecondarySize(); + case EOpVectorTimesScalar: + case EOpVectorTimesScalarAssign: + return true; + case EOpVectorTimesMatrix: + return left.getNominalSize() == right.getSecondarySize(); + case EOpVectorTimesMatrixAssign: + return left.getNominalSize() == right.getSecondarySize() && + left.getNominalSize() == right.getNominalSize(); + case EOpMatrixTimesVector: + return left.getNominalSize() == right.getNominalSize(); + case EOpMatrixTimesScalar: + case EOpMatrixTimesScalarAssign: + return true; + case EOpMatrixTimesMatrix: + return left.getNominalSize() == right.getSecondarySize(); + case EOpMatrixTimesMatrixAssign: + return left.getNominalSize() == right.getNominalSize() && + left.getSecondarySize() == right.getSecondarySize(); + default: + UNREACHABLE(); + return false; + } +} + const char* getOperatorString(TOperator op) { switch (op) { case EOpInitialize: return "="; @@ -645,7 +677,13 @@ case EOpConstructVec3: case EOpConstructVec4: case EOpConstructMat2: + case EOpConstructMat2x3: + case EOpConstructMat2x4: + case EOpConstructMat3x2: case EOpConstructMat3: + case EOpConstructMat3x4: + case EOpConstructMat4x2: + case EOpConstructMat4x3: case EOpConstructMat4: case EOpConstructFloat: case EOpConstructIVec2: @@ -752,7 +790,6 @@ } int primarySize = std::max(left->getNominalSize(), right->getNominalSize()); - int secondarySize = std::max(left->getSecondarySize(), right->getSecondarySize()); // // All scalars. Code after this test assumes this case is removed! @@ -791,25 +828,6 @@ // If we reach here, at least one of the operands is vector or matrix. // The other operand could be a scalar, vector, or matrix. - // Are the sizes compatible? - // - if (left->getNominalSize() != right->getNominalSize()) { - // If the nominal size of operands do not match: - // One of them must be scalar. - if (left->getNominalSize() != 1 && right->getNominalSize() != 1) - return false; - // Operator cannot be of type pure assignment. - if (op == EOpAssign || op == EOpInitialize) - return false; - } - - if (left->getSecondarySize() != right->getSecondarySize()) { - // Operator cannot be of type pure assignment. - if (op == EOpAssign || op == EOpInitialize) - return false; - } - - // // Can these two operands be combined? // TBasicType basicType = left->getBasicType(); @@ -817,31 +835,45 @@ case EOpMul: if (!left->isMatrix() && right->isMatrix()) { if (left->isVector()) + { op = EOpVectorTimesMatrix; + setType(TType(basicType, higherPrecision, EvqTemporary, + static_cast<unsigned char>(right->getNominalSize()), 1)); + } else { op = EOpMatrixTimesScalar; - setType(TType(basicType, higherPrecision, EvqTemporary, primarySize, secondarySize)); + setType(TType(basicType, higherPrecision, EvqTemporary, + static_cast<unsigned char>(right->getNominalSize()), static_cast<unsigned char>(right->getSecondarySize()))); } } else if (left->isMatrix() && !right->isMatrix()) { if (right->isVector()) { op = EOpMatrixTimesVector; - setType(TType(basicType, higherPrecision, EvqTemporary, primarySize, 1)); + setType(TType(basicType, higherPrecision, EvqTemporary, + static_cast<unsigned char>(left->getSecondarySize()), 1)); } else { op = EOpMatrixTimesScalar; } } else if (left->isMatrix() && right->isMatrix()) { op = EOpMatrixTimesMatrix; + setType(TType(basicType, higherPrecision, EvqTemporary, + static_cast<unsigned char>(right->getNominalSize()), static_cast<unsigned char>(left->getSecondarySize()))); } else if (!left->isMatrix() && !right->isMatrix()) { if (left->isVector() && right->isVector()) { // leave as component product } else if (left->isVector() || right->isVector()) { op = EOpVectorTimesScalar; - setType(TType(basicType, higherPrecision, EvqTemporary, primarySize, 1)); + setType(TType(basicType, higherPrecision, EvqTemporary, + static_cast<unsigned char>(primarySize), 1)); } } else { infoSink.info.message(EPrefixInternalError, "Missing elses", getLine()); return false; } + + if(!ValidateMultiplication(op, left->getType(), right->getType())) + { + return false; + } break; case EOpMulAssign: if (!left->isMatrix() && right->isMatrix()) { @@ -858,6 +890,8 @@ } } else if (left->isMatrix() && right->isMatrix()) { op = EOpMatrixTimesMatrixAssign; + setType(TType(basicType, higherPrecision, EvqTemporary, + static_cast<unsigned char>(right->getNominalSize()), static_cast<unsigned char>(left->getSecondarySize()))); } else if (!left->isMatrix() && !right->isMatrix()) { if (left->isVector() && right->isVector()) { // leave as component product @@ -865,16 +899,27 @@ if (! left->isVector()) return false; op = EOpVectorTimesScalarAssign; - setType(TType(basicType, higherPrecision, EvqTemporary, primarySize, 1)); + setType(TType(basicType, higherPrecision, EvqTemporary, + static_cast<unsigned char>(left->getNominalSize()), 1)); } } else { infoSink.info.message(EPrefixInternalError, "Missing elses", getLine()); return false; } + + if(!ValidateMultiplication(op, left->getType(), right->getType())) + { + return false; + } break; case EOpAssign: case EOpInitialize: + // No more additional checks are needed. + if ((left->getNominalSize() != right->getNominalSize()) || + (left->getSecondarySize() != right->getSecondarySize())) + return false; + break; case EOpAdd: case EOpSub: case EOpDiv: @@ -915,6 +960,8 @@ } { + const int secondarySize = std::max( + left->getSecondarySize(), right->getSecondarySize()); setType(TType(basicType, higherPrecision, EvqTemporary, static_cast<unsigned char>(primarySize), static_cast<unsigned char>(secondarySize))); if(left->isArray()) @@ -923,8 +970,6 @@ type.setArraySize(left->getArraySize()); } } - - setType(TType(basicType, higherPrecision, EvqTemporary, primarySize, secondarySize)); break; case EOpEqual: @@ -933,8 +978,8 @@ case EOpGreaterThan: case EOpLessThanEqual: case EOpGreaterThanEqual: - if ((left->isMatrix() && right->isVector()) || - (left->isVector() && right->isMatrix())) + if ((left->getNominalSize() != right->getNominalSize()) || + (left->getSecondarySize() != right->getSecondarySize())) return false; setType(TType(EbtBool, EbpUndefined)); break; @@ -1056,16 +1101,29 @@ return 0; } {// support MSVC++6.0 - int size = getNominalSize(); - tempConstArray = new ConstantUnion[size*size]; - for (int row = 0; row < size; row++) { - for (int column = 0; column < size; column++) { - tempConstArray[size * column + row].setFConst(0.0f); - for (int i = 0; i < size; i++) { - tempConstArray[size * column + row].setFConst(tempConstArray[size * column + row].getFConst() + unionArray[i * size + row].getFConst() * (rightUnionArray[column * size + i].getFConst())); + int leftNumCols = getNominalSize(); + int leftNumRows = getSecondarySize(); + int rightNumCols = node->getNominalSize(); + int rightNumRows = node->getSecondarySize(); + if(leftNumCols != rightNumRows) { + infoSink.info.message(EPrefixInternalError, "Constant Folding cannot be done for matrix multiply", getLine()); + return 0; + } + int tempNumCols = rightNumCols; + int tempNumRows = leftNumRows; + int tempNumAdds = leftNumCols; + tempConstArray = new ConstantUnion[tempNumCols*tempNumRows]; + for (int row = 0; row < tempNumRows; row++) { + for (int column = 0; column < tempNumCols; column++) { + tempConstArray[tempNumRows * column + row].setFConst(0.0f); + for (int i = 0; i < tempNumAdds; i++) { + tempConstArray[tempNumRows * column + row].setFConst(tempConstArray[tempNumRows * column + row].getFConst() + unionArray[i * leftNumRows + row].getFConst() * (rightUnionArray[column * rightNumRows + i].getFConst())); } } } + // update return type for matrix product + returnType.setNominalSize(static_cast<unsigned char>(tempNumCols)); + returnType.setSecondarySize(static_cast<unsigned char>(tempNumRows)); } break; case EOpDiv:
diff --git a/src/OpenGL/compiler/OutputASM.cpp b/src/OpenGL/compiler/OutputASM.cpp index d95d568..2af8826 100644 --- a/src/OpenGL/compiler/OutputASM.cpp +++ b/src/OpenGL/compiler/OutputASM.cpp
@@ -1430,7 +1430,7 @@ } else if(type.isMatrix()) { - return registers * type.getNominalSize(); + return registers * type.getSecondarySize(); } UNREACHABLE(); @@ -1446,7 +1446,7 @@ return registerSize(*type.getStruct()->begin()->type, 0); } - return type.getNominalSize(); + return type.isMatrix() ? type.getSecondarySize() : type.getNominalSize(); } if(type.isArray() && registers >= type.elementRegisterCount()) @@ -1590,7 +1590,7 @@ { if(src && ((src->isVector() && (!dst->isVector() || (dst->getNominalSize() != dst->getNominalSize()))) || - (src->isMatrix() && (!dst->isMatrix() || (src->getNominalSize() != dst->getNominalSize()))))) + (src->isMatrix() && (!dst->isMatrix() || (src->getNominalSize() != dst->getNominalSize()) || (src->getSecondarySize() != dst->getSecondarySize()))))) { return mContext.error(src->getLine(), "Result type should match the l-value type in compound assignment", src->isVector() ? "vector" : "matrix"); }
diff --git a/src/OpenGL/compiler/ParseHelper.cpp b/src/OpenGL/compiler/ParseHelper.cpp index f1287cc..2d9b3d5 100644 --- a/src/OpenGL/compiler/ParseHelper.cpp +++ b/src/OpenGL/compiler/ParseHelper.cpp
@@ -114,7 +114,7 @@ // Look at a '.' field selector string and change it into offsets // for a matrix. // -bool TParseContext::parseMatrixFields(const TString& compString, int matSize, TMatrixFields& fields, int line) +bool TParseContext::parseMatrixFields(const TString& compString, int matCols, int matRows, TMatrixFields& fields, int line) { fields.wholeRow = false; fields.wholeCol = false; @@ -150,7 +150,7 @@ fields.col = compString[1] - '0'; } - if (fields.row >= matSize || fields.col >= matSize) { + if (fields.row >= matRows || fields.col >= matCols) { error(line, "matrix field selection out of range", compString.c_str()); return false; }
diff --git a/src/OpenGL/compiler/ParseHelper.h b/src/OpenGL/compiler/ParseHelper.h index 4f9fa96..c494572 100644 --- a/src/OpenGL/compiler/ParseHelper.h +++ b/src/OpenGL/compiler/ParseHelper.h
@@ -76,7 +76,7 @@ void recover(); bool parseVectorFields(const TString&, int vecSize, TVectorFields&, int line); - bool parseMatrixFields(const TString&, int matSize, TMatrixFields&, int line); + bool parseMatrixFields(const TString&, int matCols, int matRows, TMatrixFields&, int line); bool reservedErrorCheck(int line, const TString& identifier); void assignError(int line, const char* op, TString left, TString right);
diff --git a/src/OpenGL/compiler/glslang.y b/src/OpenGL/compiler/glslang.y index 0dff1c9..ce04736 100644 --- a/src/OpenGL/compiler/glslang.y +++ b/src/OpenGL/compiler/glslang.y
@@ -323,9 +323,9 @@ if ($1->getType().getQualifier() == EvqConstExpr) $$->getTypePointer()->setQualifier(EvqConstExpr); } else if ($1->isMatrix() && $1->getType().getQualifier() == EvqConstExpr) - $$->setType(TType($1->getBasicType(), $1->getPrecision(), EvqConstExpr, $1->getNominalSize())); + $$->setType(TType($1->getBasicType(), $1->getPrecision(), EvqConstExpr, $1->getSecondarySize())); else if ($1->isMatrix()) - $$->setType(TType($1->getBasicType(), $1->getPrecision(), EvqTemporary, $1->getNominalSize())); + $$->setType(TType($1->getBasicType(), $1->getPrecision(), EvqTemporary, $1->getSecondarySize())); else if ($1->isVector() && $1->getType().getQualifier() == EvqConstExpr) $$->setType(TType($1->getBasicType(), $1->getPrecision(), EvqConstExpr)); else if ($1->isVector()) @@ -366,7 +366,7 @@ } } else if ($1->isMatrix()) { TMatrixFields fields; - if (! context->parseMatrixFields(*$3.string, $1->getNominalSize(), fields, $3.line)) { + if (! context->parseMatrixFields(*$3.string, $1->getNominalSize(), $1->getSecondarySize(), fields, $3.line)) { fields.wholeRow = false; fields.wholeCol = false; fields.row = 0;
diff --git a/src/OpenGL/compiler/glslang_tab.cpp b/src/OpenGL/compiler/glslang_tab.cpp index a672362..0a5f2a5 100644 --- a/src/OpenGL/compiler/glslang_tab.cpp +++ b/src/OpenGL/compiler/glslang_tab.cpp
@@ -2465,9 +2465,9 @@ if ((yyvsp[(1) - (4)].interm.intermTypedNode)->getType().getQualifier() == EvqConstExpr) (yyval.interm.intermTypedNode)->getTypePointer()->setQualifier(EvqConstExpr); } else if ((yyvsp[(1) - (4)].interm.intermTypedNode)->isMatrix() && (yyvsp[(1) - (4)].interm.intermTypedNode)->getType().getQualifier() == EvqConstExpr) - (yyval.interm.intermTypedNode)->setType(TType((yyvsp[(1) - (4)].interm.intermTypedNode)->getBasicType(), (yyvsp[(1) - (4)].interm.intermTypedNode)->getPrecision(), EvqConstExpr, (yyvsp[(1) - (4)].interm.intermTypedNode)->getNominalSize())); + (yyval.interm.intermTypedNode)->setType(TType((yyvsp[(1) - (4)].interm.intermTypedNode)->getBasicType(), (yyvsp[(1) - (4)].interm.intermTypedNode)->getPrecision(), EvqConstExpr, (yyvsp[(1) - (4)].interm.intermTypedNode)->getSecondarySize())); else if ((yyvsp[(1) - (4)].interm.intermTypedNode)->isMatrix()) - (yyval.interm.intermTypedNode)->setType(TType((yyvsp[(1) - (4)].interm.intermTypedNode)->getBasicType(), (yyvsp[(1) - (4)].interm.intermTypedNode)->getPrecision(), EvqTemporary, (yyvsp[(1) - (4)].interm.intermTypedNode)->getNominalSize())); + (yyval.interm.intermTypedNode)->setType(TType((yyvsp[(1) - (4)].interm.intermTypedNode)->getBasicType(), (yyvsp[(1) - (4)].interm.intermTypedNode)->getPrecision(), EvqTemporary, (yyvsp[(1) - (4)].interm.intermTypedNode)->getSecondarySize())); else if ((yyvsp[(1) - (4)].interm.intermTypedNode)->isVector() && (yyvsp[(1) - (4)].interm.intermTypedNode)->getType().getQualifier() == EvqConstExpr) (yyval.interm.intermTypedNode)->setType(TType((yyvsp[(1) - (4)].interm.intermTypedNode)->getBasicType(), (yyvsp[(1) - (4)].interm.intermTypedNode)->getPrecision(), EvqConstExpr)); else if ((yyvsp[(1) - (4)].interm.intermTypedNode)->isVector()) @@ -2516,7 +2516,7 @@ } } else if ((yyvsp[(1) - (3)].interm.intermTypedNode)->isMatrix()) { TMatrixFields fields; - if (! context->parseMatrixFields(*(yyvsp[(3) - (3)].lex).string, (yyvsp[(1) - (3)].interm.intermTypedNode)->getNominalSize(), fields, (yyvsp[(3) - (3)].lex).line)) { + if (! context->parseMatrixFields(*(yyvsp[(3) - (3)].lex).string, (yyvsp[(1) - (3)].interm.intermTypedNode)->getNominalSize(), (yyvsp[(1) - (3)].interm.intermTypedNode)->getSecondarySize(), fields, (yyvsp[(3) - (3)].lex).line)) { fields.wholeRow = false; fields.wholeCol = false; fields.row = 0;