Fix ambiguous function call issues in HLSL output

src/compiler/translator/OutputHLSL.cpp

@@ -2221,12 +2221,12 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
                 return false;
             }
 
+            TIntermSequence *arguments = node->getSequence();
+
             TString name = DecorateFunctionIfNeeded(node->getNameObj());
-            out << TypeString(node->getType()) << " " << name
+            out << TypeString(node->getType()) << " " << name << DisambiguateFunctionName(arguments)
                 << (mOutputLod0Function ? "Lod0(" : "(");
 
-            TIntermSequence *arguments = node->getSequence();
-
             for (unsigned int i = 0; i < arguments->size(); i++)
             {
                 TIntermSymbol *symbol = (*arguments)[i]->getAsSymbolNode();
@@ -2271,6 +2271,9 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
 
             out << TypeString(node->getType()) << " ";
 
+            TIntermSequence *sequence  = node->getSequence();
+            TIntermSequence *arguments = (*sequence)[0]->getAsAggregate()->getSequence();
+
             if (name == "main")
             {
                 out << "gl_main(";
@@ -2278,12 +2281,9 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
             else
             {
                 out << DecorateFunctionIfNeeded(node->getNameObj())
-                    << (mOutputLod0Function ? "Lod0(" : "(");
+                    << DisambiguateFunctionName(arguments) << (mOutputLod0Function ? "Lod0(" : "(");
             }
 
-            TIntermSequence *sequence = node->getSequence();
-            TIntermSequence *arguments = (*sequence)[0]->getAsAggregate()->getSequence();
-
             for (unsigned int i = 0; i < arguments->size(); i++)
             {
                 TIntermSymbol *symbol = (*arguments)[i]->getAsSymbolNode();
@@ -2347,7 +2347,9 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
                 ASSERT(index != CallDAG::InvalidIndex);
                 lod0 &= mASTMetadataList[index].mNeedsLod0;
 
-                out << DecorateFunctionIfNeeded(node->getNameObj()) << (lod0 ? "Lod0(" : "(");
+                out << DecorateFunctionIfNeeded(node->getNameObj());
+                out << DisambiguateFunctionName(node->getSequence());
+                out << (lod0 ? "Lod0(" : "(");
             }
             else
             {
@@ -3285,9 +3287,9 @@ void OutputHLSL::outputConstructor(TInfoSinkBase &out,
 
     if (visit == PreVisit)
     {
-        mStructureHLSL->addConstructor(type, name, parameters);
+        TString constructorName = mStructureHLSL->addConstructor(type, name, parameters);
 
-        out << name << "(";
+        out << constructorName << "(";
     }
     else if (visit == InVisit)
     {

src/compiler/translator/StructureHLSL.cpp

@@ -211,16 +211,18 @@ TString StructureHLSL::define(const TStructure &structure, bool useHLSLRowMajorP
     return string;
 }
 
-void StructureHLSL::addConstructor(const TType &type, const TString &name, const TIntermSequence *parameters)
+TString StructureHLSL::addConstructor(const TType &type,
+                                      const TString &name,
+                                      const TIntermSequence *parameters)
 {
     if (name == "")
     {
-        return;   // Nameless structures don't have constructors
+        return TString();  // Nameless structures don't have constructors
     }
 
     if (type.getStruct() && mStructNames.find(name) != mStructNames.end())
     {
-        return;   // Already added
+        return TString(name);  // Already added
     }
 
     TType ctorType = type;
@@ -231,6 +233,8 @@ void StructureHLSL::addConstructor(const TType &type, const TString &name, const
     typedef std::vector<TType> ParameterArray;
     ParameterArray ctorParameters;
 
+    TString constructorFunctionName;
+
     const TStructure* structure = type.getStruct();
     if (structure)
     {
@@ -265,13 +269,16 @@ void StructureHLSL::addConstructor(const TType &type, const TString &name, const
         {
             ctorParameters.push_back(*fields[i]->type());
         }
+        constructorFunctionName = TString(name);
     }
     else if (parameters)
     {
-        for (TIntermSequence::const_iterator parameter = parameters->begin(); parameter != parameters->end(); parameter++)
+        for (auto parameter : *parameters)
         {
-            ctorParameters.push_back((*parameter)->getAsTyped()->getType());
+            const TType &paramType = parameter->getAsTyped()->getType();
+            ctorParameters.push_back(paramType);
         }
+        constructorFunctionName = TString(name) + DisambiguateFunctionName(parameters);
     }
     else UNREACHABLE();
 
@@ -283,7 +290,7 @@ void StructureHLSL::addConstructor(const TType &type, const TString &name, const
     }
     else   // Built-in type
     {
-        constructor += TypeString(ctorType) + " " + name + "(";
+        constructor += TypeString(ctorType) + " " + constructorFunctionName + "(";
     }
 
     for (unsigned int parameter = 0; parameter < ctorParameters.size(); parameter++)
@@ -465,6 +472,8 @@ void StructureHLSL::addConstructor(const TType &type, const TString &name, const
     }
 
     mConstructors.insert(constructor);
+
+    return constructorFunctionName;
 }
 
 std::string StructureHLSL::structsHeader() const

src/compiler/translator/StructureHLSL.h

@@ -49,7 +49,11 @@ class StructureHLSL : angle::NonCopyable
   public:
     StructureHLSL();
 
-    void addConstructor(const TType &type, const TString &name, const TIntermSequence *parameters);
+    // Returns the name of the constructor function. "name" parameter is the name of the type being
+    // constructed.
+    TString addConstructor(const TType &type,
+                           const TString &name,
+                           const TIntermSequence *parameters);
     std::string structsHeader() const;
 
     TString defineQualified(const TStructure &structure, bool useHLSLRowMajorPacking, bool useStd140Packing);

src/compiler/translator/UtilsHLSL.cpp

@@ -435,4 +435,23 @@ int HLSLTextureCoordsCount(const TBasicType samplerType)
     }
     return 0;
 }
+
+TString DisambiguateFunctionName(const TIntermSequence *parameters)
+{
+    TString disambiguatingString;
+    for (auto parameter : *parameters)
+    {
+        const TType &paramType = parameter->getAsTyped()->getType();
+        // Disambiguation is needed for float2x2 and float4 parameters. These are the only parameter
+        // types that HLSL thinks are identical. float2x3 and float3x2 are different types, for
+        // example. Other parameter types are not added to function names to avoid making function
+        // names longer.
+        if (paramType.getObjectSize() == 4 && paramType.getBasicType() == EbtFloat)
+        {
+            disambiguatingString += "_" + TypeString(paramType);
+        }
+    }
+    return disambiguatingString;
 }
+
+}  // namespace sh

src/compiler/translator/UtilsHLSL.h

@@ -11,6 +11,7 @@
 #define COMPILER_TRANSLATOR_UTILSHLSL_H_
 
 #include <vector>
+#include "compiler/translator/IntermNode.h"
 #include "compiler/translator/Types.h"
 
 #include "angle_gl.h"
@@ -73,6 +74,9 @@ TString QualifiedStructNameString(const TStructure &structure, bool useHLSLRowMa
 TString InterpolationString(TQualifier qualifier);
 TString QualifierString(TQualifier qualifier);
 int HLSLTextureCoordsCount(const TBasicType samplerType);
+// Parameters may need to be included in function names to disambiguate between overloaded
+// functions.
+TString DisambiguateFunctionName(const TIntermSequence *parameters);
 }
 
 #endif // COMPILER_TRANSLATOR_UTILSHLSL_H_

src/tests/deqp_support/deqp_gles2_test_expectations.txt

@@ -49,8 +49,6 @@
 
 // TODO(jmadill): triage these into temporary and permanent suppressions
 
-1030 WIN : dEQP-GLES2.functional.attribute_location.bind_hole.mat2 = FAIL
-1030 WIN : dEQP-GLES2.functional.attribute_location.bind_relink_hole.mat2 = FAIL
 1027 WIN : dEQP-GLES2.functional.fbo.render.color_clear.rbo_rgb5_a1 = FAIL
 1027 WIN : dEQP-GLES2.functional.fbo.render.color_clear.rbo_rgb5_a1_stencil_index8 = FAIL
 1027 WIN : dEQP-GLES2.functional.fbo.render.color_clear.rbo_rgb5_a1_depth_component16 = FAIL

src/tests/deqp_support/deqp_gles3_test_expectations.txt

@@ -568,12 +568,6 @@
 1098 WIN : dEQP-GLES3.functional.uniform_api.random.81 = FAIL
 1098 WIN : dEQP-GLES3.functional.uniform_api.random.83 = FAIL
 1098 WIN : dEQP-GLES3.functional.uniform_api.random.87 = FAIL
-1099 WIN : dEQP-GLES3.functional.attribute_location.bind_hole.mat2 = FAIL
-1099 WIN : dEQP-GLES3.functional.attribute_location.bind_hole.mat2x2 = FAIL
-1099 WIN : dEQP-GLES3.functional.attribute_location.layout_hole.mat2 = FAIL
-1099 WIN : dEQP-GLES3.functional.attribute_location.layout_hole.mat2x2 = FAIL
-1099 WIN : dEQP-GLES3.functional.attribute_location.mixed_hole.mat2 = FAIL
-1099 WIN : dEQP-GLES3.functional.attribute_location.mixed_hole.mat2x2 = FAIL
 1101 WIN : dEQP-GLES3.functional.polygon_offset.fixed16_render_with_units = FAIL
 1101 WIN : dEQP-GLES3.functional.polygon_offset.fixed24_render_with_units = FAIL
 1101 WIN : dEQP-GLES3.functional.lifetime.attach.deleted_input.buffer_vertex_array = FAIL

src/tests/gl_tests/GLSLTest.cpp

@@ -1428,6 +1428,142 @@ TEST_P(GLSLTest, VerifyMaxFragmentUniformVectorsExceeded)
     CompileGLSLWithUniformsAndSamplers(0, maxUniforms + 1, 0, 0, false);
 }
 
+// Test that two constructors which have vec4 and mat2 parameters get disambiguated (issue in
+// HLSL).
+TEST_P(GLSLTest_ES3, AmbiguousConstructorCall2x2)
+{
+    const std::string fragmentShaderSource =
+        "#version 300 es\n"
+        "precision highp float;\n"
+        "out vec4 my_FragColor;\n"
+        "void main()\n"
+        "{\n"
+        "    my_FragColor = vec4(0.0);\n"
+        "}";
+
+    const std::string vertexShaderSource =
+        "#version 300 es\n"
+        "precision highp float;\n"
+        "in vec4 a_vec;\n"
+        "in mat2 a_mat;\n"
+        "void main()\n"
+        "{\n"
+        "    gl_Position = vec4(a_vec) + vec4(a_mat);\n"
+        "}";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+// Test that two constructors which have mat2x3 and mat3x2 parameters get disambiguated.
+// This was suspected to be an issue in HLSL, but HLSL seems to be able to natively choose between
+// the function signatures in this case.
+TEST_P(GLSLTest_ES3, AmbiguousConstructorCall2x3)
+{
+    const std::string fragmentShaderSource =
+        "#version 300 es\n"
+        "precision highp float;\n"
+        "out vec4 my_FragColor;\n"
+        "void main()\n"
+        "{\n"
+        "    my_FragColor = vec4(0.0);\n"
+        "}";
+
+    const std::string vertexShaderSource =
+        "#version 300 es\n"
+        "precision highp float;\n"
+        "in mat3x2 a_matA;\n"
+        "in mat2x3 a_matB;\n"
+        "void main()\n"
+        "{\n"
+        "    gl_Position = vec4(a_matA) + vec4(a_matB);\n"
+        "}";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+// Test that two functions which have vec4 and mat2 parameters get disambiguated (issue in HLSL).
+TEST_P(GLSLTest_ES3, AmbiguousFunctionCall2x2)
+{
+    const std::string fragmentShaderSource =
+        "#version 300 es\n"
+        "precision highp float;\n"
+        "out vec4 my_FragColor;\n"
+        "void main()\n"
+        "{\n"
+        "    my_FragColor = vec4(0.0);\n"
+        "}";
+
+    const std::string vertexShaderSource =
+        "#version 300 es\n"
+        "precision highp float;\n"
+        "in vec4 a_vec;\n"
+        "in mat2 a_mat;\n"
+        "vec4 foo(vec4 a)\n"
+        "{\n"
+        "    return a;\n"
+        "}\n"
+        "vec4 foo(mat2 a)\n"
+        "{\n"
+        "    return vec4(a[0][0]);\n"
+        "}\n"
+        "void main()\n"
+        "{\n"
+        "    gl_Position = foo(a_vec) + foo(a_mat);\n"
+        "}";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+// Test that an user-defined function with a large number of float4 parameters doesn't fail due to
+// the function name being too long.
+TEST_P(GLSLTest_ES3, LargeNumberOfFloat4Parameters)
+{
+    const std::string fragmentShaderSource =
+        "#version 300 es\n"
+        "precision highp float;\n"
+        "out vec4 my_FragColor;\n"
+        "void main()\n"
+        "{\n"
+        "    my_FragColor = vec4(0.0);\n"
+        "}";
+
+    std::stringstream vertexShaderStream;
+    const unsigned int paramCount = 1024u;
+
+    vertexShaderStream << "#version 300 es\n"
+                          "precision highp float;\n"
+                          "in vec4 a_vec;\n"
+                          "vec4 lotsOfVec4Parameters(";
+    for (unsigned int i = 0; i < paramCount; ++i)
+    {
+        vertexShaderStream << "vec4 a" << i << ", ";
+    }
+    vertexShaderStream << "vec4 aLast)\n"
+                          "{\n"
+                          "    return ";
+    for (unsigned int i = 0; i < paramCount; ++i)
+    {
+        vertexShaderStream << "a" << i << " + ";
+    }
+    vertexShaderStream << "aLast;\n"
+                          "}\n"
+                          "void main()\n"
+                          "{\n"
+                          "    gl_Position = lotsOfVec4Parameters(";
+    for (unsigned int i = 0; i < paramCount; ++i)
+    {
+        vertexShaderStream << "a_vec, ";
+    }
+    vertexShaderStream << "a_vec);\n"
+                          "}";
+
+    GLuint program = CompileProgram(vertexShaderStream.str(), fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 ANGLE_INSTANTIATE_TEST(GLSLTest,
                        ES2_D3D9(),