Merge pull request #257 from antiagainst/ostrstream

Avoid printing to stdout directly in library functions.

SPIRV/CMakeLists.txt

@@ -1,6 +1,7 @@
 set(SOURCES
     GlslangToSpv.cpp
     InReadableOrder.cpp
+    Logger.cpp
     SpvBuilder.cpp
     SPVRemapper.cpp
     doc.cpp
@@ -10,6 +11,7 @@ set(HEADERS
     spirv.hpp
     GLSL.std.450.h
     GlslangToSpv.h
+    Logger.h
     SpvBuilder.h
     SPVRemapper.h
     spvIR.h

SPIRV/GlslangToSpv.cpp

@@ -52,12 +52,12 @@ namespace spv {
 #include "../glslang/MachineIndependent/SymbolTable.h"
 #include "../glslang/Include/Common.h"
 
-#include <string>
-#include <map>
-#include <list>
-#include <vector>
-#include <stack>
 #include <fstream>
+#include <list>
+#include <map>
+#include <stack>
+#include <string>
+#include <vector>
 
 namespace {
 
@@ -94,7 +94,7 @@ private:
 //
 class TGlslangToSpvTraverser : public glslang::TIntermTraverser {
 public:
-    TGlslangToSpvTraverser(const glslang::TIntermediate*);
+    TGlslangToSpvTraverser(const glslang::TIntermediate*, spv::SpvBuildLogger* logger);
     virtual ~TGlslangToSpvTraverser();
 
     bool visitAggregate(glslang::TVisit, glslang::TIntermAggregate*);
@@ -159,6 +159,8 @@ protected:
     spv::Instruction* entryPoint;
     int sequenceDepth;
 
+    spv::SpvBuildLogger* logger;
+
     // There is a 1:1 mapping between a spv builder and a module; this is thread safe
     spv::Builder builder;
     bool inMain;
@@ -433,7 +435,7 @@ spv::BuiltIn TGlslangToSpvTraverser::TranslateBuiltInDecoration(glslang::TBuiltI
     case glslang::EbvBaseInstance:
     case glslang::EbvDrawId:
         // TODO: Add SPIR-V builtin ID.
-        spv::MissingFunctionality("Draw parameters");
+        logger->missingFunctionality("Draw parameters");
         return (spv::BuiltIn)spv::BadValue;
     case glslang::EbvPrimitiveId:          return spv::BuiltInPrimitiveId;
     case glslang::EbvInvocationId:         return spv::BuiltInInvocationId;
@@ -606,9 +608,9 @@ bool HasNonLayoutQualifiers(const glslang::TQualifier& qualifier)
 // Implement the TGlslangToSpvTraverser class.
 //
 
-TGlslangToSpvTraverser::TGlslangToSpvTraverser(const glslang::TIntermediate* glslangIntermediate)
+TGlslangToSpvTraverser::TGlslangToSpvTraverser(const glslang::TIntermediate* glslangIntermediate, spv::SpvBuildLogger* buildLogger)
-    : TIntermTraverser(true, false, true), shaderEntry(0), sequenceDepth(0),
+    : TIntermTraverser(true, false, true), shaderEntry(0), sequenceDepth(0), logger(buildLogger),
-      builder((glslang::GetKhronosToolId() << 16) | GeneratorVersion),
+      builder((glslang::GetKhronosToolId() << 16) | GeneratorVersion, logger),
       inMain(false), mainTerminated(false), linkageOnly(false),
       glslangIntermediate(glslangIntermediate)
 {
@@ -984,7 +986,7 @@ bool TGlslangToSpvTraverser::visitBinary(glslang::TVisit /* visit */, glslang::T
 
     builder.clearAccessChain();
     if (! result) {
-        spv::MissingFunctionality("unknown glslang binary operation");
+        logger->missingFunctionality("unknown glslang binary operation");
         return true;  // pick up a child as the place-holder result
     } else {
         builder.setAccessChainRValue(result);
@@ -1109,7 +1111,7 @@ bool TGlslangToSpvTraverser::visitUnary(glslang::TVisit /* visit */, glslang::TI
         return false;
 
     default:
-        spv::MissingFunctionality("unknown glslang unary");
+        logger->missingFunctionality("unknown glslang unary");
         return true;  // pick up operand as placeholder result
     }
 }
@@ -1220,7 +1222,7 @@ bool TGlslangToSpvTraverser::visitAggregate(glslang::TVisit visit, glslang::TInt
             builder.clearAccessChain();
             builder.setAccessChainRValue(result);
         } else
-            spv::MissingFunctionality("missing user function; linker needs to catch that");
+            logger->missingFunctionality("missing user function; linker needs to catch that");
 
         return false;
     }
@@ -1468,7 +1470,7 @@ bool TGlslangToSpvTraverser::visitAggregate(glslang::TVisit visit, glslang::TInt
         return false;
 
     if (! result) {
-        spv::MissingFunctionality("unknown glslang aggregate");
+        logger->missingFunctionality("unknown glslang aggregate");
         return true;  // pick up a child as a placeholder operand
     } else {
         builder.clearAccessChain();
@@ -1761,7 +1763,7 @@ spv::Id TGlslangToSpvTraverser::convertGlslangToSpvType(const glslang::TType& ty
         spvType = builder.makeUintType(64);
         break;
     case glslang::EbtAtomicUint:
-        spv::TbdFunctionality("Is atomic_uint an opaque handle in the uniform storage class, or an addresses in the atomic storage class?");
+        logger->tbdFunctionality("Is atomic_uint an opaque handle in the uniform storage class, or an addresses in the atomic storage class?");
         spvType = builder.makeUintType(32);
         break;
     case glslang::EbtSampler:
@@ -3147,7 +3149,7 @@ spv::Id TGlslangToSpvTraverser::createUnaryOperation(glslang::TOperator op, spv:
     case glslang::EOpUnpackInt2x32:
     case glslang::EOpPackUint2x32:
     case glslang::EOpUnpackUint2x32:
-        spv::MissingFunctionality("shader int64");
+        logger->missingFunctionality("shader int64");
         libCall = spv::GLSLstd450Bad; // TODO: This is a placeholder.
         break;
 
@@ -3778,7 +3780,7 @@ spv::Id TGlslangToSpvTraverser::createNoArgOperation(glslang::TOperator op)
         builder.createMemoryBarrier(spv::ScopeDevice, spv::MemorySemanticsCrossWorkgroupMemoryMask);
         return 0;
     default:
-        spv::MissingFunctionality("unknown operation with no arguments");
+        logger->missingFunctionality("unknown operation with no arguments");
         return 0;
     }
 }
@@ -3939,7 +3941,7 @@ spv::Id TGlslangToSpvTraverser::createSpvConstant(const glslang::TIntermTyped& n
 
     // Neither a front-end constant node, nor a specialization constant node with constant union array or
     // constant sub tree as initializer.
-    spv::MissingFunctionality("Neither a front-end constant nor a spec constant.");
+    logger->missingFunctionality("Neither a front-end constant nor a spec constant.");
     exit(1);
     return spv::NoResult;
 }
@@ -4193,6 +4195,12 @@ void OutputSpv(const std::vector<unsigned int>& spirv, const char* baseName)
 // Set up the glslang traversal
 //
 void GlslangToSpv(const glslang::TIntermediate& intermediate, std::vector<unsigned int>& spirv)
+{
+    spv::SpvBuildLogger logger;
+    GlslangToSpv(intermediate, spirv, &logger);
+}
+
+void GlslangToSpv(const glslang::TIntermediate& intermediate, std::vector<unsigned int>& spirv, spv::SpvBuildLogger* logger)
 {
     TIntermNode* root = intermediate.getTreeRoot();
 
@@ -4201,7 +4209,7 @@ void GlslangToSpv(const glslang::TIntermediate& intermediate, std::vector<unsign
 
     glslang::GetThreadPoolAllocator().push();
 
-    TGlslangToSpvTraverser it(&intermediate);
+    TGlslangToSpvTraverser it(&intermediate, logger);
 
     root->traverse(&it);
 

SPIRV/GlslangToSpv.h

@@ -34,10 +34,16 @@
 
 #include "../glslang/Include/intermediate.h"
 
+#include <string>
+#include <vector>
+
+#include "Logger.h"
+
 namespace glslang {
 
 void GetSpirvVersion(std::string&);
 void GlslangToSpv(const glslang::TIntermediate& intermediate, std::vector<unsigned int>& spirv);
+void GlslangToSpv(const glslang::TIntermediate& intermediate, std::vector<unsigned int>& spirv, spv::SpvBuildLogger* logger);
 void OutputSpv(const std::vector<unsigned int>& spirv, const char* baseName);
 
 }

SPIRV/Logger.cpp

@@ -0,0 +1,68 @@
+//
+// Copyright (C) 2016 Google, Inc.
+//
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions
+// are met:
+//
+//    Redistributions of source code must retain the above copyright
+//    notice, this list of conditions and the following disclaimer.
+//
+//    Redistributions in binary form must reproduce the above
+//    copyright notice, this list of conditions and the following
+//    disclaimer in the documentation and/or other materials provided
+//    with the distribution.
+//
+//    Neither the name of Google Inc. nor the names of its
+//    contributors may be used to endorse or promote products derived
+//    from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+// FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+// COPYRIGHT HOLDERS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+// INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+// BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+// LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+// LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+// ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+// POSSIBILITY OF SUCH DAMAGE.
+
+#include "Logger.h"
+
+#include <algorithm>
+#include <iterator>
+#include <sstream>
+
+namespace spv {
+
+void SpvBuildLogger::tbdFunctionality(const char* f)
+{
+    if (std::find(std::begin(tbdFeatures), std::end(tbdFeatures), f) == std::end(tbdFeatures))
+        tbdFeatures.push_back(f);
+}
+
+void SpvBuildLogger::missingFunctionality(const char* f)
+{
+    if (std::find(std::begin(missingFeatures), std::end(missingFeatures), f) == std::end(missingFeatures))
+        missingFeatures.push_back(f);
+}
+
+std::string SpvBuildLogger::getAllMessages() const {
+    std::ostringstream messages;
+    for (const auto& f : tbdFeatures)
+        messages << "TBD functionality: " << f << "\n";
+    for (const auto& f : missingFeatures)
+        messages << "Missing functionality: " << f << "\n";
+    for (const auto& w : warnings)
+        messages << "warning: " << w << "\n";
+    for (const auto& e : errors)
+        messages << "error: " << e << "\n";
+    return messages.str();
+}
+
+} // end spv namespace

SPIRV/Logger.h

@@ -0,0 +1,73 @@
+//
+// Copyright (C) 2016 Google, Inc.
+//
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions
+// are met:
+//
+//    Redistributions of source code must retain the above copyright
+//    notice, this list of conditions and the following disclaimer.
+//
+//    Redistributions in binary form must reproduce the above
+//    copyright notice, this list of conditions and the following
+//    disclaimer in the documentation and/or other materials provided
+//    with the distribution.
+//
+//    Neither the name of Google Inc. nor the names of its
+//    contributors may be used to endorse or promote products derived
+//    from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+// FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+// COPYRIGHT HOLDERS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+// INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+// BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+// LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+// LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+// ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+// POSSIBILITY OF SUCH DAMAGE.
+
+#ifndef GLSLANG_SPIRV_LOGGER_H
+#define GLSLANG_SPIRV_LOGGER_H
+
+#include <string>
+#include <vector>
+
+namespace spv {
+
+// A class for holding all SPIR-V build status messages, including
+// missing/TBD functionalities, warnings, and errors.
+class SpvBuildLogger {
+public:
+    SpvBuildLogger() = default;
+    SpvBuildLogger(const SpvBuildLogger&) = delete;
+
+    // Registers a TBD functionality.
+    void tbdFunctionality(const char* f);
+    // Registers a missing functionality.
+    void missingFunctionality(const char* f);
+
+    // Logs a warning.
+    void warning(const std::string& w) { warnings.push_back(w); }
+    // Logs an error.
+    void error(const std::string& e) { errors.push_back(e); }
+
+    // Returns all messages accumulated in the order of:
+    // TBD functionalities, missing functionalities, warnings, errors.
+    std::string getAllMessages() const;
+
+private:
+    std::vector<const char*> tbdFeatures;
+    std::vector<const char*> missingFeatures;
+    std::vector<std::string> warnings;
+    std::vector<std::string> errors;
+};
+
+} // end spv namespace
+
+#endif // GLSLANG_SPIRV_LOGGER_H

SPIRV/SpvBuilder.cpp

@@ -43,7 +43,6 @@
 //
 
 #include <assert.h>
-#include <stdio.h>
 #include <stdlib.h>
 
 #include <unordered_set>
@@ -57,7 +56,7 @@
 
 namespace spv {
 
-Builder::Builder(unsigned int magicNumber) :
+Builder::Builder(unsigned int magicNumber, SpvBuildLogger* buildLogger) :
     source(SourceLanguageUnknown),
     sourceVersion(0),
     addressModel(AddressingModelLogical),
@@ -66,7 +65,8 @@ Builder::Builder(unsigned int magicNumber) :
     buildPoint(0),
     uniqueId(0),
     mainFunction(0),
-    generatingOpCodeForSpecConst(false)
+    generatingOpCodeForSpecConst(false),
+    logger(buildLogger)
 {
     clearAccessChain();
 }
@@ -2111,7 +2111,7 @@ void Builder::accessChainStore(Id rvalue)
     Id base = collapseAccessChain();
 
     if (accessChain.swizzle.size() && accessChain.component != NoResult)
-        MissingFunctionality("simultaneous l-value swizzle and dynamic component selection");
+        logger->missingFunctionality("simultaneous l-value swizzle and dynamic component selection");
 
     // If swizzle still exists, it is out-of-order or not full, we must load the target vector,
     // extract and insert elements to perform writeMask and/or swizzle.
@@ -2487,19 +2487,4 @@ void Builder::dumpInstructions(std::vector<unsigned int>& out, const std::vector
     }
 }
 
-void TbdFunctionality(const char* tbd)
-{
-    static std::unordered_set<const char*> issued;
-
-    if (issued.find(tbd) == issued.end()) {
-        printf("TBD functionality: %s\n", tbd);
-        issued.insert(tbd);
-    }
-}
-
-void MissingFunctionality(const char* fun)
-{
-    printf("Missing functionality: %s\n", fun);
-}
-
 }; // end spv namespace

SPIRV/SpvBuilder.h

@@ -49,20 +49,22 @@
 #ifndef SpvBuilder_H
 #define SpvBuilder_H
 
+#include "Logger.h"
 #include "spirv.hpp"
 #include "spvIR.h"
 
 #include <algorithm>
-#include <memory>
-#include <stack>
 #include <map>
+#include <memory>
 #include <set>
+#include <sstream>
+#include <stack>
 
 namespace spv {
 
 class Builder {
 public:
-    Builder(unsigned int userNumber);
+    Builder(unsigned int userNumber, SpvBuildLogger* logger);
     virtual ~Builder();
 
     static const int maxMatrixSize = 4;
@@ -580,14 +582,11 @@ public:
 
     // Our loop stack.
     std::stack<LoopBlocks> loops;
+
+    // The stream for outputing warnings and errors.
+    SpvBuildLogger* logger;
 };  // end Builder class
 
-// Use for non-fatal notes about what's not complete
-void TbdFunctionality(const char*);
-
-// Use for fatal missing functionality
-void MissingFunctionality(const char*);
-
 };  // end spv namespace
 
 #endif // SpvBuilder_H

StandAlone/StandAlone.cpp

@@ -671,11 +671,14 @@ void CompileAndLinkShaderUnits(std::vector<ShaderCompUnit> compUnits)
             for (int stage = 0; stage < EShLangCount; ++stage) {
                 if (program.getIntermediate((EShLanguage)stage)) {
                     std::vector<unsigned int> spirv;
-                    glslang::GlslangToSpv(*program.getIntermediate((EShLanguage)stage), spirv);
+                    std::string warningsErrors;
+                    spv::SpvBuildLogger logger;
+                    glslang::GlslangToSpv(*program.getIntermediate((EShLanguage)stage), spirv, &logger);
 
                     // Dump the spv to a file or stdout, etc., but only if not doing
                     // memory/perf testing, as it's not internal to programmatic use.
                     if (! (Options & EOptionMemoryLeakMode)) {
+                      printf("%s", logger.getAllMessages().c_str());
                         glslang::OutputSpv(spirv, GetBinaryName((EShLanguage)stage));
                         if (Options & EOptionHumanReadableSpv) {
                             spv::Disassemble(std::cout, spirv);

Test/baseResults/spv.int64.frag.out

@@ -5,9 +5,6 @@ Warning, version 450 is not yet complete; most version-specific features are pre
 Linked fragment stage:
 
 
-Missing functionality: shader int64
-Missing functionality: shader int64
-Missing functionality: shader int64
 Missing functionality: shader int64
 // Module Version 10000
 // Generated by (magic number): 80001

gtests/TestFixture.h

@@ -156,6 +156,7 @@ public:
         const std::string compilationError;
         const std::string linkingOutput;
         const std::string linkingError;
+        const std::string spirvWarningsErrors;
         const std::string spirv;  // Optional SPIR-V disassembly text.
     };
 
@@ -187,20 +188,23 @@ public:
         program.addShader(&shader);
         success &= program.link(messages);
 
+        spv::SpvBuildLogger logger;
+
         if (success && target == Target::Spirv) {
             std::vector<uint32_t> spirv_binary;
             glslang::GlslangToSpv(*program.getIntermediate(language),
-                                  spirv_binary);
+                                  spirv_binary, &logger);
 
             std::ostringstream disassembly_stream;
             spv::Parameterize();
             spv::Disassemble(disassembly_stream, spirv_binary);
             return {shader.getInfoLog(), shader.getInfoDebugLog(),
                     program.getInfoLog(), program.getInfoDebugLog(),
-                    disassembly_stream.str()};
+                    logger.getAllMessages(), disassembly_stream.str()};
         } else {
             return {shader.getInfoLog(), shader.getInfoDebugLog(),
-                    program.getInfoLog(), program.getInfoDebugLog(), ""};
+                    program.getInfoLog(), program.getInfoDebugLog(),
+                    "", ""};
         }
     }
 
@@ -231,6 +235,7 @@ public:
         outputIfNotEmpty(result.compilationError);
         outputIfNotEmpty(result.linkingOutput);
         outputIfNotEmpty(result.linkingError);
+        stream << result.spirvWarningsErrors;
         if (target == Target::Spirv) {
             stream
                 << (result.spirv.empty()