HLSL: Insert return statements into functions that are missing them.

src/compiler.gypi

@@ -160,6 +160,8 @@
         ],
         'angle_translator_lib_hlsl_sources':
         [
+            'compiler/translator/AddDefaultReturnStatements.cpp',
+            'compiler/translator/AddDefaultReturnStatements.h',
             'compiler/translator/ArrayReturnValueToOutParameter.cpp',
             'compiler/translator/ArrayReturnValueToOutParameter.h',
             'compiler/translator/ASTMetadataHLSL.cpp',

src/compiler/translator/AddDefaultReturnStatements.cpp

+//
+// Copyright (c) 2016 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// AddDefaultReturnStatements.cpp: Add default return statements to functions that do not end in a
+//                                 return.
+//
+
+#include "compiler/translator/AddDefaultReturnStatements.h"
+
+#include "compiler/translator/IntermNode.h"
+#include "compiler/translator/util.h"
+
+namespace sh
+{
+
+namespace
+{
+
+class AddDefaultReturnStatementsTraverser : private TIntermTraverser
+{
+  public:
+    static void Apply(TIntermNode *root)
+    {
+        AddDefaultReturnStatementsTraverser separateInit;
+        root->traverse(&separateInit);
+        separateInit.updateTree();
+    }
+
+  private:
+    AddDefaultReturnStatementsTraverser() : TIntermTraverser(true, false, false) {}
+
+    static bool IsFunctionWithoutReturnStatement(TIntermAggregate *node, TType *returnType)
+    {
+        *returnType = node->getType();
+        if (node->getOp() != EOpFunction || node->getType().getBasicType() == EbtVoid)
+        {
+            return false;
+        }
+
+        TIntermAggregate *lastNode = node->getSequence()->back()->getAsAggregate();
+        if (lastNode == nullptr)
+        {
+            return true;
+        }
+
+        TIntermBranch *returnNode = lastNode->getSequence()->front()->getAsBranchNode();
+        if (returnNode != nullptr && returnNode->getFlowOp() == EOpReturn)
+        {
+            return false;
+        }
+
+        return true;
+    }
+
+    static TIntermTyped *GenerateTypeConstructor(const TType &returnType)
+    {
+        // Base case, constructing a single element
+        if (!returnType.isArray())
+        {
+            size_t objectSize             = returnType.getObjectSize();
+            TConstantUnion *constantUnion = new TConstantUnion[objectSize];
+            for (size_t constantIdx = 0; constantIdx < objectSize; constantIdx++)
+            {
+                constantUnion[constantIdx].setFConst(0.0f);
+            }
+
+            TIntermConstantUnion *intermConstantUnion =
+                new TIntermConstantUnion(constantUnion, returnType);
+            return intermConstantUnion;
+        }
+
+        // Recursive case, construct an array of single elements
+        TIntermAggregate *constructorAggrigate =
+            new TIntermAggregate(TypeToConstructorOperator(returnType));
+        constructorAggrigate->setType(returnType);
+
+        size_t arraySize = returnType.getArraySize();
+        for (size_t arrayIdx = 0; arrayIdx < arraySize; arrayIdx++)
+        {
+            TType arrayElementType(returnType);
+            arrayElementType.clearArrayness();
+
+            constructorAggrigate->getSequence()->push_back(
+                GenerateTypeConstructor(arrayElementType));
+        }
+
+        return constructorAggrigate;
+    }
+
+    bool visitAggregate(Visit, TIntermAggregate *node) override
+    {
+        TType returnType;
+        if (IsFunctionWithoutReturnStatement(node, &returnType))
+        {
+            TIntermBranch *branch =
+                new TIntermBranch(EOpReturn, GenerateTypeConstructor(returnType));
+
+            TIntermAggregate *lastNode = node->getSequence()->back()->getAsAggregate();
+            lastNode->getSequence()->push_back(branch);
+
+            return false;
+        }
+
+        return true;
+    }
+};
+}  // anonymous namespace
+
+void AddDefaultReturnStatements(TIntermNode *node)
+{
+    AddDefaultReturnStatementsTraverser::Apply(node);
+}
+
+}  // namespace sh

src/compiler/translator/AddDefaultReturnStatements.h

+//
+// Copyright (c) 2016 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// AddDefaultReturnStatements.h: Add default return statements to functions that do not end in a
+//                               return.
+//
+
+#ifndef COMPILER_TRANSLATOR_ADDDEFAULTRETURNSTATEMENTS_H_
+#define COMPILER_TRANSLATOR_ADDDEFAULTRETURNSTATEMENTS_H_
+
+class TIntermNode;
+
+namespace sh
+{
+
+void AddDefaultReturnStatements(TIntermNode *node);
+
+}  // namespace sh
+
+#endif  // COMPILER_TRANSLATOR_ADDDEFAULTRETURNSTATEMENTS_H_

src/compiler/translator/ParseContext.cpp

@@ -2241,135 +2241,7 @@ TFunction *TParseContext::addConstructorFunc(const TPublicType &publicTypeIn)
     }
     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 = sh::TypeToConstructorOperator(TType(publicType));
         if (op == EOpNull)
         {
             error(publicType.line, "cannot construct this type", getBasicString(publicType.type));

src/compiler/translator/TranslatorHLSL.cpp

@@ -6,6 +6,7 @@
 
 #include "compiler/translator/TranslatorHLSL.h"
 
+#include "compiler/translator/AddDefaultReturnStatements.h"
 #include "compiler/translator/ArrayReturnValueToOutParameter.h"
 #include "compiler/translator/EmulatePrecision.h"
 #include "compiler/translator/IntermNodePatternMatcher.h"
@@ -28,6 +29,8 @@ void TranslatorHLSL::translate(TIntermNode *root, int compileOptions)
     const ShBuiltInResources &resources = getResources();
     int numRenderTargets = resources.EXT_draw_buffers ? resources.MaxDrawBuffers : 1;
 
+    sh::AddDefaultReturnStatements(root);
+
     SeparateDeclarations(root);
 
     // TODO (oetuaho): Sequence operators should also be split in case there is dynamic indexing of

src/compiler/translator/util.cpp

@@ -330,6 +330,134 @@ TType ConvertShaderVariableTypeToTType(sh::GLenum type)
     }
 }
 
