Don't try to apply ForLoopUnroll to loops it can't handle
ForLoopUnroll should only mark loops that fit the limitations in ESSL
1.00 Appendix A.
BUG=angleproject:1253
TEST=angle_unittests, WebGL conformance tests
Change-Id: I00b0a7d29cd42efea9611d020aa1f873ac04773f
Reviewed-on: https://chromium-review.googlesource.com/317551
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 2f2be16..a48e0b4 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -294,12 +294,14 @@
// Unroll for-loop markup needs to happen after validateLimitations pass.
if (success && (compileOptions & SH_UNROLL_FOR_LOOP_WITH_INTEGER_INDEX))
{
- ForLoopUnrollMarker marker(ForLoopUnrollMarker::kIntegerIndex);
+ ForLoopUnrollMarker marker(ForLoopUnrollMarker::kIntegerIndex,
+ shouldRunLoopAndIndexingValidation(compileOptions));
root->traverse(&marker);
}
if (success && (compileOptions & SH_UNROLL_FOR_LOOP_WITH_SAMPLER_ARRAY_INDEX))
{
- ForLoopUnrollMarker marker(ForLoopUnrollMarker::kSamplerArrayIndex);
+ ForLoopUnrollMarker marker(ForLoopUnrollMarker::kSamplerArrayIndex,
+ shouldRunLoopAndIndexingValidation(compileOptions));
root->traverse(&marker);
if (marker.samplerArrayIndexIsFloatLoopIndex())
{
@@ -696,7 +698,7 @@
bool TCompiler::validateLimitations(TIntermNode* root)
{
- ValidateLimitations validate(shaderType, infoSink.info);
+ ValidateLimitations validate(shaderType, &infoSink.info);
root->traverse(&validate);
return validate.numErrors() == 0;
}
diff --git a/src/compiler/translator/ForLoopUnroll.cpp b/src/compiler/translator/ForLoopUnroll.cpp
index f3be20d..4cc1c26 100644
--- a/src/compiler/translator/ForLoopUnroll.cpp
+++ b/src/compiler/translator/ForLoopUnroll.cpp
@@ -6,6 +6,9 @@
#include "compiler/translator/ForLoopUnroll.h"
+#include "compiler/translator/ValidateLimitations.h"
+#include "angle_gl.h"
+
bool ForLoopUnrollMarker::visitBinary(Visit, TIntermBinary *node)
{
if (mUnrollCondition != kSamplerArrayIndex)
@@ -38,11 +41,16 @@
bool ForLoopUnrollMarker::visitLoop(Visit, TIntermLoop *node)
{
- if (mUnrollCondition == kIntegerIndex)
+ bool canBeUnrolled = mHasRunLoopValidation;
+ if (!mHasRunLoopValidation)
+ {
+ canBeUnrolled = ValidateLimitations::IsLimitedForLoop(node);
+ }
+ if (mUnrollCondition == kIntegerIndex && canBeUnrolled)
{
// Check if loop index type is integer.
- // This is called after ValidateLimitations pass, so all the calls
- // should be valid. See ValidateLimitations::validateForLoopInit().
+ // This is called after ValidateLimitations pass, so the loop has the limited form specified
+ // in ESSL 1.00 appendix A.
TIntermSequence *declSeq = node->getInit()->getAsAggregate()->getSequence();
TIntermSymbol *symbol = (*declSeq)[0]->getAsBinaryNode()->getLeft()->getAsSymbolNode();
if (symbol->getBasicType() == EbtInt)
@@ -50,11 +58,18 @@
}
TIntermNode *body = node->getBody();
- if (body != NULL)
+ if (body != nullptr)
{
- mLoopStack.push(node);
- body->traverse(this);
- mLoopStack.pop();
+ if (canBeUnrolled)
+ {
+ mLoopStack.push(node);
+ body->traverse(this);
+ mLoopStack.pop();
+ }
+ else
+ {
+ body->traverse(this);
+ }
}
// The loop is fully processed - no need to visit children.
return false;
diff --git a/src/compiler/translator/ForLoopUnroll.h b/src/compiler/translator/ForLoopUnroll.h
index 84894d4..9c49eca 100644
--- a/src/compiler/translator/ForLoopUnroll.h
+++ b/src/compiler/translator/ForLoopUnroll.h
@@ -24,11 +24,12 @@
kSamplerArrayIndex
};
- ForLoopUnrollMarker(UnrollCondition condition)
+ ForLoopUnrollMarker(UnrollCondition condition, bool hasRunLoopValidation)
: TIntermTraverser(true, false, false),
mUnrollCondition(condition),
mSamplerArrayIndexIsFloatLoopIndex(false),
- mVisitSamplerArrayIndexNodeInsideLoop(false)
+ mVisitSamplerArrayIndexNodeInsideLoop(false),
+ mHasRunLoopValidation(hasRunLoopValidation)
{
}
@@ -46,6 +47,7 @@
TLoopStack mLoopStack;
bool mSamplerArrayIndexIsFloatLoopIndex;
bool mVisitSamplerArrayIndexNodeInsideLoop;
+ bool mHasRunLoopValidation;
};
#endif // COMPILER_TRANSLATOR_FORLOOPUNROLL_H_
diff --git a/src/compiler/translator/ValidateLimitations.cpp b/src/compiler/translator/ValidateLimitations.cpp
index 4a84fb6..918f125 100644
--- a/src/compiler/translator/ValidateLimitations.cpp
+++ b/src/compiler/translator/ValidateLimitations.cpp
@@ -53,15 +53,37 @@
} // namespace anonymous
-ValidateLimitations::ValidateLimitations(sh::GLenum shaderType,
- TInfoSinkBase &sink)
+ValidateLimitations::ValidateLimitations(sh::GLenum shaderType, TInfoSinkBase *sink)
: TIntermTraverser(true, false, false),
mShaderType(shaderType),
mSink(sink),
- mNumErrors(0)
+ mNumErrors(0),
+ mValidateIndexing(true),
+ mValidateInnerLoops(true)
{
}
+// static
+bool ValidateLimitations::IsLimitedForLoop(TIntermLoop *loop)
+{
+ // The shader type doesn't matter in this case.
+ ValidateLimitations validate(GL_FRAGMENT_SHADER, nullptr);
+ validate.mValidateIndexing = false;
+ validate.mValidateInnerLoops = false;
+ if (!validate.validateLoopType(loop))
+ return false;
+ if (!validate.validateForLoopHeader(loop))
+ return false;
+ TIntermNode *body = loop->getBody();
+ if (body != nullptr)
+ {
+ validate.mLoopStack.push(loop);
+ body->traverse(&validate);
+ validate.mLoopStack.pop();
+ }
+ return (validate.mNumErrors == 0);
+}
+
bool ValidateLimitations::visitBinary(Visit, TIntermBinary *node)
{
// Check if loop index is modified in the loop body.
@@ -72,10 +94,11 @@
{
case EOpIndexDirect:
case EOpIndexIndirect:
- validateIndexing(node);
- break;
+ if (mValidateIndexing)
+ validateIndexing(node);
+ break;
default:
- break;
+ break;
}
return true;
}
@@ -102,6 +125,9 @@
bool ValidateLimitations::visitLoop(Visit, TIntermLoop *node)
{
+ if (!mValidateInnerLoops)
+ return true;
+
if (!validateLoopType(node))
return false;
@@ -123,9 +149,12 @@
void ValidateLimitations::error(TSourceLoc loc,
const char *reason, const char *token)
{
- mSink.prefix(EPrefixError);
- mSink.location(loc);
- mSink << "'" << token << "' : " << reason << "\n";
+ if (mSink)
+ {
+ mSink->prefix(EPrefixError);
+ mSink->location(loc);
+ (*mSink) << "'" << token << "' : " << reason << "\n";
+ }
++mNumErrors;
}
diff --git a/src/compiler/translator/ValidateLimitations.h b/src/compiler/translator/ValidateLimitations.h
index ec8891b..666e38f 100644
--- a/src/compiler/translator/ValidateLimitations.h
+++ b/src/compiler/translator/ValidateLimitations.h
@@ -17,7 +17,7 @@
class ValidateLimitations : public TIntermTraverser
{
public:
- ValidateLimitations(sh::GLenum shaderType, TInfoSinkBase &sink);
+ ValidateLimitations(sh::GLenum shaderType, TInfoSinkBase *sink);
int numErrors() const { return mNumErrors; }
@@ -26,6 +26,8 @@
bool visitAggregate(Visit, TIntermAggregate *) override;
bool visitLoop(Visit, TIntermLoop *) override;
+ static bool IsLimitedForLoop(TIntermLoop *node);
+
private:
void error(TSourceLoc loc, const char *reason, const char *token);
@@ -51,9 +53,11 @@
bool validateIndexing(TIntermBinary *node);
sh::GLenum mShaderType;
- TInfoSinkBase &mSink;
+ TInfoSinkBase *mSink;
int mNumErrors;
TLoopStack mLoopStack;
+ bool mValidateIndexing;
+ bool mValidateInnerLoops;
};
#endif // COMPILER_TRANSLATOR_VALIDATELIMITATIONS_H_