Track what ins/outs/uniforms are used, so that errors like "declare after use" or "can't use both XXX and YYY" can be issued. So far, used this for invariant, gl_FragColor et. al., and gl_FragCoord use before redeclaration.

Also made all tests in testlist include linker tests.


git-svn-id: https://cvs.khronos.org/svn/repos/ogl/trunk/ecosystem/public/sdk/tools/glslang@24156 e7fa87d3-cd2b-0410-9028-fcbf551c1848

glslang/Include/Types.h

@@ -312,6 +312,29 @@ public:
         }
     }
 
+    bool isIo() const
+    {
+        switch (storage) {
+        case EvqUniform:
+        case EvqBuffer:
+        case EvqVaryingIn:
+        case EvqFragCoord:
+        case EvqPointCoord:
+        case EvqFace:
+        case EvqVertexId:
+        case EvqInstanceId:
+        case EvqPosition:
+        case EvqPointSize:
+        case EvqClipVertex:
+        case EvqVaryingOut:
+        case EvqFragColor:
+        case EvqFragDepth:
+            return true;
+        default:
+            return false;
+        }
+    }
+
     // Implementing an embedded layout-qualifier class here, since C++ can't have a real class bitfield
     void clearLayout()
     {

glslang/MachineIndependent/ParseHelper.cpp

@@ -54,7 +54,7 @@ TParseContext::TParseContext(TSymbolTable& symt, TIntermediate& interm, bool pb,
             contextPragma(true, false), loopNestingLevel(0), structNestingLevel(0),
             tokensBeforeEOF(false), currentScanner(0),
             numErrors(0), parsingBuiltins(pb), afterEOF(false),
-            anyIndexLimits(false), fragCoordUsedBeforeRedeclaration(false)
+            anyIndexLimits(false)
 {
     // ensure we always have a linkage node, even if empty, to simplify tree topology algorithms
     linkage = new TIntermAggregate;
@@ -387,6 +387,7 @@ void C_DECL TParseContext::warn(TSourceLoc loc, const char *szReason, const char
 TIntermTyped* TParseContext::handleVariable(TSourceLoc loc, TSymbol* symbol, TString* string)
 {
     TIntermTyped* node = 0;
+    bool noteAccess = false;
 
     // Error check for function requiring specific extensions present.
     if (symbol && symbol->getNumExtensions())
@@ -403,6 +404,10 @@ TIntermTyped* TParseContext::handleVariable(TSourceLoc loc, TSymbol* symbol, TSt
 
         node = intermediate.addIndex(EOpIndexDirectStruct, container, constNode, loc);
         node->setType(*(*variable->getType().getStruct())[anon->getMemberNumber()].type);
+        if (variable->getType().getQualifier().isIo())
+            noteAccess = true;
+
+        // TODO: does this create any accidental type sharing with the built-in level?
     } else {
         // The symbol table search was done in the lexical phase, but
         // if this is a new symbol, it wouldn't have found it.
@@ -419,18 +424,19 @@ TIntermTyped* TParseContext::handleVariable(TSourceLoc loc, TSymbol* symbol, TSt
             TType* type;
             if (variable->isReadOnly()) {
                 type = new TType;
-                // break type sharing with built-ins
+                // break type sharing with built-ins; only costs if there are arrays or structures
                 type->deepCopy(variable->getType());
-
-                // track use of unredeclared gl_FragCoord
-                if (variable->getName() == "gl_FragCoord")
-                    fragCoordUsedBeforeRedeclaration = true;
             } else
                 type = &variable->getWritableType();
             node = intermediate.addSymbol(variable->getUniqueId(), variable->getName(), *type, loc);
+            if (type->getQualifier().isIo())
+                noteAccess = true;
         }
     }
 
+    if (noteAccess)
+        intermediate.addIoAccessed(*string);
+
     return node;
 }
 
@@ -2069,10 +2075,8 @@ TSymbol* TParseContext::redeclareBuiltinVariable(TSourceLoc loc, const TString&
                 symbolQualifier.storage != qualifier.storage)
                 error(loc, "cannot change qualification of", "redeclaration", symbol->getName().c_str());
         } else if (identifier == "gl_FragCoord") {
-            if (fragCoordUsedBeforeRedeclaration)
+            if (intermediate.inIoAccessed("gl_FragCoord"))
                 error(loc, "cannot redeclare after use", "gl_FragCoord", "");
-                // Note: this did not catch the case of 1) declare, 2) use, 3) declare again, because the "use" was of a redeclaration, and so didn't set fragCoordUsedBeforeRedeclaration.
-                // (and that's what the rules are too, as long as #3 matches #1)
             if (qualifier.nopersp != symbolQualifier.nopersp || qualifier.flat != symbolQualifier.flat ||
                 qualifier.isMemory() || qualifier.isAuxiliary())
                 error(loc, "can only change layout qualification of", "redeclaration", symbol->getName().c_str());
@@ -3425,6 +3429,8 @@ void TParseContext::addQualifierToExisting(TSourceLoc loc, TQualifier qualifier,
         symbol = symbolTable.copyUp(symbol);
 
     if (qualifier.invariant) {
+        if (intermediate.inIoAccessed(identifier))
+            error(loc, "cannot change qualification after use", "invariant", "");
         symbol->getWritableType().getQualifier().invariant = true;
         invariantCheck(loc, symbol->getType(), identifier);
     }

glslang/MachineIndependent/ParseHelper.h

@@ -251,7 +251,6 @@ protected:
     TIdSetType inductiveLoopIds;
     bool anyIndexLimits;
     TVector<TIntermTyped*> needsIndexLimitationChecking;
-    bool fragCoordUsedBeforeRedeclaration;
 
     //
     // Geometry shader input arrays:

glslang/MachineIndependent/linkValidate.cpp

@@ -96,6 +96,8 @@ void TIntermediate::merge(TInfoSink& infoSink, TIntermediate& unit)
 
     mergeBodies(infoSink, globals, unitGlobals);
     mergeLinkerObjects(infoSink, linkerObjects, unitLinkerObjects);
+
+    ioAccessed.insert(unit.ioAccessed.begin(), unit.ioAccessed.end());
 }
 
 //
@@ -249,6 +251,14 @@ void TIntermediate::errorCheck(TInfoSink& infoSink)
 
     // overlap/alias/missing I/O, etc.
     inOutLocationCheck(infoSink);
+
+    if (inIoAccessed("gl_ClipDistance") && inIoAccessed("gl_ClipVertex"))
+        error(infoSink, "Can only use one of gl_ClipDistance or gl_ClipVertex (gl_ClipDistance is preferred)");
+
+    if (userOutputUsed() && (inIoAccessed("gl_FragColor") || inIoAccessed("gl_FragData")))
+        error(infoSink, "Cannot use gl_FragColor or gl_FragData when using user-defined outputs");
+    if (inIoAccessed("gl_FragColor") && inIoAccessed("gl_FragData"))
+        error(infoSink, "Cannot use both gl_FragColor and gl_FragData");
 }
 
 //
@@ -365,7 +375,7 @@ void TIntermediate::inOutLocationCheck(TInfoSink& infoSink)
     }        
 }
 
-TIntermSequence& TIntermediate::findLinkerObjects()
+TIntermSequence& TIntermediate::findLinkerObjects() const
 {
     // Get the top-level globals
     TIntermSequence& globals = treeRoot->getAsAggregate()->getSequence();
@@ -376,4 +386,25 @@ TIntermSequence& TIntermediate::findLinkerObjects()
     return globals.back()->getAsAggregate()->getSequence();
 }
 
+// See if a variable was both a user-declared output and used.
+// Note: the spec discusses writing to one, but this looks at read or write, which 
+// is more useful, and perhaps the spec should be changed to reflect that.
+bool TIntermediate::userOutputUsed() const
+{
+    const TIntermSequence& linkerObjects = findLinkerObjects();
+
+    bool found = false;
+    for (size_t i = 0; i < linkerObjects.size(); ++i) {
+        const TIntermSymbol& symbolNode = *linkerObjects[i]->getAsSymbolNode();
+        if (symbolNode.getQualifier().storage == EvqVaryingOut &&
+            symbolNode.getName().compare(0, 3, "gl_") != 0 &&
+            inIoAccessed(symbolNode.getName())) {            
+            found = true;
+            break;
+        }
+    }
+
+    return found;
+}
+
 } // end namespace glslang

glslang/MachineIndependent/localintermediate.h

@@ -40,6 +40,7 @@
 #include "Versions.h"
 
 #include <algorithm>
+#include <set>
 
 class TInfoSink;
 
@@ -143,6 +144,9 @@ public:
     void merge(TInfoSink&, TIntermediate&);
     void errorCheck(TInfoSink&);
 
+    void addIoAccessed(const TString& name) { ioAccessed.insert(name); }
+    bool inIoAccessed(const TString& name) const { return ioAccessed.find(name) != ioAccessed.end(); }
+
     void outputTree(TInfoSink&);
 	void removeTree();
 
@@ -153,7 +157,8 @@ protected:
     void mergeErrorCheck(TInfoSink&, const TIntermSymbol&, const TIntermSymbol&, bool crossStage);
     void checkCallGraphCycles(TInfoSink&);
     void inOutLocationCheck(TInfoSink&);
-    TIntermSequence& findLinkerObjects();
+    TIntermSequence& findLinkerObjects() const;
+    bool userOutputUsed() const;
 
 protected:
     const EShLanguage language;
@@ -179,9 +184,11 @@ protected:
         bool currentPath;
         bool errorGiven;
     };
-    typedef std::list<TCall> TGraph;
+    typedef TList<TCall> TGraph;
     TGraph callGraph;
 
+    std::set<TString> ioAccessed;  // set of names of statically read/written I/O that might need extra checking
+
 private:
     void operator=(TIntermediate&); // prevent assignments
 };