M117: Metal: Fix dropped out arguments from functions with many args.
RewriteOutArgs has an early-exit if it spots a potentially aliased
arg. It's also responsible for marking out args as references,
which caused an issue in Google Earth.
Removing the early-exit fixes both issues.
Bug: chromium:1474736
Change-Id: Ib68dd3f3e2e0a1e773e4e09edcdfa3a4bdfc1ef2
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4823006
Commit-Queue: Jonah Ryan-Davis <jonahr@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
(cherry picked from commit 1ab5d01d9b0ff56522ddbfd6cbfe7cbea51e0c8e)
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4851051
Commit-Queue: Prudhvikumar Bommana <pbommana@google.com>
diff --git a/src/compiler/translator/tree_ops/msl/RewriteOutArgs.cpp b/src/compiler/translator/tree_ops/msl/RewriteOutArgs.cpp
index dda3d1f..6534457 100644
--- a/src/compiler/translator/tree_ops/msl/RewriteOutArgs.cpp
+++ b/src/compiler/translator/tree_ops/msl/RewriteOutArgs.cpp
@@ -125,23 +125,11 @@
const TVariable ¶m = *func->getParam(i);
const TType ¶mType = param.getType();
const TQualifier paramQual = paramType.getQualifier();
- switch (paramQual)
- {
- case TQualifier::EvqParamOut:
- case TQualifier::EvqParamInOut:
- if (!mSymbolEnv.isReference(param))
- {
- mSymbolEnv.markAsReference(param, AddressSpace::Thread);
- }
- break;
- default:
- break;
- }
return paramQual;
};
+ // Check which params might be aliased, and mark all out params as references.
bool mightAlias = false;
-
for (size_t i = 0; i < argCount; ++i)
{
const TQualifier paramQual = getParamQualifier(i);
@@ -151,11 +139,17 @@
case TQualifier::EvqParamOut:
case TQualifier::EvqParamInOut:
{
+ const TVariable ¶m = *func->getParam(i);
+ if (!mSymbolEnv.isReference(param))
+ {
+ mSymbolEnv.markAsReference(param, AddressSpace::Thread);
+ }
+ // Note: not the same as param above, this refers to the variable in the
+ // argument list in the callsite.
const TVariable *var = GetVariable(*args[i]);
if (mVarBuffer.insert(var).count > 1)
{
mightAlias = true;
- i = argCount;
}
}
break;
diff --git a/src/tests/compiler_tests/MSLOutput_test.cpp b/src/tests/compiler_tests/MSLOutput_test.cpp
index 1355dde..79f1df9 100644
--- a/src/tests/compiler_tests/MSLOutput_test.cpp
+++ b/src/tests/compiler_tests/MSLOutput_test.cpp
@@ -635,3 +635,215 @@
// When WebKit build is able to run the tests, this should be changed to something else.
// ASSERT_TRUE(foundInCode(SH_MSL_METAL_OUTPUT, "__unnamed"));
}
+
+TEST_F(MSLOutputTest, GlobalRescopingSimple)
+{
+ const std::string &shaderString =
+ R"(#version 300 es
+ precision mediump float;
+
+ // Should rescope uf into main
+
+ float uf;
+ out vec4 my_FragColor;
+
+ void main()
+ {
+ uf += 1.0f;
+ my_FragColor = vec4(uf, 0.0, 0.0, 1.0);
+ })";
+ compile(shaderString);
+}
+
+TEST_F(MSLOutputTest, GlobalRescopingNoRescope)
+{
+ const std::string &shaderString =
+ R"(#version 300 es
+ precision mediump float;
+
+ // Should not rescope any variable
+
+ float uf;
+ out vec4 my_FragColor;
+ void modifyGlobal()
+ {
+ uf = 1.0f;
+ }
+ void main()
+ {
+ modifyGlobal();
+ my_FragColor = vec4(uf, 0.0, 0.0, 1.0);
+ })";
+ compile(shaderString);
+}
+
+TEST_F(MSLOutputTest, GlobalRescopingInitializer)
+{
+ const std::string &shaderString =
+ R"(#version 300 es
+ precision mediump float;
+
+ // Should rescope uf into main
+
+ float uf = 1.0f;
+ out vec4 my_FragColor;
+
+ void main()
+ {
+ uf += 1.0;
+ my_FragColor = vec4(uf, 0.0, 0.0, 1.0);
+ })";
+ compile(shaderString);
+}
+
+TEST_F(MSLOutputTest, GlobalRescopingInitializerNoRescope)
+{
+ const std::string &shaderString =
+ R"(#version 300 es
+ precision mediump float;
+
+ // Should not rescope any variable
+
+ float uf = 1.0f;
+ out vec4 my_FragColor;
+
+ void modifyGlobal()
+ {
+ uf =+ 1.0f;
+ }
+ void main()
+ {
+ modifyGlobal();
+ my_FragColor = vec4(uf, 0.0, 0.0, 1.0);
+ })";
+ compile(shaderString);
+}
+
+TEST_F(MSLOutputTest, GlobalRescopingNestedFunction)
+{
+ const std::string &shaderString =
+ R"(#version 300 es
+ precision mediump float;
+
+ // Should rescope a info modifyGlobal
+
+ float a = 1.0f;
+ float uf = 1.0f;
+ out vec4 my_FragColor;
+
+ void modifyGlobal()
+ {
+ uf =+ a;
+ }
+ void main()
+ {
+ modifyGlobal();
+ my_FragColor = vec4(uf, 0.0, 0.0, 1.0);
+ })";
+ compile(shaderString);
+}
+
+TEST_F(MSLOutputTest, GlobalRescopingMultipleUses)
+{
+ const std::string &shaderString =
+ R"(#version 300 es
+ precision mediump float;
+
+ // Should rescope uf into main
+
+ float uf = 1.0f;
+ out vec4 my_FragColor;
+
+ void main()
+ {
+ uf =+ 1.0;
+ if (uf > 0.0)
+ {
+ uf =- 0.5;
+ }
+ my_FragColor = vec4(uf, 0.0, 0.0, 1.0);
+ })";
+ compile(shaderString);
+}
+
+TEST_F(MSLOutputTest, GlobalRescopingGloballyReferencedVar)
+{
+ const std::string &shaderString =
+ R"(#version 300 es
+ precision mediump float;
+
+ // Should rescope uf into main
+
+ const float a = 1.0f;
+ float uf = a;
+ out vec4 my_FragColor;
+
+ void main()
+ {
+ my_FragColor = vec4(uf, 0.0, a, 0.0);
+ })";
+ compile(shaderString);
+}
+
+TEST_F(MSLOutputTest, GlobalRescopingDeclarationAfterFunction)
+{
+ const std::string &shaderString =
+ R"(#version 300 es
+ precision mediump float;
+
+ // Should rescope c and b into main
+
+ float a = 1.0f;
+ float c = 1.0f;
+ out vec4 my_FragColor;
+
+ void modifyGlobal()
+ {
+ a =+ 1.0f;
+ }
+
+ float b = 1.0f;
+
+ void main()
+ {
+ modifyGlobal();
+ my_FragColor = vec4(a, b, c, 0.0);
+ }
+
+ )";
+ compile(shaderString);
+}
+
+TEST_F(MSLOutputTest, ReusedOutVarName)
+{
+ const std::string &shaderString =
+ R"(#version 300 es
+ precision mediump float;
+
+ out vec4 my_FragColor;
+
+ void funcWith1Out(
+ out float outC) {
+ outC = 1.0;
+ }
+
+ void funcWith4Outs(
+ out float outA,
+ out float outB,
+ out float outC,
+ out float outD) {
+ outA = 1.0;
+ outB = 1.0;
+ outD = 1.0;
+ }
+
+
+ void main()
+ {
+ funcWith1Out(my_FragColor.g);
+ funcWith4Outs(my_FragColor.r, my_FragColor.g, my_FragColor.b, my_FragColor.a);
+ }
+
+ )";
+ compile(shaderString);
+}
diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp
index c11f6f8..c35d1ee 100644
--- a/src/tests/gl_tests/GLSLTest.cpp
+++ b/src/tests/gl_tests/GLSLTest.cpp
@@ -16188,6 +16188,34 @@
ASSERT_GL_NO_ERROR();
}
+// Test that aliasing function out parameters work even when multiple params are aliased.
+TEST_P(GLSLTest, AliasingFunctionOutParamsMultiple)
+{
+ constexpr char kFS[] = R"(precision highp float;
+
+const vec4 colorGreen = vec4(0.,1.,0.,1.);
+const vec4 colorRed = vec4(1.,0.,0.,1.);
+
+bool outParametersAreDistinct(out float x, out float y, out float z, out float a) {
+ x = 1.0;
+ y = 2.0;
+ z = 3.0;
+ a = 4.0;
+ return x == 1.0 && y == 2.0 && z == 3.0 && a == 4.0;
+}
+void main() {
+ float x = 0.0;
+ float y = 0.0;
+ gl_FragColor = outParametersAreDistinct(x, x, y, y) ? colorGreen : colorRed;
+}
+)";
+
+ ANGLE_GL_PROGRAM(testProgram, essl1_shaders::vs::Simple(), kFS);
+ drawQuad(testProgram, essl3_shaders::PositionAttrib(), 0.5f, 1.0f, true);
+ EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+ ASSERT_GL_NO_ERROR();
+}
+
// Test that aliasing function inout parameters work.
TEST_P(GLSLTest, AliasingFunctionInOutParams)
{
@@ -16213,6 +16241,34 @@
ASSERT_GL_NO_ERROR();
}
+// Test that aliasing function inout parameters work when more than one param is aliased.
+TEST_P(GLSLTest, AliasingFunctionInOutParamsMultiple)
+{
+ constexpr char kFS[] = R"(precision highp float;
+
+const vec4 colorGreen = vec4(0.,1.,0.,1.);
+const vec4 colorRed = vec4(1.,0.,0.,1.);
+
+bool inoutParametersAreDistinct(inout float x, inout float y, inout float z, inout float a) {
+ x = 1.0;
+ y = 2.0;
+ z = 3.0;
+ a = 4.0;
+ return x == 1.0 && y == 2.0 && z == 3.0 && a == 4.0;
+}
+void main() {
+ float x = 0.0;
+ float y = 0.0;
+ gl_FragColor = inoutParametersAreDistinct(x, x, y, y) ? colorGreen : colorRed;
+}
+)";
+
+ ANGLE_GL_PROGRAM(testProgram, essl1_shaders::vs::Simple(), kFS);
+ drawQuad(testProgram, essl3_shaders::PositionAttrib(), 0.5f, 1.0f, true);
+ EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+ ASSERT_GL_NO_ERROR();
+}
+
// Test that aliasing function out parameter with a global works.
TEST_P(GLSLTest, AliasingFunctionOutParamAndGlobal)
{
