diff --git a/src/Error.cpp b/src/Error.cpp index 3f5cfc3..1b30ead 100644 --- a/src/Error.cpp +++ b/src/Error.cpp @@ -109,8 +109,7 @@ std::string Error::toString() const s << "openvcl: "; } - if( m_warning ) - s << "warning: "; + s << (m_warning ? "warning: " : "error: "); s << m_string; diff --git a/src/Parser.cpp b/src/Parser.cpp index b8b1946..055d259 100644 --- a/src/Parser.cpp +++ b/src/Parser.cpp @@ -2101,6 +2101,12 @@ bool Parser::generateCode() bool Parser::writeOutput() { + // write nothing once an error has been reported: a .vsm left behind + // would look up to date to make, and the next build would use it + + if( Error::HasErrors() ) + return false; + if( m_cmdLine.output() == "-" ) return writeOutputStream( std::cout ); @@ -2171,7 +2177,9 @@ std::string Parser::tempFilename() { std::stringstream buffer; - buffer << "/tmp/vcl" << std::hex << std::setfill('0') << std::setw(8) << ((getpid()+rand()) << 8) + m_tempCounter++; + // unsigned, so the sum and the shift wrap instead of overflowing an int + buffer << "/tmp/vcl" << std::hex << std::setfill('0') << std::setw(8) + << ((static_cast(getpid()) + static_cast(rand())) << 8) + m_tempCounter++; std::string temp = buffer.str(); std::ofstream file(temp.c_str(),std::ifstream::out); diff --git a/src/RegisterAllocator.cpp b/src/RegisterAllocator.cpp index cf6d79b..4d0507e 100644 --- a/src/RegisterAllocator.cpp +++ b/src/RegisterAllocator.cpp @@ -269,9 +269,10 @@ bool RegisterAllocator::process( std::list& tokens ) if( m_aliases.size() > 0 ) { - if( !processAliases() ) + Alias* unallocated = NULL; + if( !processAliases( unallocated ) ) { - Error::Display( Error( "Register allocation ran out of registers" ) ); + reportUnallocatedAlias( tokens, unallocated ); return false; } } @@ -559,7 +560,7 @@ bool RegisterAllocator::collectLabels( std::list::iterator start, std::li /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -bool RegisterAllocator::processAliases() +bool RegisterAllocator::processAliases( Alias*& unallocated ) { // allocate registers that do not overlap usage @@ -798,6 +799,7 @@ bool RegisterAllocator::processAliases() std::cerr << "Failed to allocate " << (dest->type() == Alias::FLOAT ? "FLOAT" : "INTEGER") << " register for alias" << std::endl; } + unallocated = dest; return false; } //assert( dest->allocatedRegister() ); @@ -808,6 +810,49 @@ bool RegisterAllocator::processAliases() /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +void RegisterAllocator::reportUnallocatedAlias( const std::list& tokens, const Alias* alias ) const +{ + if( !alias ) + { + Error::Display( Error( "Register allocation ran out of registers" ) ); + return; + } + + // the registers .init_vf/.init_vi left to the allocator + + const bool isFloat = (alias->type() == Alias::FLOAT); + unsigned int available = 0; + for( unsigned int i = 0; i < (isFloat ? 32u : 16u); ++i ) + { + if( (isFloat ? m_floats[i] : m_integers[i]).available() ) + ++available; + } + + std::stringstream message; + message << "Register allocation ran out of " << (isFloat ? "VF" : "VI") + << " registers (" << available << " available)"; + + // name the variable, where it first appears + + for( std::list::const_iterator i = tokens.begin(); i != tokens.end(); ++i ) + { + for( std::list::const_iterator a = (*i).arguments().begin(); a != (*i).arguments().end(); ++a ) + { + if( ((*a).content() == Token::Argument::ALIAS) && (*a).dependency() + && ((*a).dependency()->alias() == alias) ) + { + message << " for " << (*a).alias(); + Error::Display( Error( message.str(), *i, *a ) ); + return; + } + } + } + + Error::Display( Error( message.str() ) ); +} + +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + namespace { void extendContinuationBlockLiveRanges( std::list::iterator blockStart, std::list::iterator blockEnd ) diff --git a/src/RegisterAllocator.h b/src/RegisterAllocator.h index 738fca5..0da00da 100644 --- a/src/RegisterAllocator.h +++ b/src/RegisterAllocator.h @@ -88,7 +88,8 @@ class RegisterAllocator bool processCommonDirective( Token& token ); - bool processAliases(); + bool processAliases( Alias*& unallocated ); + void reportUnallocatedAlias( const std::list& tokens, const Alias* alias ) const; // Walks all emittable tokens and records, for each physical register, // the line numbers where it appears as a *literal* (non-alias) operand. diff --git a/src/Tokenizer.cpp b/src/Tokenizer.cpp index c906fd6..8dce64c 100644 --- a/src/Tokenizer.cpp +++ b/src/Tokenizer.cpp @@ -24,6 +24,7 @@ Tokenizer::Tokenizer() { setVsmMode( false ); setComment( false ); + setNewSyntax( false ); m_availableFloats = 0; m_availableIntegers = 0; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e1a162b..a1a5459 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -69,6 +69,7 @@ set(OPENVCL_TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/unit/test_vsm_cost_analyzer.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/openvcl_runner.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_cli.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_diagnostics.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_init_vf_range.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_clip_validation.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_clip_implicit_w.cpp diff --git a/test/integration/test_diagnostics.cpp b/test/integration/test_diagnostics.cpp new file mode 100644 index 0000000..d6a3237 --- /dev/null +++ b/test/integration/test_diagnostics.cpp @@ -0,0 +1,134 @@ +// Regression tests: openvcl's diagnostics are easy to find and act on. +// +// Errors were printed as "file(line) : message", without the "error:" tag +// warnings already had, so searching a build log for "error:" found +// nothing. Running out of registers said only "Register allocation ran out +// of registers", with no variable, register type or location. And a failed +// build still wrote its output file, which then looked up to date to make. + +#include "test_harness.h" +#include "openvcl_runner.h" + +#include +#include +#include +#include +#include +#include +#include + +namespace +{ + bool contains(const std::string& text, const std::string& needle) + { + return text.find(needle) != std::string::npos; + } + + std::string program(const std::string& directives, const std::string& body) + { + return directives + + "\t.syntax new\n" + "\t.name vsmDiagnostics\n" + "\t--enter\n" + "\t--endenter\n" + + body + + "\t--exit\n" + "\t--endexit\n"; + } + + const char* kAllRegisters = "\t.init_vf_all\n\t.init_vi_all\n"; + + // Line 8 passes clipw a field other than W. + std::string invalidProgram() + { + return program(kAllRegisters, + "\tlq fA, 1(vi00)\n" + "\tclipw.xyz fA, fA[x]\n" + "\tfcand vi01, 0x3f\n" + "\tisw.x vi01, 2(vi00)\n"); + } + + std::string validProgram() + { + return program(kAllRegisters, + "\tlq fA, 1(vi00)\n" + "\tsq fA, 2(vi00)\n"); + } +} + +TEST_CASE("openvcl: an error at a source line is tagged error:") +{ + ::test::RunResult r = ::test::run_openvcl(std::vector(), invalidProgram()); + + CHECK(r.exit_code != 0); + CHECK(contains(r.stderr_data, "(8) : error: Invalid argument (clipw.xyz fA, >>> fA[x] <<<")); +} + +TEST_CASE("openvcl: an error without a location is tagged error:") +{ + std::vector args; + args.push_back("-o"); + args.push_back("/nonexistent-openvcl-dir/out.vsm"); + ::test::RunResult r = ::test::run_openvcl(args, validProgram()); + + CHECK(r.exit_code != 0); + CHECK(contains(r.stderr_data, "openvcl: error: Could not open output file")); +} + +TEST_CASE("openvcl: running out of VI registers names the variable and where it is set") +{ + // only two VI registers for three values live at once; line 9 sets the third + ::test::RunResult r = ::test::run_openvcl(std::vector(), program( + "\t.init_vf_all\n\t.init_vi vi01, vi02\n", + "\tilw.x iA, 0(vi00)\n" + "\tilw.x iB, 1(vi00)\n" + "\tilw.x iC, 2(vi00)\n" + "\tiadd iD, iA, iB\n" + "\tiadd iE, iD, iC\n" + "\tisw.x iE, 3(vi00)\n")); + + CHECK(r.exit_code != 0); + CHECK(contains(r.stderr_data, "(9) : error: Register allocation ran out of VI registers (2 available) for iC (ilw.x >>> iC <<<, 2(vi00))")); +} + +TEST_CASE("openvcl: running out of VF registers names the variable and where it is set") +{ + ::test::RunResult r = ::test::run_openvcl(std::vector(), program( + "\t.init_vf vf01-vf02\n\t.init_vi_all\n", + "\tlq fA, 0(vi00)\n" + "\tlq fB, 1(vi00)\n" + "\tlq fC, 2(vi00)\n" + "\tadd fD, fA, fB\n" + "\tadd fE, fD, fC\n" + "\tsq fE, 3(vi00)\n")); + + CHECK(r.exit_code != 0); + CHECK(contains(r.stderr_data, "(9) : error: Register allocation ran out of VF registers (2 available) for fC (lq >>> fC <<<")); +} + +TEST_CASE("openvcl: a failed build writes no output") +{ + char path[] = "/tmp/openvcl_diag_XXXXXX.vsm"; + int fd = mkstemps(path, 4); + REQUIRE(fd >= 0); + const char stale[] = "stale output from an earlier build\n"; + REQUIRE(write(fd, stale, sizeof(stale) - 1) == static_cast(sizeof(stale) - 1)); + close(fd); + + std::vector args; + args.push_back("-o"); + args.push_back(path); + ::test::RunResult r = ::test::run_openvcl(args, invalidProgram()); + + std::ifstream f(path); + std::stringstream contents; + contents << f.rdbuf(); + std::remove(path); + + CHECK(r.exit_code != 0); + CHECK(contents.str() == stale); + + ::test::RunResult toStdout = ::test::run_openvcl(std::vector(), invalidProgram()); + CHECK(toStdout.exit_code != 0); + CHECK(toStdout.stdout_data.empty()); +}