Commit a1884f2b by Corentin Wallez

Fixes for the tagging of discontinuous loops

The first fix was for all loops being considered discontinuous in OutputHLSL because of a typo that produced a tautology. The error was not detected by the unit tests because as an optimization we do not generate Lod0 calls when they are not needed for the callee function (which was correctly detected by the analysis in this case). Fixed the unit tests by adding a call to a builtin gradient operation. The second fix was for discard not being taken into account in the analyses of the AST, which caused a WebGL test regression after the first fix for conformance/glsl/bugs/conditional-discard-in-loop BUG=angleproject:982 Change-Id: I1315eac1ad36f726be52d7fda5facf3104341b1f Reviewed-on: https://chromium-review.googlesource.com/267814Tested-by: 's avatarCorentin Wallez <cwallez@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org>
parent ec6bae71
...@@ -213,8 +213,6 @@ class PullComputeDiscontinuousLoops : public TIntermTraverser ...@@ -213,8 +213,6 @@ class PullComputeDiscontinuousLoops : public TIntermTraverser
{ {
switch (node->getFlowOp()) switch (node->getFlowOp())
{ {
case EOpKill:
break;
case EOpBreak: case EOpBreak:
{ {
ASSERT(!mLoopsAndSwitches.empty()); ASSERT(!mLoopsAndSwitches.empty());
...@@ -241,8 +239,9 @@ class PullComputeDiscontinuousLoops : public TIntermTraverser ...@@ -241,8 +239,9 @@ class PullComputeDiscontinuousLoops : public TIntermTraverser
onDiscontinuousLoop(); onDiscontinuousLoop();
} }
break; break;
case EOpKill:
case EOpReturn: case EOpReturn:
// A return jumps out of all the enclosing loops // A return or discard jumps out of all the enclosing loops
if (!mLoopsAndSwitches.empty()) if (!mLoopsAndSwitches.empty())
{ {
for (TIntermNode* node : mLoopsAndSwitches) for (TIntermNode* node : mLoopsAndSwitches)
......
...@@ -2415,7 +2415,7 @@ bool OutputHLSL::visitLoop(Visit visit, TIntermLoop *node) ...@@ -2415,7 +2415,7 @@ bool OutputHLSL::visitLoop(Visit visit, TIntermLoop *node)
bool wasDiscontinuous = mInsideDiscontinuousLoop; bool wasDiscontinuous = mInsideDiscontinuousLoop;
mInsideDiscontinuousLoop = mInsideDiscontinuousLoop || mInsideDiscontinuousLoop = mInsideDiscontinuousLoop ||
mCurrentFunctionMetadata->mDiscontinuousLoops.count(node) >= 0; mCurrentFunctionMetadata->mDiscontinuousLoops.count(node) > 0;
if (mOutputType == SH_HLSL9_OUTPUT) if (mOutputType == SH_HLSL9_OUTPUT)
{ {
......
...@@ -149,12 +149,13 @@ TEST_F(UnrollFlattenTest, GradientNotInDiscont) ...@@ -149,12 +149,13 @@ TEST_F(UnrollFlattenTest, GradientNotInDiscont)
" for (int i = 0; i < 10; i++) {\n" // 4 " for (int i = 0; i < 10; i++) {\n" // 4
" if (a > 1.0) {}\n" // 5 " if (a > 1.0) {}\n" // 5
" a = fun(a);\n" // 6 " a = fun(a);\n" // 6
" a += texture2D(tex, vec2(a, 0.0)).x;" // 7
" }\n" " }\n"
" return a;\n" " return a;\n"
"}\n" "}\n"
"void main() {\n" "void main() {\n"
" float accum = 0.0;\n" " float accum = 0.0;\n"
" if (f < 5.0) {accum = fun2(accum);}\n" // 7 " if (f < 5.0) {accum = fun2(accum);}\n" // 8
" gl_FragColor = vec4(accum);\n" " gl_FragColor = vec4(accum);\n"
"}\n"; "}\n";
// 1 - shouldn't get a Lod0 version generated // 1 - shouldn't get a Lod0 version generated
...@@ -162,13 +163,14 @@ TEST_F(UnrollFlattenTest, GradientNotInDiscont) ...@@ -162,13 +163,14 @@ TEST_F(UnrollFlattenTest, GradientNotInDiscont)
// 3 - shouldn't get a Lod0 version generated (not in discont loop) // 3 - shouldn't get a Lod0 version generated (not in discont loop)
// 4 - should have LOOP because it contains a gradient operation (even if Lod0) // 4 - should have LOOP because it contains a gradient operation (even if Lod0)
// 5 - no FLATTEN because doesn't contain discont loop // 5 - no FLATTEN because doesn't contain discont loop
// 6 - call Lod0 version // 6 - call non-Lod0 version
// 7 - no FLATTEN // 7 - call non-Lod0 version
// 8 - no FLATTEN
compile(shaderString); compile(shaderString);
const char *expectations[] = const char *expectations[] =
{ {
"fun(", "texture2D(", "fun(", "texture2D(",
"fun2(", "LOOP", "for", "if", "fun(", "fun2(", "LOOP", "for", "if", "fun(", "texture2D(",
"main(", "if", "fun2(" "main(", "if", "fun2("
}; };
expect(expectations, ArraySize(expectations)); expect(expectations, ArraySize(expectations));
...@@ -189,12 +191,13 @@ TEST_F(UnrollFlattenTest, GradientInDiscont) ...@@ -189,12 +191,13 @@ TEST_F(UnrollFlattenTest, GradientInDiscont)
" for (int i = 0; i < 10; i++) {\n" // 4 " for (int i = 0; i < 10; i++) {\n" // 4
" if (a > 1.0) {break;}\n" // 5 " if (a > 1.0) {break;}\n" // 5
" a = fun(a);\n" // 6 " a = fun(a);\n" // 6
" a += texture2D(tex, vec2(a, 0.0)).x;" // 7
" }\n" " }\n"
" return a;\n" " return a;\n"
"}\n" "}\n"
"void main() {\n" "void main() {\n"
" float accum = 0.0;\n" " float accum = 0.0;\n"
" if (f < 5.0) {accum = fun2(accum);}\n" // 7 " if (f < 5.0) {accum = fun2(accum);}\n" // 8
" gl_FragColor = vec4(accum);\n" " gl_FragColor = vec4(accum);\n"
"}\n"; "}\n";
// 1 - should get a Lod0 version generated (gradient + discont loop) // 1 - should get a Lod0 version generated (gradient + discont loop)
...@@ -203,13 +206,14 @@ TEST_F(UnrollFlattenTest, GradientInDiscont) ...@@ -203,13 +206,14 @@ TEST_F(UnrollFlattenTest, GradientInDiscont)
// 4 - should have LOOP because it contains a gradient operation (even if Lod0) // 4 - should have LOOP because it contains a gradient operation (even if Lod0)
// 5 - no FLATTEN because doesn't contain discont loop // 5 - no FLATTEN because doesn't contain discont loop
// 6 - call Lod0 version // 6 - call Lod0 version
// 7 - should have a FLATTEN because has a discont loop and gradient // 7 - call Lod0 version
// 8 - should have a FLATTEN because has a discont loop and gradient
compile(shaderString); compile(shaderString);
const char *expectations[] = const char *expectations[] =
{ {
"fun(", "texture2D(", "fun(", "texture2D(",
"funLod0(", "texture2DLod0(", "funLod0(", "texture2DLod0(",
"fun2(", "LOOP", "for", "if", "break", "funLod0(", "fun2(", "LOOP", "for", "if", "break", "funLod0(", "texture2DLod0",
"main(", "FLATTEN", "if", "fun2(" "main(", "FLATTEN", "if", "fun2("
}; };
expect(expectations, ArraySize(expectations)); expect(expectations, ArraySize(expectations));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment