Revert "Merge pull request #456 from steve-lunarg/remapper-literal64"

This reverts commit ad08b30f696267ec6243dd0ad68a3db889cd2d21, reversing
changes made to 28660bb580d4c61e16ef75008550a87b3be90b91.

This backs out the pull request
https://github.com/KhronosGroup/glslang/pull/456 because it introduced
several internal errors even on code that only uses 32-bit numeric
types.

diff --git a/SPIRV/SPVRemapper.cpp b/SPIRV/SPVRemapper.cpp
index 6de729c..05f220a 100755 (executable)
--- a/SPIRV/SPVRemapper.cpp
+++ b/SPIRV/SPVRemapper.cpp
@@ -127,37 +127,6 @@ namespace spv {
         }
     }
 
-    // Return the size of a type in 32-bit words.  This currently only
-    // handles ints and floats, and is only invoked by queries which must be
-    // integer types.  If ever needed, it can be generalized.
-    unsigned spirvbin_t::typeSizeInWords(spv::Id id) const
-    {
-        const unsigned typeStart = idPos(id);
-        const spv::Op  opCode    = asOpCode(typeStart);
-
-        switch (opCode) {
-        case spv::OpTypeInt:   // fall through...
-        case spv::OpTypeFloat: return (spv[typeStart+2]+31)/32;
-        default:
-            error("unimplemented type size request");
-            return 0;
-        }
-    }
-
-    // Looks up the type of a given const or variable ID, and
-    // returns its size in 32-bit words.
-    unsigned spirvbin_t::idTypeSizeInWords(spv::Id id) const
-    {
-        const unsigned idStart = idPos(id);
-        const spv::Op  opCode  = asOpCode(idStart);
-
-        if (spv::InstructionDesc[opCode].hasType())
-            return typeSizeInWords(asId(idStart+1));
-
-        error("asked for type of typeless ID");
-        return 0;
-    }
-
     // Is this an opcode we should remove when using --strip?
     bool spirvbin_t::isStripOp(spv::Op opCode) const
     {
@@ -171,7 +140,6 @@ namespace spv {
         }
     }
 
-    // Return true if this opcode is flow control
     bool spirvbin_t::isFlowCtrl(spv::Op opCode) const
     {
         switch (opCode) {
@@ -187,7 +155,6 @@ namespace spv {
         }
     }
 
-    // Return true if this opcode defines a type
     bool spirvbin_t::isTypeOp(spv::Op opCode) const
     {
         switch (opCode) {
@@ -215,7 +182,6 @@ namespace spv {
         }
     }
 
-    // Return true if this opcode defines a constant
     bool spirvbin_t::isConstOp(spv::Op opCode) const
     {
         switch (opCode) {
@@ -358,7 +324,7 @@ namespace spv {
         fnPosDCE.clear();
         fnCalls.clear();
         typeConstPos.clear();
-        idPosR.clear();
+        typeConstPosR.clear();
         entryPoint = spv::NoResult;
         largestNewId = 0;
 
@@ -374,14 +340,6 @@ namespace spv {
                 if ((options & STRIP) && isStripOp(opCode))
                     stripInst(start);
 
-                unsigned word = start+1;
-
-                if (spv::InstructionDesc[opCode].hasType())
-                    word++;
-
-                if (spv::InstructionDesc[opCode].hasResult())
-                    idPosR[asId(word++)] = start;
-
                 if (opCode == spv::Op::OpName) {
                     const spv::Id    target = asId(start+1);
                     const std::string  name = literalString(start+2);
@@ -405,9 +363,11 @@ namespace spv {
                 } else if (isConstOp(opCode)) {
                     assert(asId(start + 2) != spv::NoResult);
                     typeConstPos.insert(start);
+                    typeConstPosR[asId(start + 2)] = start;
                 } else if (isTypeOp(opCode)) {
                     assert(asId(start + 1) != spv::NoResult);
                     typeConstPos.insert(start);
+                    typeConstPosR[asId(start + 1)] = start;
                 }
 
                 return false;
@@ -499,22 +459,13 @@ namespace spv {
                 // word += numOperands;
                 return nextInst;
 
-            case spv::OperandVariableLiteralId: {
-                if (opCode == OpSwitch) {
-                    // word-2 is the position of the selector ID.  Literals match its type.
-                    const unsigned literalSize = idTypeSizeInWords(asId(word-2));
-                    unsigned numLiteralIdPairs = (nextInst-word) / (1+literalSize);
-
-                    for (unsigned arg=0; arg<numLiteralIdPairs; ++arg) {
-                        word += literalSize;  // literal
-                        idFn(asId(word++));   // label
-                    }
-                } else {
-                    assert(0); // currentely, only OpSwitch uses OperandVariableLiteralId
+            case spv::OperandVariableLiteralId:
+                while (numOperands > 0) {
+                    ++word;             // immediate
+                    idFn(asId(word++)); // ID
+                    numOperands -= 2;
                 }
-
                 return nextInst;
-            }
 
             case spv::OperandLiteralString: {
                 const int stringWordCount = literalStringWords(literalString(word));
@@ -1015,27 +966,23 @@ namespace spv {
 
         std::unordered_map<spv::Id, int> typeUseCount;
 
-        // This is not the most efficient algorithm, but this is an offline tool, and
-        // it's easy to write this way.  Can be improved opportunistically if needed.
-        bool changed = true;
-        while (changed) {
-            changed = false;
-            strip();
-            typeUseCount.clear();
+        // Count total type usage
+        process(inst_fn_nop,
+            [&](spv::Id& id) { if (isType[id]) ++typeUseCount[id]; }
+        );
 
-            // Count total type usage
+        // Remove types from deleted code
+        for (const auto& fn : fnPosDCE)
             process(inst_fn_nop,
-                    [&](spv::Id& id) { if (isType[id]) ++typeUseCount[id]; }
-                    );
-
-            // Remove single reference types
-            for (const auto typeStart : typeConstPos) {
-                const spv::Id typeId = asTypeConstId(typeStart);
-                if (typeUseCount[typeId] == 1) {
-                    changed = true;
-                    --typeUseCount[typeId];
-                    stripInst(typeStart);
-                }
+            [&](spv::Id& id) { if (isType[id]) --typeUseCount[id]; },
+            fn.second.first, fn.second.second);
+
+        // Remove single reference types
+        for (const auto typeStart : typeConstPos) {
+            const spv::Id typeId = asTypeConstId(typeStart);
+            if (typeUseCount[typeId] == 1) {
+                --typeUseCount[typeId];
+                stripInst(typeStart);
             }
         }
     }
@@ -1113,12 +1060,12 @@ namespace spv {
     }
 #endif // NOTDEF
 
-    // Return start position in SPV of given Id.  error if not found.
-    unsigned spirvbin_t::idPos(spv::Id id) const
+    // Return start position in SPV of given type.  error if not found.
+    unsigned spirvbin_t::typePos(spv::Id id) const
     {
-        const auto tid_it = idPosR.find(id);
-        if (tid_it == idPosR.end())
-            error("ID not found");
+        const auto tid_it = typeConstPosR.find(id);
+        if (tid_it == typeConstPosR.end())
+            error("type ID not found");
 
         return tid_it->second;
     }
@@ -1136,11 +1083,11 @@ namespace spv {
         case spv::OpTypeInt:          return 3 + (spv[typeStart+3]);
         case spv::OpTypeFloat:        return 5;
         case spv::OpTypeVector:
-            return 6 + hashType(idPos(spv[typeStart+2])) * (spv[typeStart+3] - 1);
+            return 6 + hashType(typePos(spv[typeStart+2])) * (spv[typeStart+3] - 1);
         case spv::OpTypeMatrix:
-            return 30 + hashType(idPos(spv[typeStart+2])) * (spv[typeStart+3] - 1);
+            return 30 + hashType(typePos(spv[typeStart+2])) * (spv[typeStart+3] - 1);
         case spv::OpTypeImage:
-            return 120 + hashType(idPos(spv[typeStart+2])) +
+            return 120 + hashType(typePos(spv[typeStart+2])) +
                 spv[typeStart+3] +            // dimensionality
                 spv[typeStart+4] * 8 * 16 +   // depth
                 spv[typeStart+5] * 4 * 16 +   // arrayed
@@ -1151,24 +1098,24 @@ namespace spv {
         case spv::OpTypeSampledImage:
             return 502;
         case spv::OpTypeArray:
-            return 501 + hashType(idPos(spv[typeStart+2])) * spv[typeStart+3];
+            return 501 + hashType(typePos(spv[typeStart+2])) * spv[typeStart+3];
         case spv::OpTypeRuntimeArray:
-            return 5000  + hashType(idPos(spv[typeStart+2]));
+            return 5000  + hashType(typePos(spv[typeStart+2]));
         case spv::OpTypeStruct:
             {
                 std::uint32_t hash = 10000;
                 for (unsigned w=2; w < wordCount; ++w)
-                    hash += w * hashType(idPos(spv[typeStart+w]));
+                    hash += w * hashType(typePos(spv[typeStart+w]));
                 return hash;
             }
 
         case spv::OpTypeOpaque:         return 6000 + spv[typeStart+2];
-        case spv::OpTypePointer:        return 100000  + hashType(idPos(spv[typeStart+3]));
+        case spv::OpTypePointer:        return 100000  + hashType(typePos(spv[typeStart+3]));
         case spv::OpTypeFunction:
             {
                 std::uint32_t hash = 200000;
                 for (unsigned w=2; w < wordCount; ++w)
-                    hash += w * hashType(idPos(spv[typeStart+w]));
+                    hash += w * hashType(typePos(spv[typeStart+w]));
                 return hash;
             }
 
@@ -1185,14 +1132,14 @@ namespace spv {
         case spv::OpConstantFalse:       return 300008;
         case spv::OpConstantComposite:
             {
-                std::uint32_t hash = 300011 + hashType(idPos(spv[typeStart+1]));
+                std::uint32_t hash = 300011 + hashType(typePos(spv[typeStart+1]));
                 for (unsigned w=3; w < wordCount; ++w)
-                    hash += w * hashType(idPos(spv[typeStart+w]));
+                    hash += w * hashType(typePos(spv[typeStart+w]));
                 return hash;
             }
         case spv::OpConstant:
             {
-                std::uint32_t hash = 400011 + hashType(idPos(spv[typeStart+1]));
+                std::uint32_t hash = 400011 + hashType(typePos(spv[typeStart+1]));
                 for (unsigned w=3; w < wordCount; ++w)
                     hash += w * spv[typeStart+w];
                 return hash;
@@ -1265,19 +1212,19 @@ namespace spv {
         msg(3, 4, std::string("ID bound: ") + std::to_string(bound()));
 
         strip();        // strip out data we decided to eliminate
+
         if (options & OPT_LOADSTORE) optLoadStore();
         if (options & OPT_FWD_LS)    forwardLoadStores();
         if (options & DCE_FUNCS)     dceFuncs();
         if (options & DCE_VARS)      dceVars();
         if (options & DCE_TYPES)     dceTypes();
-        strip();        // strip out data we decided to eliminate
-
         if (options & MAP_TYPES)     mapTypeConst();
         if (options & MAP_NAMES)     mapNames();
         if (options & MAP_FUNCS)     mapFnBodies();
 
         mapRemainder(); // map any unmapped IDs
         applyMap();     // Now remap each shader to the new IDs we've come up with
+        strip();        // strip out data we decided to eliminate
     }
 
     // remap from a memory image

diff --git a/SPIRV/SPVRemapper.h b/SPIRV/SPVRemapper.h
index 2454a68..c2dc529 100755 (executable)
--- a/SPIRV/SPVRemapper.h
+++ b/SPIRV/SPVRemapper.h
@@ -162,15 +162,13 @@ private:
    // handle error
    void error(const std::string& txt) const { errorHandler(txt); }
 
-   bool     isConstOp(spv::Op opCode)      const;
-   bool     isTypeOp(spv::Op opCode)       const;
-   bool     isStripOp(spv::Op opCode)      const;
-   bool     isFlowCtrl(spv::Op opCode)     const;
-   range_t  literalRange(spv::Op opCode)   const;
-   range_t  typeRange(spv::Op opCode)      const;
-   range_t  constRange(spv::Op opCode)     const;
-   unsigned typeSizeInWords(spv::Id id)    const;
-   unsigned idTypeSizeInWords(spv::Id id)  const;
+   bool    isConstOp(spv::Op opCode)       const;
+   bool    isTypeOp(spv::Op opCode)        const;
+   bool    isStripOp(spv::Op opCode)       const;
+   bool    isFlowCtrl(spv::Op opCode)      const;
+   range_t literalRange(spv::Op opCode)    const;
+   range_t typeRange(spv::Op opCode)       const;
+   range_t constRange(spv::Op opCode)      const;
    
    spv::Id&        asId(unsigned word)                { return spv[word]; }
    const spv::Id&  asId(unsigned word)          const { return spv[word]; }
@@ -179,10 +177,10 @@ private:
    spv::Decoration asDecoration(unsigned word)  const { return spv::Decoration(spv[word]); }
    unsigned        asWordCount(unsigned word)   const { return opWordCount(spv[word]); }
    spv::Id         asTypeConstId(unsigned word) const { return asId(word + (isTypeOp(asOpCode(word)) ? 1 : 2)); }
-   unsigned        idPos(spv::Id id)            const;
+   unsigned        typePos(spv::Id id)          const;
 
-   static unsigned opWordCount(spirword_t data) { return data >> spv::WordCountShift; }
-   static spv::Op  opOpCode(spirword_t data)    { return spv::Op(data & spv::OpCodeMask); }
+   static unsigned    opWordCount(spirword_t data) { return data >> spv::WordCountShift; }
+   static spv::Op     opOpCode(spirword_t data)    { return spv::Op(data & spv::OpCodeMask); }
 
    // Header access & set methods
    spirword_t  magic()    const       { return spv[0]; } // return magic number
@@ -265,8 +263,8 @@ private:
    // Which functions are called, anywhere in the module, with a call count
    std::unordered_map<spv::Id, int> fnCalls;
    
-   posmap_t     typeConstPos;  // word positions that define types & consts (ordered)
-   posmap_rev_t idPosR;        // reverse map from IDs to positions
+   posmap_t     typeConstPos;   // word positions that define types & consts (ordered)
+   posmap_rev_t typeConstPosR;  // reverse map from IDs to positions
    
    std::vector<spv::Id>  idMapL;   // ID {M}ap from {L}ocal to {G}lobal IDs