+TOperator TypeToConstructorOperator(const TType &type)
+{
+    switch (type.getBasicType())
+    {
+        case EbtFloat:
+            if (type.isMatrix())
+            {
+                switch (type.getCols())
+                {
+                    case 2:
+                        switch (type.getRows())
+                        {
+                            case 2:
+                                return EOpConstructMat2;
+                            case 3:
+                                return EOpConstructMat2x3;
+                            case 4:
+                                return EOpConstructMat2x4;
+                            default:
+                                break;
+                        }
+                        break;
+
+                    case 3:
+                        switch (type.getRows())
+                        {
+                            case 2:
+                                return EOpConstructMat3x2;
+                            case 3:
+                                return EOpConstructMat3;
+                            case 4:
+                                return EOpConstructMat3x4;
+                            default:
+                                break;
+                        }
+                        break;
+
+                    case 4:
+                        switch (type.getRows())
+                        {
+                            case 2:
+                                return EOpConstructMat4x2;
+                            case 3:
+                                return EOpConstructMat4x3;
+                            case 4:
+                                return EOpConstructMat4;
+                            default:
+                                break;
+                        }
+                        break;
+                }
+            }
+            else
+            {
+                switch (type.getNominalSize())
+                {
+                    case 1:
+                        return EOpConstructFloat;
+                    case 2:
+                        return EOpConstructVec2;
+                    case 3:
+                        return EOpConstructVec3;
+                    case 4:
+                        return EOpConstructVec4;
+                    default:
+                        break;
+                }
+            }
+            break;
+
+        case EbtInt:
+            switch (type.getNominalSize())
+            {
+                case 1:
+                    return EOpConstructInt;
+                case 2:
+                    return EOpConstructIVec2;
+                case 3:
+                    return EOpConstructIVec3;
+                case 4:
+                    return EOpConstructIVec4;
+                default:
+                    break;
+            }
+            break;
+
+        case EbtUInt:
+            switch (type.getNominalSize())
+            {
+                case 1:
+                    return EOpConstructUInt;
+                case 2:
+                    return EOpConstructUVec2;
+                case 3:
+                    return EOpConstructUVec3;
+                case 4:
+                    return EOpConstructUVec4;
+                default:
+                    break;
+            }
+            break;
+
+        case EbtBool:
+            switch (type.getNominalSize())
+            {
+                case 1:
+                    return EOpConstructBool;
+                case 2:
+                    return EOpConstructBVec2;
+                case 3:
+                    return EOpConstructBVec3;
+                case 4:
+                    return EOpConstructBVec4;
+                default:
+                    break;
+            }
+            break;
+
+        case EbtStruct:
+            return EOpConstructStruct;
+
+        default:
+            break;
+    }
+
+    return EOpNull;
+}
+
 GetVariableTraverser::GetVariableTraverser(const TSymbolTable &symbolTable)
     : mSymbolTable(symbolTable)
 {

src/compiler/translator/util.h

@@ -12,6 +12,7 @@
 #include "angle_gl.h"
 #include <GLSLANG/ShaderLang.h>
 
+#include "compiler/translator/Operator.h"
 #include "compiler/translator/Types.h"
 
 // strtof_clamp is like strtof but
@@ -39,6 +40,8 @@ TString ArrayString(const TType &type);
 // Handles only basic output variable types.
 TType ConvertShaderVariableTypeToTType(sh::GLenum type);
 
+TOperator TypeToConstructorOperator(const TType &type);
+
 class GetVariableTraverser : angle::NonCopyable
 {
   public:

src/tests/deqp_support/deqp_gles2_test_expectations.txt

@@ -69,8 +69,6 @@
 1027 WIN : dEQP-GLES2.functional.fbo.render.recreate_colorbuffer.no_rebind_tex2d_rgb_stencil_index8 = FAIL
 1027 WIN : dEQP-GLES2.functional.fbo.render.recreate_colorbuffer.no_rebind_tex2d_rgba_stencil_index8 = FAIL
 1033 WIN : dEQP-GLES2.functional.polygon_offset.fixed16_render_with_units = FAIL
-1015 WIN : dEQP-GLES2.functional.shaders.functions.misc.missing_returns_vertex = FAIL
-1015 WIN : dEQP-GLES2.functional.shaders.functions.misc.missing_returns_fragment = FAIL
 1017 WIN : dEQP-GLES2.functional.shaders.loops.for_constant_iterations.nested_sequence_* = FAIL
 1017 WIN : dEQP-GLES2.functional.shaders.loops.for_constant_iterations.nested_tricky_dataflow_* = FAIL
 1017 WIN : dEQP-GLES2.functional.shaders.loops.while_constant_iterations.nested_tricky_dataflow_* = FAIL

src/tests/gl_tests/GLSLTest.cpp

@@ -906,6 +906,159 @@ TEST_P(GLSLTest, InvariantAllBoth)
     EXPECT_NE(0u, program);
 }
 
+// Verify that functions without return statements still compile
+TEST_P(GLSLTest, MissingReturnFloat)
+{
+    const std::string vertexShaderSource =
+        "varying float v_varying;\n"
+        "float f() { if (v_varying > 0.0) return 1.0; }\n"
+        "void main() { gl_Position = vec4(f(), 0, 0, 1); }\n";
+
+    const std::string fragmentShaderSource =
+        "precision mediump float;\n"
+        "void main() { gl_FragColor = vec4(0, 0, 0, 1); }\n";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+// Verify that functions without return statements still compile
+TEST_P(GLSLTest, MissingReturnVec2)
+{
+    const std::string vertexShaderSource =
+        "varying float v_varying;\n"
+        "vec2 f() { if (v_varying > 0.0) return vec2(1.0, 1.0); }\n"
+        "void main() { gl_Position = vec4(f().x, 0, 0, 1); }\n";
+
+    const std::string fragmentShaderSource =
+        "precision mediump float;\n"
+        "void main() { gl_FragColor = vec4(0, 0, 0, 1); }\n";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+// Verify that functions without return statements still compile
+TEST_P(GLSLTest, MissingReturnVec3)
+{
+    const std::string vertexShaderSource =
+        "varying float v_varying;\n"
+        "vec3 f() { if (v_varying > 0.0) return vec3(1.0, 1.0, 1.0); }\n"
+        "void main() { gl_Position = vec4(f().x, 0, 0, 1); }\n";
+
+    const std::string fragmentShaderSource =
+        "precision mediump float;\n"
+        "void main() { gl_FragColor = vec4(0, 0, 0, 1); }\n";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+// Verify that functions without return statements still compile
+TEST_P(GLSLTest, MissingReturnVec4)
+{
+    const std::string vertexShaderSource =
+        "varying float v_varying;\n"
+        "vec4 f() { if (v_varying > 0.0) return vec4(1.0, 1.0, 1.0, 1.0); }\n"
+        "void main() { gl_Position = vec4(f().x, 0, 0, 1); }\n";
+
+    const std::string fragmentShaderSource =
+        "precision mediump float;\n"
+        "void main() { gl_FragColor = vec4(0, 0, 0, 1); }\n";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+// Verify that functions without return statements still compile
+TEST_P(GLSLTest, MissingReturnIVec4)
+{
+    const std::string vertexShaderSource =
+        "varying float v_varying;\n"
+        "ivec4 f() { if (v_varying > 0.0) return ivec4(1, 1, 1, 1); }\n"
+        "void main() { gl_Position = vec4(f().x, 0, 0, 1); }\n";
+
+    const std::string fragmentShaderSource =
+        "precision mediump float;\n"
+        "void main() { gl_FragColor = vec4(0, 0, 0, 1); }\n";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+// Verify that functions without return statements still compile
+TEST_P(GLSLTest, MissingReturnMat4)
+{
+    const std::string vertexShaderSource =
+        "varying float v_varying;\n"
+        "mat4 f() { if (v_varying > 0.0) return mat4(1.0); }\n"
+        "void main() { gl_Position = vec4(f()[0][0], 0, 0, 1); }\n";
+
+    const std::string fragmentShaderSource =
+        "precision mediump float;\n"
+        "void main() { gl_FragColor = vec4(0, 0, 0, 1); }\n";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+// Verify that functions without return statements still compile
+TEST_P(GLSLTest, MissingReturnStruct)
+{
+    const std::string vertexShaderSource =
+        "varying float v_varying;\n"
+        "struct s { float a; int b; vec2 c; };\n"
+        "s f() { if (v_varying > 0.0) return s(1.0, 1, vec2(1.0, 1.0)); }\n"
+        "void main() { gl_Position = vec4(f().a, 0, 0, 1); }\n";
+
+    const std::string fragmentShaderSource =
+        "precision mediump float;\n"
+        "void main() { gl_FragColor = vec4(0, 0, 0, 1); }\n";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+// Verify that functions without return statements still compile
+TEST_P(GLSLTest_ES3, MissingReturnArray)
+{
+    const std::string vertexShaderSource =
+        "#version 300 es\n"
+        "in float v_varying;\n"
+        "vec2[2] f() { if (v_varying > 0.0) { return vec2[2](vec2(1.0, 1.0), vec2(1.0, 1.0)); } }\n"
+        "void main() { gl_Position = vec4(f()[0].x, 0, 0, 1); }\n";
+
+    const std::string fragmentShaderSource =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "out vec4 my_FragColor;\n"
+        "void main() { my_FragColor = vec4(0, 0, 0, 1); }\n";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+// Verify that functions without return statements still compile
+TEST_P(GLSLTest_ES3, MissingReturnArrayOfStructs)
+{
+    const std::string vertexShaderSource =
+        "#version 300 es\n"
+        "in float v_varying;\n"
+        "struct s { float a; int b; vec2 c; };\n"
+        "s[2] f() { if (v_varying > 0.0) { return s[2](s(1.0, 1, vec2(1.0, 1.0)), s(1.0, 1, "
+        "vec2(1.0, 1.0))); } }\n"
+        "void main() { gl_Position = vec4(f()[0].a, 0, 0, 1); }\n";
+
+    const std::string fragmentShaderSource =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "out vec4 my_FragColor;\n"
+        "void main() { my_FragColor = vec4(0, 0, 0, 1); }\n";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
 // Verify that using invariant(all) in both shaders fails in ESSL 3.00.
 TEST_P(GLSLTest_ES3, InvariantAllBoth)
 {