From 09caf12beca6a9bfc11f2140c8fec62ed4c7e95b Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Mon, 2 May 2016 18:11:54 -0400 Subject: [PATCH] Avoid printing to stdout directly in library functions. Previously GlslangToSpv() reported missing/TBD functionalities by directly writing to stdout using printf. That could cause problems to callers of GlslangToSpv(). This patch cleans up the error reporting logic in GlslangToSpv(), TGlslangToSpvTraverser, and spv::Builder a little bit to use ostringstream. Also fixed the usage of GlslangToSpv() in GTest fixtures to capture warnings/errors reported when translating AST to SPIR-V. --- SPIRV/GlslangToSpv.cpp | 31 +++++++++++++++++++++---------- SPIRV/GlslangToSpv.h | 1 + SPIRV/SpvBuilder.cpp | 16 ++++++++-------- SPIRV/SpvBuilder.h | 14 +++++++++----- StandAlone/StandAlone.cpp | 4 +++- gtests/TestFixture.h | 11 ++++++++--- 6 files changed, 50 insertions(+), 27 deletions(-) diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index 0c62d52..b4d5fb1 100755 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -109,6 +109,8 @@ public: void dumpSpv(std::vector& out); + std::string getWarningsAndErrors() const { return warningsErrors.str(); } + protected: spv::Decoration TranslateInterpolationDecoration(const glslang::TQualifier& qualifier); spv::BuiltIn TranslateBuiltInDecoration(glslang::TBuiltInVariable); @@ -159,6 +161,8 @@ protected: spv::Instruction* entryPoint; int sequenceDepth; + std::ostringstream warningsErrors; + // There is a 1:1 mapping between a spv builder and a module; this is thread safe spv::Builder builder; bool inMain; @@ -433,7 +437,7 @@ spv::BuiltIn TGlslangToSpvTraverser::TranslateBuiltInDecoration(glslang::TBuiltI case glslang::EbvBaseInstance: case glslang::EbvDrawId: // TODO: Add SPIR-V builtin ID. - spv::MissingFunctionality("Draw parameters"); + spv::MissingFunctionality(warningsErrors, "Draw parameters"); return (spv::BuiltIn)spv::BadValue; case glslang::EbvPrimitiveId: return spv::BuiltInPrimitiveId; case glslang::EbvInvocationId: return spv::BuiltInInvocationId; @@ -608,7 +612,7 @@ bool HasNonLayoutQualifiers(const glslang::TQualifier& qualifier) TGlslangToSpvTraverser::TGlslangToSpvTraverser(const glslang::TIntermediate* glslangIntermediate) : TIntermTraverser(true, false, true), shaderEntry(0), sequenceDepth(0), - builder((glslang::GetKhronosToolId() << 16) | GeneratorVersion), + builder((glslang::GetKhronosToolId() << 16) | GeneratorVersion, warningsErrors), inMain(false), mainTerminated(false), linkageOnly(false), glslangIntermediate(glslangIntermediate) { @@ -984,7 +988,7 @@ bool TGlslangToSpvTraverser::visitBinary(glslang::TVisit /* visit */, glslang::T builder.clearAccessChain(); if (! result) { - spv::MissingFunctionality("unknown glslang binary operation"); + spv::MissingFunctionality(warningsErrors, "unknown glslang binary operation"); return true; // pick up a child as the place-holder result } else { builder.setAccessChainRValue(result); @@ -1109,7 +1113,7 @@ bool TGlslangToSpvTraverser::visitUnary(glslang::TVisit /* visit */, glslang::TI return false; default: - spv::MissingFunctionality("unknown glslang unary"); + spv::MissingFunctionality(warningsErrors, "unknown glslang unary"); return true; // pick up operand as placeholder result } } @@ -1220,7 +1224,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"); + spv::MissingFunctionality(warningsErrors, "missing user function; linker needs to catch that"); return false; } @@ -1468,7 +1472,7 @@ bool TGlslangToSpvTraverser::visitAggregate(glslang::TVisit visit, glslang::TInt return false; if (! result) { - spv::MissingFunctionality("unknown glslang aggregate"); + spv::MissingFunctionality(warningsErrors, "unknown glslang aggregate"); return true; // pick up a child as a placeholder operand } else { builder.clearAccessChain(); @@ -1761,7 +1765,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?"); + spv::TbdFunctionality(warningsErrors, "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 +3151,7 @@ spv::Id TGlslangToSpvTraverser::createUnaryOperation(glslang::TOperator op, spv: case glslang::EOpUnpackInt2x32: case glslang::EOpPackUint2x32: case glslang::EOpUnpackUint2x32: - spv::MissingFunctionality("shader int64"); + spv::MissingFunctionality(warningsErrors, "shader int64"); libCall = spv::GLSLstd450Bad; // TODO: This is a placeholder. break; @@ -3778,7 +3782,7 @@ spv::Id TGlslangToSpvTraverser::createNoArgOperation(glslang::TOperator op) builder.createMemoryBarrier(spv::ScopeDevice, spv::MemorySemanticsCrossWorkgroupMemoryMask); return 0; default: - spv::MissingFunctionality("unknown operation with no arguments"); + spv::MissingFunctionality(warningsErrors, "unknown operation with no arguments"); return 0; } } @@ -3939,7 +3943,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."); + spv::MissingFunctionality(warningsErrors, "Neither a front-end constant nor a spec constant."); exit(1); return spv::NoResult; } @@ -4194,6 +4198,11 @@ void OutputSpv(const std::vector& spirv, const char* baseName) // void GlslangToSpv(const glslang::TIntermediate& intermediate, std::vector& spirv) { + GlslangToSpv(intermediate, spirv, nullptr); +} + +void GlslangToSpv(const glslang::TIntermediate& intermediate, std::vector& spirv, std::string* messages) +{ TIntermNode* root = intermediate.getTreeRoot(); if (root == 0) @@ -4207,6 +4216,8 @@ void GlslangToSpv(const glslang::TIntermediate& intermediate, std::vector& spirv); +void GlslangToSpv(const glslang::TIntermediate& intermediate, std::vector& spirv, std::string* messages); void OutputSpv(const std::vector& spirv, const char* baseName); } diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp index 0171b15..8250dc7 100644 --- a/SPIRV/SpvBuilder.cpp +++ b/SPIRV/SpvBuilder.cpp @@ -43,7 +43,6 @@ // #include -#include #include #include @@ -57,7 +56,7 @@ namespace spv { -Builder::Builder(unsigned int magicNumber) : +Builder::Builder(unsigned int magicNumber, std::ostringstream& warnError) : 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), + warningsErrors(warnError) { 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"); + MissingFunctionality(warningsErrors, "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,19 @@ void Builder::dumpInstructions(std::vector& out, const std::vector } } -void TbdFunctionality(const char* tbd) +void TbdFunctionality(std::ostringstream& stream, const char* tbd) { static std::unordered_set issued; if (issued.find(tbd) == issued.end()) { - printf("TBD functionality: %s\n", tbd); + stream << "TBD functionality: " << tbd << "\n"; issued.insert(tbd); } } -void MissingFunctionality(const char* fun) +void MissingFunctionality(std::ostringstream& stream, const char* fun) { - printf("Missing functionality: %s\n", fun); + stream << "Missing functionality: " << fun << "\n"; } }; // end spv namespace diff --git a/SPIRV/SpvBuilder.h b/SPIRV/SpvBuilder.h index 8c05f4e..93cdf2f 100755 --- a/SPIRV/SpvBuilder.h +++ b/SPIRV/SpvBuilder.h @@ -53,16 +53,17 @@ #include "spvIR.h" #include -#include -#include #include +#include #include +#include +#include namespace spv { class Builder { public: - Builder(unsigned int userNumber); + Builder(unsigned int userNumber, std::ostringstream& warnError); virtual ~Builder(); static const int maxMatrixSize = 4; @@ -580,13 +581,16 @@ public: // Our loop stack. std::stack loops; + + // The stream for outputing warnings and errors. + std::ostringstream& warningsErrors; }; // end Builder class // Use for non-fatal notes about what's not complete -void TbdFunctionality(const char*); +void TbdFunctionality(std::ostringstream&, const char*); // Use for fatal missing functionality -void MissingFunctionality(const char*); +void MissingFunctionality(std::ostringstream&, const char*); }; // end spv namespace diff --git a/StandAlone/StandAlone.cpp b/StandAlone/StandAlone.cpp index 2f5a718..b2bee85 100644 --- a/StandAlone/StandAlone.cpp +++ b/StandAlone/StandAlone.cpp @@ -671,11 +671,13 @@ void CompileAndLinkShaderUnits(std::vector compUnits) for (int stage = 0; stage < EShLangCount; ++stage) { if (program.getIntermediate((EShLanguage)stage)) { std::vector spirv; - glslang::GlslangToSpv(*program.getIntermediate((EShLanguage)stage), spirv); + std::string warningsErrors; + glslang::GlslangToSpv(*program.getIntermediate((EShLanguage)stage), spirv, &warningsErrors); // 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", warningsErrors.c_str()); glslang::OutputSpv(spirv, GetBinaryName((EShLanguage)stage)); if (Options & EOptionHumanReadableSpv) { spv::Disassemble(std::cout, spirv); diff --git a/gtests/TestFixture.h b/gtests/TestFixture.h index 87a365b..a770247 100644 --- a/gtests/TestFixture.h +++ b/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); + std::string spirvWarningsErrors; + if (success && target == Target::Spirv) { std::vector spirv_binary; glslang::GlslangToSpv(*program.getIntermediate(language), - spirv_binary); + spirv_binary, &spirvWarningsErrors); std::ostringstream disassembly_stream; spv::Parameterize(); spv::Disassemble(disassembly_stream, spirv_binary); return {shader.getInfoLog(), shader.getInfoDebugLog(), program.getInfoLog(), program.getInfoDebugLog(), - disassembly_stream.str()}; + spirvWarningsErrors, 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() -- 2.7.4