Improvement on loop unrolling with loops indexing sampler arrays

include/GLSLANG/ShaderLang.h

@@ -170,9 +170,14 @@ typedef enum {
   SH_SOURCE_PATH             = 0x0020,
   SH_MAP_LONG_VARIABLE_NAMES = 0x0040,
   SH_UNROLL_FOR_LOOP_WITH_INTEGER_INDEX = 0x0080,
+  // If a sampler array index happens to be a loop index,
+  //   1) if its type is integer, unroll the loop.
+  //   2) if its type is float, fail the shader compile.
+  // This is to work around a mac driver bug.
+  SH_UNROLL_FOR_LOOP_WITH_SAMPLER_ARRAY_INDEX = 0x0100,
 
   // This is needed only as a workaround for certain OpenGL driver bugs.
-  SH_EMULATE_BUILT_IN_FUNCTIONS = 0x0100,
+  SH_EMULATE_BUILT_IN_FUNCTIONS = 0x0200,
 
   // This is an experimental flag to enforce restrictions that aim to prevent 
   // timing attacks.
@@ -180,15 +185,15 @@ typedef enum {
   // texture information via the timing channel.
   // To use this flag, you must compile the shader under the WebGL spec
   // (using the SH_WEBGL_SPEC flag).
-  SH_TIMING_RESTRICTIONS = 0x0200,
-    
+  SH_TIMING_RESTRICTIONS = 0x0400,
+
   // This flag prints the dependency graph that is used to enforce timing
   // restrictions on fragment shaders.
   // This flag only has an effect if all of the following are true:
   // - The shader spec is SH_WEBGL_SPEC.
   // - The compile options contain the SH_TIMING_RESTRICTIONS flag.
   // - The shader type is SH_FRAGMENT_SHADER.
-  SH_DEPENDENCY_GRAPH = 0x0400,
+  SH_DEPENDENCY_GRAPH = 0x0800,
 
   // Enforce the GLSL 1.017 Appendix A section 7 packing restrictions.
   // This flag only enforces (and can only enforce) the packing
@@ -196,7 +201,7 @@ typedef enum {
   // shaders. ShCheckVariablesWithinPackingLimits() lets embedders
   // enforce the packing restrictions for varying variables during
   // program link time.
-  SH_ENFORCE_PACKING_RESTRICTIONS = 0x0800,
+  SH_ENFORCE_PACKING_RESTRICTIONS = 0x1000,
 
   // This flag ensures all indirect (expression-based) array indexing
   // is clamped to the bounds of the array. This ensures, for example,
@@ -204,32 +209,32 @@ typedef enum {
   // vec234, or mat234 type. The ShArrayIndexClampingStrategy enum,
   // specified in the ShBuiltInResources when constructing the
   // compiler, selects the strategy for the clamping implementation.
-  SH_CLAMP_INDIRECT_ARRAY_BOUNDS = 0x1000,
+  SH_CLAMP_INDIRECT_ARRAY_BOUNDS = 0x2000,
 
   // This flag limits the complexity of an expression.
-  SH_LIMIT_EXPRESSION_COMPLEXITY = 0x2000,
+  SH_LIMIT_EXPRESSION_COMPLEXITY = 0x4000,
 
   // This flag limits the depth of the call stack.
-  SH_LIMIT_CALL_STACK_DEPTH = 0x4000,
+  SH_LIMIT_CALL_STACK_DEPTH = 0x8000,
 
   // This flag initializes gl_Position to vec4(0,0,0,0) at the
   // beginning of the vertex shader's main(), and has no effect in the
   // fragment shader. It is intended as a workaround for drivers which
   // incorrectly fail to link programs if gl_Position is not written.
-  SH_INIT_GL_POSITION = 0x8000,
+  SH_INIT_GL_POSITION = 0x10000,
 
   // This flag replaces
   //   "a && b" with "a ? b : false",
   //   "a || b" with "a ? true : b".
   // This is to work around a MacOSX driver bug that |b| is executed
   // independent of |a|'s value.
-  SH_UNFOLD_SHORT_CIRCUIT = 0x10000,
+  SH_UNFOLD_SHORT_CIRCUIT = 0x20000,
 
   // This flag initializes varyings without static use in vertex shader
   // at the beginning of main(), and has no effects in the fragment shader.
   // It is intended as a workaround for drivers which incorrectly optimize
   // out such varyings and cause a link failure.
-  SH_INIT_VARYINGS_WITHOUT_STATIC_USE = 0x20000,
+  SH_INIT_VARYINGS_WITHOUT_STATIC_USE = 0x40000,
 } ShCompileOptions;
 
 // Defines alternate strategies for implementing array index clamping.

src/compiler/translator/Compiler.cpp

@@ -201,7 +201,21 @@ bool TCompiler::compile(const char* const shaderStrings[],
 
         // Unroll for-loop markup needs to happen after validateLimitations pass.
         if (success && (compileOptions & SH_UNROLL_FOR_LOOP_WITH_INTEGER_INDEX))
-            ForLoopUnroll::MarkForLoopsWithIntegerIndicesForUnrolling(root);
+	{
+	    ForLoopUnrollMarker marker(ForLoopUnrollMarker::kIntegerIndex);
+            root->traverse(&marker);
+        }
+        if (success && (compileOptions & SH_UNROLL_FOR_LOOP_WITH_SAMPLER_ARRAY_INDEX))
+	{
+	    ForLoopUnrollMarker marker(ForLoopUnrollMarker::kSamplerArrayIndex);
+            root->traverse(&marker);
+            if (marker.samplerArrayIndexIsFloatLoopIndex())
+            {
+                infoSink.info.prefix(EPrefixError);
+                infoSink.info << "sampler array index is float loop index";
+                success = false;
+            }
+        }
 
         // Built-in function emulation needs to happen after validateLimitations pass.
         if (success && (compileOptions & SH_EMULATE_BUILT_IN_FUNCTIONS))

src/compiler/translator/ForLoopUnroll.cpp

@@ -6,210 +6,77 @@
 
 #include "compiler/translator/ForLoopUnroll.h"
 
-namespace {
-
-class IntegerForLoopUnrollMarker : public TIntermTraverser {
-public:
-
-    virtual bool visitLoop(Visit, TIntermLoop* node)
-    {
-        // This is called after ValidateLimitations pass, so all the ASSERT
-        // should never fail.
-        // See ValidateLimitations::validateForLoopInit().
-        ASSERT(node);
-        ASSERT(node->getType() == ELoopFor);
-        ASSERT(node->getInit());
-        TIntermAggregate* decl = node->getInit()->getAsAggregate();
-        ASSERT(decl && decl->getOp() == EOpDeclaration);
-        TIntermSequence& declSeq = decl->getSequence();
-        ASSERT(declSeq.size() == 1);
-        TIntermBinary* declInit = declSeq[0]->getAsBinaryNode();
-        ASSERT(declInit && declInit->getOp() == EOpInitialize);
-        ASSERT(declInit->getLeft());
-        TIntermSymbol* symbol = declInit->getLeft()->getAsSymbolNode();
-        ASSERT(symbol);
-        TBasicType type = symbol->getBasicType();
-        ASSERT(type == EbtInt || type == EbtFloat);
-        if (type == EbtInt)
-            node->setUnrollFlag(true);
-        return true;
-    }
-
-};
-
-}  // anonymous namepsace
-
-void ForLoopUnroll::FillLoopIndexInfo(TIntermLoop* node, TLoopIndexInfo& info)
+bool ForLoopUnrollMarker::visitBinary(Visit, TIntermBinary *node)
 {
-    ASSERT(node->getType() == ELoopFor);
-    ASSERT(node->getUnrollFlag());
-
-    TIntermNode* init = node->getInit();
-    ASSERT(init != NULL);
-    TIntermAggregate* decl = init->getAsAggregate();
-    ASSERT((decl != NULL) && (decl->getOp() == EOpDeclaration));
-    TIntermSequence& declSeq = decl->getSequence();
-    ASSERT(declSeq.size() == 1);
-    TIntermBinary* declInit = declSeq[0]->getAsBinaryNode();
-    ASSERT((declInit != NULL) && (declInit->getOp() == EOpInitialize));
-    TIntermSymbol* symbol = declInit->getLeft()->getAsSymbolNode();
-    ASSERT(symbol != NULL);
-    ASSERT(symbol->getBasicType() == EbtInt);
-
-    info.id = symbol->getId();
-
-    ASSERT(declInit->getRight() != NULL);
-    TIntermConstantUnion* initNode = declInit->getRight()->getAsConstantUnion();
-    ASSERT(initNode != NULL);
-
-    info.initValue = evaluateIntConstant(initNode);
-    info.currentValue = info.initValue;
-
-    TIntermNode* cond = node->getCondition();
-    ASSERT(cond != NULL);
-    TIntermBinary* binOp = cond->getAsBinaryNode();
-    ASSERT(binOp != NULL);
-    ASSERT(binOp->getRight() != NULL);
-    ASSERT(binOp->getRight()->getAsConstantUnion() != NULL);
-
-    info.incrementValue = getLoopIncrement(node);
-    info.stopValue = evaluateIntConstant(
-        binOp->getRight()->getAsConstantUnion());
-    info.op = binOp->getOp();
-}
-
-void ForLoopUnroll::Step()
-{
-    ASSERT(mLoopIndexStack.size() > 0);
-    TLoopIndexInfo& info = mLoopIndexStack[mLoopIndexStack.size() - 1];
-    info.currentValue += info.incrementValue;
-}
+    if (mUnrollCondition != kSamplerArrayIndex)
+        return true;
 
-bool ForLoopUnroll::SatisfiesLoopCondition()
-{
-    ASSERT(mLoopIndexStack.size() > 0);
-    TLoopIndexInfo& info = mLoopIndexStack[mLoopIndexStack.size() - 1];
-    // Relational operator is one of: > >= < <= == or !=.
-    switch (info.op) {
-      case EOpEqual:
-        return (info.currentValue == info.stopValue);
-      case EOpNotEqual:
-        return (info.currentValue != info.stopValue);
-      case EOpLessThan:
-        return (info.currentValue < info.stopValue);
-      case EOpGreaterThan:
-        return (info.currentValue > info.stopValue);
-      case EOpLessThanEqual:
-        return (info.currentValue <= info.stopValue);
-      case EOpGreaterThanEqual:
-        return (info.currentValue >= info.stopValue);
+    // If a sampler array index is also the loop index,
+    //   1) if the index type is integer, mark the loop for unrolling;
+    //   2) if the index type if float, set a flag to later fail compile.
+    switch (node->getOp())
+    {
+      case EOpIndexIndirect:
+        if (node->getLeft() != NULL && node->getRight() != NULL && node->getLeft()->getAsSymbolNode())
+        {
+            TIntermSymbol *symbol = node->getLeft()->getAsSymbolNode();
+            if (IsSampler(symbol->getBasicType()) && symbol->isArray() && !mLoopStack.empty())
+            {
+                mVisitSamplerArrayIndexNodeInsideLoop = true;
+                node->getRight()->traverse(this);
+                mVisitSamplerArrayIndexNodeInsideLoop = false;
+                // We have already visited all the children.
+                return false;
+            }
+        }
+        break;
       default:
-        UNREACHABLE();
+        break;
     }
-    return false;
+    return true;
 }
 
-bool ForLoopUnroll::NeedsToReplaceSymbolWithValue(TIntermSymbol* symbol)
+bool ForLoopUnrollMarker::visitLoop(Visit, TIntermLoop *node)
 {
-    for (TVector<TLoopIndexInfo>::iterator i = mLoopIndexStack.begin();
-         i != mLoopIndexStack.end();
-         ++i) {
-        if (i->id == symbol->getId())
-            return true;
+    if (mUnrollCondition == kIntegerIndex)
+    {
+        // Check if loop index type is integer.
+        // This is called after ValidateLimitations pass, so all the calls
+        // should be valid. See ValidateLimitations::validateForLoopInit().
+        TIntermSequence& declSeq = node->getInit()->getAsAggregate()->getSequence();
+        TIntermSymbol* symbol = declSeq[0]->getAsBinaryNode()->getLeft()->getAsSymbolNode();
+        if (symbol->getBasicType() == EbtInt)
+            node->setUnrollFlag(true);
     }
-    return false;
-}
 
-int ForLoopUnroll::GetLoopIndexValue(TIntermSymbol* symbol)
-{
-    for (TVector<TLoopIndexInfo>::iterator i = mLoopIndexStack.begin();
-         i != mLoopIndexStack.end();
-         ++i) {
-        if (i->id == symbol->getId())
-            return i->currentValue;
+    TIntermNode *body = node->getBody();
+    if (body != NULL)
+    {
+        mLoopStack.push(node);
+        body->traverse(this);
+        mLoopStack.pop();
     }
-    UNREACHABLE();
+    // The loop is fully processed - no need to visit children.
     return false;
 }
 
-void ForLoopUnroll::Push(TLoopIndexInfo& info)
-{
-    mLoopIndexStack.push_back(info);
-}
-
-void ForLoopUnroll::Pop()
+void ForLoopUnrollMarker::visitSymbol(TIntermSymbol* symbol)
 {
-    mLoopIndexStack.pop_back();
-}
-
-// static
-void ForLoopUnroll::MarkForLoopsWithIntegerIndicesForUnrolling(
-    TIntermNode* root)
-{
-    ASSERT(root);
-
-    IntegerForLoopUnrollMarker marker;
-    root->traverse(&marker);
-}
-
-int ForLoopUnroll::getLoopIncrement(TIntermLoop* node)
-{
-    TIntermNode* expr = node->getExpression();
-    ASSERT(expr != NULL);
-    // for expression has one of the following forms:
-    //     loop_index++
-    //     loop_index--
-    //     loop_index += constant_expression
-    //     loop_index -= constant_expression
-    //     ++loop_index
-    //     --loop_index
-    // The last two forms are not specified in the spec, but I am assuming
-    // its an oversight.
-    TIntermUnary* unOp = expr->getAsUnaryNode();
-    TIntermBinary* binOp = unOp ? NULL : expr->getAsBinaryNode();
-
-    TOperator op = EOpNull;
-    TIntermConstantUnion* incrementNode = NULL;
-    if (unOp != NULL) {
-        op = unOp->getOp();
-    } else if (binOp != NULL) {
-        op = binOp->getOp();
-        ASSERT(binOp->getRight() != NULL);
-        incrementNode = binOp->getRight()->getAsConstantUnion();
-        ASSERT(incrementNode != NULL);
-    }
-
-    int increment = 0;
-    // The operator is one of: ++ -- += -=.
-    switch (op) {
-        case EOpPostIncrement:
-        case EOpPreIncrement:
-            ASSERT((unOp != NULL) && (binOp == NULL));
-            increment = 1;
-            break;
-        case EOpPostDecrement:
-        case EOpPreDecrement:
-            ASSERT((unOp != NULL) && (binOp == NULL));
-            increment = -1;
-            break;
-        case EOpAddAssign:
-            ASSERT((unOp == NULL) && (binOp != NULL));
-            increment = evaluateIntConstant(incrementNode);
+    if (!mVisitSamplerArrayIndexNodeInsideLoop)
+        return;
+    TIntermLoop *loop = mLoopStack.findLoop(symbol);
+    if (loop)
+    {
+        switch (symbol->getBasicType())
+        {
+          case EbtFloat:
+            mSamplerArrayIndexIsFloatLoopIndex = true;
             break;
-        case EOpSubAssign:
-            ASSERT((unOp == NULL) && (binOp != NULL));
-            increment = - evaluateIntConstant(incrementNode);
+          case EbtInt:
+            loop->setUnrollFlag(true);
             break;
-        default:
-            ASSERT(false);
+          default:
+            UNREACHABLE();
+        }
     }
-
-    return increment;
-}
-
-int ForLoopUnroll::evaluateIntConstant(TIntermConstantUnion* node)
-{
-    ASSERT((node != NULL) && (node->getUnionArrayPointer() != NULL));
-    return node->getIConst(0);
 }
-

src/compiler/translator/ForLoopUnroll.h

@@ -7,46 +7,44 @@
 #ifndef COMPILER_FORLOOPUNROLL_H_
 #define COMPILER_FORLOOPUNROLL_H_
 
-#include "compiler/translator/intermediate.h"
-
-struct TLoopIndexInfo {
-    int id;
-    int initValue;
-    int stopValue;
-    int incrementValue;
-    TOperator op;
-    int currentValue;
-};
-
-class ForLoopUnroll {
-public:
-    ForLoopUnroll() { }
-
-    void FillLoopIndexInfo(TIntermLoop* node, TLoopIndexInfo& info);
-
-    // Update the info.currentValue for the next loop iteration.
-    void Step();
-
-    // Return false if loop condition is no longer satisfied.
-    bool SatisfiesLoopCondition();
-
-    // Check if the symbol is the index of a loop that's unrolled.
-    bool NeedsToReplaceSymbolWithValue(TIntermSymbol* symbol);
-
-    // Return the current value of a given loop index symbol.
-    int GetLoopIndexValue(TIntermSymbol* symbol);
-
-    void Push(TLoopIndexInfo& info);
-    void Pop();
-
-    static void MarkForLoopsWithIntegerIndicesForUnrolling(TIntermNode* root);
-
-private:
-    int getLoopIncrement(TIntermLoop* node);
-
-    int evaluateIntConstant(TIntermConstantUnion* node);
-
-    TVector<TLoopIndexInfo> mLoopIndexStack;
+#include "compiler/translator/LoopInfo.h"
+
+// This class detects for-loops that needs to be unrolled.
+// Currently we support two unroll conditions:
+//   1) kForLoopWithIntegerIndex: unroll if the index type is integer.
+//   2) kForLoopWithSamplerArrayIndex: unroll where a sampler array index
+//      is also the loop integer index, and reject and fail a compile
+//      where a sampler array index is also the loop float index.
+class ForLoopUnrollMarker : public TIntermTraverser
+{
+  public:
+    enum UnrollCondition
+    {
+        kIntegerIndex,
+        kSamplerArrayIndex
+    };
+
+    ForLoopUnrollMarker(UnrollCondition condition)
+        : mUnrollCondition(condition),
+          mSamplerArrayIndexIsFloatLoopIndex(false),
+          mVisitSamplerArrayIndexNodeInsideLoop(false)
+    {
+    }
+
+    virtual bool visitBinary(Visit, TIntermBinary *node);
+    virtual bool visitLoop(Visit, TIntermLoop *node);
+    virtual void visitSymbol(TIntermSymbol *node);
+
+    bool samplerArrayIndexIsFloatLoopIndex() const
+    {
+        return mSamplerArrayIndexIsFloatLoopIndex;
+    }
+
+  private:
+    UnrollCondition mUnrollCondition;
+    TLoopStack mLoopStack;
+    bool mSamplerArrayIndexIsFloatLoopIndex;
+    bool mVisitSamplerArrayIndexNodeInsideLoop;
 };
 
 #endif

src/compiler/translator/LoopInfo.cpp

+//
+// Copyright (c) 2002-2014 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.
+//
+
+#include "compiler/translator/LoopInfo.h"
+
+namespace
+{
+
+int EvaluateIntConstant(TIntermConstantUnion *node)
+{
+    ASSERT(node && node->getUnionArrayPointer());
+    return node->getIConst(0);
+}
+
+int GetLoopIntIncrement(TIntermLoop *node)
+{
+    TIntermNode *expr = node->getExpression();
+    // for expression has one of the following forms:
+    //     loop_index++
+    //     loop_index--
+    //     loop_index += constant_expression
+    //     loop_index -= constant_expression
+    //     ++loop_index
+    //     --loop_index
+    // The last two forms are not specified in the spec, but I am assuming
+    // its an oversight.
+    TIntermUnary *unOp = expr->getAsUnaryNode();
+    TIntermBinary *binOp = unOp ? NULL : expr->getAsBinaryNode();
+
+    TOperator op = EOpNull;
+    TIntermConstantUnion *incrementNode = NULL;
+    if (unOp)
+    {
+        op = unOp->getOp();
+    }
+    else if (binOp)
+    {
+        op = binOp->getOp();
+        ASSERT(binOp->getRight());
+        incrementNode = binOp->getRight()->getAsConstantUnion();
+        ASSERT(incrementNode);
+    }
+
+    int increment = 0;
+    // The operator is one of: ++ -- += -=.
+    switch (op)
+    {
+      case EOpPostIncrement:
+      case EOpPreIncrement:
+        ASSERT(unOp && !binOp);
+        increment = 1;
+        break;
+      case EOpPostDecrement:
+      case EOpPreDecrement:
+        ASSERT(unOp && !binOp);
+        increment = -1;
+        break;
+      case EOpAddAssign:
+        ASSERT(!unOp && binOp);
+        increment = EvaluateIntConstant(incrementNode);
+        break;
+      case EOpSubAssign:
+        ASSERT(!unOp && binOp);
+        increment = - EvaluateIntConstant(incrementNode);
+        break;
+      default:
+        UNREACHABLE();
+    }
+
+    return increment;
+}
+
+}  // namespace anonymous
+
+TLoopIndexInfo::TLoopIndexInfo()
+    : mId(-1),
+      mType(EbtVoid),
+      mInitValue(0),
+      mStopValue(0),
+      mIncrementValue(0),
+      mOp(EOpNull),
+      mCurrentValue(0)
+{
+}
+
+void TLoopIndexInfo::fillInfo(TIntermLoop *node)
+{
+    if (node == NULL)
+        return;
+
+    // Here we assume all the operations are valid, because the loop node is
+    // already validated in ValidateLimitations.
+    TIntermSequence &declSeq =
+        node->getInit()->getAsAggregate()->getSequence();
+    TIntermBinary *declInit = declSeq[0]->getAsBinaryNode();
+    TIntermSymbol *symbol = declInit->getLeft()->getAsSymbolNode();
+
+    mId = symbol->getId();
+    mType = symbol->getBasicType();
+
+    if (mType == EbtInt)
+    {
+        TIntermConstantUnion* initNode = declInit->getRight()->getAsConstantUnion();
+        mInitValue = EvaluateIntConstant(initNode);
+        mCurrentValue = mInitValue;
+        mIncrementValue = GetLoopIntIncrement(node);
+
+        TIntermBinary* binOp = node->getCondition()->getAsBinaryNode();
+        mStopValue = EvaluateIntConstant(
+            binOp->getRight()->getAsConstantUnion());
+        mOp = binOp->getOp();
+    }
+}
+
+bool TLoopIndexInfo::satisfiesLoopCondition() const
+{
+    // Relational operator is one of: > >= < <= == or !=.
+    switch (mOp)
+    {
+      case EOpEqual:
+        return (mCurrentValue == mStopValue);
+      case EOpNotEqual:
+        return (mCurrentValue != mStopValue);
+      case EOpLessThan:
+        return (mCurrentValue < mStopValue);
+      case EOpGreaterThan:
+        return (mCurrentValue > mStopValue);
+      case EOpLessThanEqual:
+        return (mCurrentValue <= mStopValue);
+      case EOpGreaterThanEqual:
+        return (mCurrentValue >= mStopValue);
+      default:
+        UNREACHABLE();
+        return false;
+    }
+}
+
+TLoopInfo::TLoopInfo()
+    : loop(NULL)
+{
+}
+
+TLoopInfo::TLoopInfo(TIntermLoop *node)
+    : loop(node)
+{
+    index.fillInfo(node);
+}
+
+TIntermLoop *TLoopStack::findLoop(TIntermSymbol *symbol)
+{
+    if (!symbol)
+        return NULL;
+    for (iterator iter = begin(); iter != end(); ++iter)
+    {
+        if (iter->index.getId() == symbol->getId())
+            return iter->loop;
+    }
+    return NULL;
+}
+
+TLoopIndexInfo *TLoopStack::getIndexInfo(TIntermSymbol *symbol)
+{
+    if (!symbol)
+        return NULL;
+    for (iterator iter = begin(); iter != end(); ++iter)
+    {
+        if (iter->index.getId() == symbol->getId())
+            return &(iter->index);
+    }
+    return NULL;
+}
+
+void TLoopStack::step()
+{
+    ASSERT(!empty());
+    rbegin()->index.step();
+}
+
+bool TLoopStack::satisfiesLoopCondition()
+{
+    ASSERT(!empty());
+    return rbegin()->index.satisfiesLoopCondition();
+}
+
+bool TLoopStack::needsToReplaceSymbolWithValue(TIntermSymbol *symbol)
+{
+    TIntermLoop *loop = findLoop(symbol);
+    return loop && loop->getUnrollFlag();
+}
+
+int TLoopStack::getLoopIndexValue(TIntermSymbol *symbol)
+{
+    TLoopIndexInfo *info = getIndexInfo(symbol);
+    ASSERT(info);
+    return info->getCurrentValue();
+}
+
+void TLoopStack::push(TIntermLoop *loop)
+{
+    TLoopInfo info(loop);
+    push_back(info);
+}
+
+void TLoopStack::pop()
+{
+    pop_back();
+}
+

src/compiler/translator/LoopInfo.h

+//
+// Copyright (c) 2002-2014 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.
+//
+
+#ifndef COMPILER_TRANSLATOR_LOOP_INFO_H_
+#define COMPILER_TRANSLATOR_LOOP_INFO_H_
+
+#include "compiler/translator/intermediate.h"
+
+class TLoopIndexInfo
+{
+  public:
+    TLoopIndexInfo();
+
+    // If type is EbtInt, fill all fields of the structure with info
+    // extracted from a loop node.
+    // If type is not EbtInt, only fill id and type.
+    void fillInfo(TIntermLoop *node);
+
+    int getId() const { return mId; }
+    void setId(int id) { mId = id; }
+    TBasicType getType() const { return mType; }
+    void setType(TBasicType type) { mType = type; }
+    int getCurrentValue() const { return mCurrentValue; }
+
+    void step() { mCurrentValue += mIncrementValue; }
+
+    // Check if the current value satisfies the loop condition.
+    bool satisfiesLoopCondition() const;
+
+  private:
+    int mId;
+    TBasicType mType;  // Either EbtInt or EbtFloat
+
+    // Below fields are only valid if the index's type is int.
+    int mInitValue;
+    int mStopValue;
+    int mIncrementValue;
+    TOperator mOp;
+    int mCurrentValue;
+};
+
+struct TLoopInfo
+{
+    TLoopIndexInfo index;
+    TIntermLoop *loop;
+
+    TLoopInfo();
+    TLoopInfo(TIntermLoop *node);
+};
+
+class TLoopStack : public TVector<TLoopInfo>
+{
+  public:
+    // Search loop stack for a loop whose index matches the input symbol.
+    TIntermLoop *findLoop(TIntermSymbol *symbol);
+
+    // Find the loop index info in the loop stack by the input symbol.
+    TLoopIndexInfo *getIndexInfo(TIntermSymbol *symbol);
+
+    // Update the currentValue for the next loop iteration.
+    void step();
+
+    // Return false if loop condition is no longer satisfied.
+    bool satisfiesLoopCondition();
+
+    // Check if the symbol is the index of a loop that's unrolled.
+    bool needsToReplaceSymbolWithValue(TIntermSymbol *symbol);
+
+    // Return the current value of a given loop index symbol.
+    int getLoopIndexValue(TIntermSymbol *symbol);
+
+    void push(TIntermLoop *info);
+    void pop();
+};
+
+#endif // COMPILER_TRANSLATOR_LOOP_INDEX_H_
+

src/compiler/translator/OutputGLSLBase.cpp

@@ -161,8 +161,8 @@ const ConstantUnion* TOutputGLSLBase::writeConstantUnion(const TType& type,
 void TOutputGLSLBase::visitSymbol(TIntermSymbol* node)
 {
     TInfoSinkBase& out = objSink();
-    if (mLoopUnroll.NeedsToReplaceSymbolWithValue(node))
-        out << mLoopUnroll.GetLoopIndexValue(node);
+    if (mLoopUnrollStack.needsToReplaceSymbolWithValue(node))
+        out << mLoopUnrollStack.getLoopIndexValue(node);
     else
         out << hashVariableName(node->getSymbol());
 
@@ -645,7 +645,8 @@ bool TOutputGLSLBase::visitLoop(Visit visit, TIntermLoop* node)
     TLoopType loopType = node->getType();
     if (loopType == ELoopFor)  // for loop
     {
-        if (!node->getUnrollFlag()) {
+        if (!node->getUnrollFlag())
+        {
             out << "for (";
             if (node->getInit())
                 node->getInit()->traverse(this);
@@ -659,6 +660,18 @@ bool TOutputGLSLBase::visitLoop(Visit visit, TIntermLoop* node)
                 node->getExpression()->traverse(this);
             out << ")\n";
         }
+        else
+        {
+            // Need to put a one-iteration loop here to handle break.
+            TIntermSequence &declSeq =
+                node->getInit()->getAsAggregate()->getSequence();
+            TIntermSymbol *indexSymbol =
+                declSeq[0]->getAsBinaryNode()->getLeft()->getAsSymbolNode();
+            TString name = hashVariableName(indexSymbol->getSymbol());
+            out << "for (int " << name << " = 0; "
+                << name << " < 1; "
+                << "++" << name << ")\n";
+        }
     }
     else if (loopType == ELoopWhile)  // while loop
     {
@@ -676,15 +689,15 @@ bool TOutputGLSLBase::visitLoop(Visit visit, TIntermLoop* node)
     // Loop body.
     if (node->getUnrollFlag())
     {
-        TLoopIndexInfo indexInfo;
-        mLoopUnroll.FillLoopIndexInfo(node, indexInfo);
-        mLoopUnroll.Push(indexInfo);
-        while (mLoopUnroll.SatisfiesLoopCondition())
+        out << "{\n";
+        mLoopUnrollStack.push(node);
+        while (mLoopUnrollStack.satisfiesLoopCondition())
         {
             visitCodeBlock(node->getBody());
-            mLoopUnroll.Step();
+            mLoopUnrollStack.step();
         }
-        mLoopUnroll.Pop();
+        mLoopUnrollStack.pop();
+        out << "}\n";
     }
     else
     {

src/compiler/translator/OutputGLSLBase.h

@@ -9,8 +9,8 @@
 
 #include <set>
 
-#include "compiler/translator/ForLoopUnroll.h"
 #include "compiler/translator/intermediate.h"
+#include "compiler/translator/LoopInfo.h"
 #include "compiler/translator/ParseContext.h"
 
 class TOutputGLSLBase : public TIntermTraverser
@@ -65,7 +65,8 @@ private:
     typedef std::set<TString> DeclaredStructs;
     DeclaredStructs mDeclaredStructs;
 
-    ForLoopUnroll mLoopUnroll;
+    // Stack of loops that need to be unrolled.
+    TLoopStack mLoopUnrollStack;
 
     ShArrayIndexClampingStrategy mClampingStrategy;
 

src/compiler/translator/ValidateLimitations.h

@@ -6,53 +6,50 @@
 
 #include "GLSLANG/ShaderLang.h"
 #include "compiler/translator/intermediate.h"
+#include "compiler/translator/LoopInfo.h"
 
 class TInfoSinkBase;
 
-struct TLoopInfo {
-    struct TIndex {
-        int id;  // symbol id.
-    } index;
-    TIntermLoop* loop;
-};
-typedef TVector<TLoopInfo> TLoopStack;
-
 // Traverses intermediate tree to ensure that the shader does not exceed the
 // minimum functionality mandated in GLSL 1.0 spec, Appendix A.
-class ValidateLimitations : public TIntermTraverser {
-public:
-    ValidateLimitations(ShShaderType shaderType, TInfoSinkBase& sink);
+class ValidateLimitations : public TIntermTraverser
+{
+  public:
+    ValidateLimitations(ShShaderType shaderType, TInfoSinkBase &sink);
 
     int numErrors() const { return mNumErrors; }
 
-    virtual bool visitBinary(Visit, TIntermBinary*);
-    virtual bool visitUnary(Visit, TIntermUnary*);
-    virtual bool visitAggregate(Visit, TIntermAggregate*);
-    virtual bool visitLoop(Visit, TIntermLoop*);
+    virtual bool visitBinary(Visit, TIntermBinary *);
+    virtual bool visitUnary(Visit, TIntermUnary *);
+    virtual bool visitAggregate(Visit, TIntermAggregate *);
+    virtual bool visitLoop(Visit, TIntermLoop *);
 
-private:
-    void error(TSourceLoc loc, const char *reason, const char* token);
+  private:
+    void error(TSourceLoc loc, const char *reason, const char *token);
 
     bool withinLoopBody() const;
-    bool isLoopIndex(const TIntermSymbol* symbol) const;
-    bool validateLoopType(TIntermLoop* node);
-    bool validateForLoopHeader(TIntermLoop* node, TLoopInfo* info);
-    bool validateForLoopInit(TIntermLoop* node, TLoopInfo* info);
-    bool validateForLoopCond(TIntermLoop* node, TLoopInfo* info);
-    bool validateForLoopExpr(TIntermLoop* node, TLoopInfo* info);
+    bool isLoopIndex(TIntermSymbol *symbol);
+    bool validateLoopType(TIntermLoop *node);
+
+    bool validateForLoopHeader(TIntermLoop *node);
+    // If valid, return the index symbol id; Otherwise, return -1.
+    int validateForLoopInit(TIntermLoop *node);
+    bool validateForLoopCond(TIntermLoop *node, int indexSymbolId);
+    bool validateForLoopExpr(TIntermLoop *node, int indexSymbolId);
+
     // Returns true if none of the loop indices is used as the argument to
     // the given function out or inout parameter.
-    bool validateFunctionCall(TIntermAggregate* node);
-    bool validateOperation(TIntermOperator* node, TIntermNode* operand);
+    bool validateFunctionCall(TIntermAggregate *node);
+    bool validateOperation(TIntermOperator *node, TIntermNode *operand);
 
     // Returns true if indexing does not exceed the minimum functionality
     // mandated in GLSL 1.0 spec, Appendix A, Section 5.
-    bool isConstExpr(TIntermNode* node);
-    bool isConstIndexExpr(TIntermNode* node);
-    bool validateIndexing(TIntermBinary* node);
+    bool isConstExpr(TIntermNode *node);
+    bool isConstIndexExpr(TIntermNode *node);
+    bool validateIndexing(TIntermBinary *node);
 
     ShShaderType mShaderType;
-    TInfoSinkBase& mSink;
+    TInfoSinkBase &mSink;
     int mNumErrors;
     TLoopStack mLoopStack;
 };