diff --git a/src/BranchState.h b/src/BranchState.h index 9e8c979..17dca0c 100644 --- a/src/BranchState.h +++ b/src/BranchState.h @@ -167,6 +167,10 @@ class BranchState const Register* inputRegister( Alias::Type type, const std::string& name ); + // the block's outputs, by name, and the number of the register each leaves in + const std::map& floatOutputs() const; + const std::map& integerOutputs() const; + BranchState* clone(); private: diff --git a/src/BranchState.inl b/src/BranchState.inl index 512f896..cfde5c8 100644 --- a/src/BranchState.inl +++ b/src/BranchState.inl @@ -34,6 +34,23 @@ inline BranchState::BranchState( const BranchState& a ) : m_allocator(a.m_alloca m_floatInputs = a.m_floatInputs; m_integerInputs = a.m_integerInputs; + + m_floatOutputs = a.m_floatOutputs; + m_integerOutputs = a.m_integerOutputs; +} + +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +inline const std::map& BranchState::floatOutputs() const +{ + return m_floatOutputs; +} + +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +inline const std::map& BranchState::integerOutputs() const +{ + return m_integerOutputs; } /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/RegisterAllocator.cpp b/src/RegisterAllocator.cpp index 7a293e6..0a954ad 100644 --- a/src/RegisterAllocator.cpp +++ b/src/RegisterAllocator.cpp @@ -581,6 +581,46 @@ namespace std::vector live; }; + // The read of an output at a block's exit, and the register it names. + struct ExitRead + { + std::string name; + Dependency* read; + const Register* reg; + }; + + // Adds a read of every lane of each output at node `exit`, for the + // outputs that are written in the block or enter it as inputs. + void addExitReads( RegisterAllocator& allocator, BranchState* entry, Alias::Type type, const std::map& outputs, + std::list::iterator exitToken, unsigned int exit, std::map< Variable, VariableAccesses >& variables, std::vector& exitReads ) + { + for( std::map::const_iterator o = outputs.begin(); o != outputs.end(); ++o ) + { + const Variable variable( type, o->first ); + std::map< Variable, VariableAccesses >::const_iterator v = variables.find( variable ); + const bool written = (v != variables.end()) && !v->second.writes.empty(); + if( !written && !entry->inputRegister( type, o->first ) ) + continue; + + Dependency* read = new Dependency; + read->setAlias( allocator.obtainAlias( type ) ); + read->setToken( *exitToken ); + read->setState( entry ); + + VariableAccess access; + access.node = exit; + access.lanes = (type == Alias::FLOAT) ? (Token::X|Token::Y|Token::Z|Token::W) : 1; + access.dependency = read; + variables[ variable ].reads.push_back( access ); + + ExitRead exitRead; + exitRead.name = o->first; + exitRead.read = read; + exitRead.reg = (type == Alias::FLOAT) ? allocator.floatRegister( o->second ) : allocator.integerRegister( o->second ); + exitReads.push_back( exitRead ); + } + } + bool sameRegister( const Token::Argument& a, const Token::Argument& b ) { if( a.content() != b.content() ) @@ -839,6 +879,17 @@ bool RegisterAllocator::completeLiveRanges( std::list::iterator begin, st } } + // the exit, node `count`, reads the block's outputs; control reaches it + // from the last token when that runs on + + std::vector exitReads; + if( flow.fallsOffEnd ) + { + addExitReads( *this, entry, Alias::FLOAT, entry->floatOutputs(), end, count, variables, exitReads ); + addExitReads( *this, entry, Alias::INTEGER, entry->integerOutputs(), end, count, variables, exitReads ); + } + const std::vector exitPredecessors( 1, count - 1 ); + std::vector reached; std::map< Variable, std::vector > inputReads; @@ -875,8 +926,9 @@ bool RegisterAllocator::completeLiveRanges( std::list::iterator begin, st bool reachesEntry = ((*r).node == 0); std::vector< std::pair > back; - for( unsigned int p = 0; p < predecessors[(*r).node].size(); ++p ) - back.push_back( std::make_pair( predecessors[(*r).node][p], (*r).lanes ) ); + const std::vector& from = ((*r).node == count) ? exitPredecessors : predecessors[(*r).node]; + for( unsigned int p = 0; p < from.size(); ++p ) + back.push_back( std::make_pair( from[p], (*r).lanes ) ); while( !back.empty() ) { @@ -960,6 +1012,20 @@ bool RegisterAllocator::completeLiveRanges( std::list::iterator begin, st } } + // each output leaves in the register its out_vf or out_vi names + + for( std::vector::const_iterator e = exitReads.begin(); e != exitReads.end(); ++e ) + { + Alias* alias = (*e).read->alias(); + if( !alias->allocatedRegister() ) + alias->setAllocatedRegister( (*e).reg ); + else if( alias->allocatedRegister() != (*e).reg ) + { + reportRegisterConflict( (*e).name, alias->allocatedRegister(), (*e).reg, (*e).read->token() ); + return false; + } + } + // after the joins, each read's alias is live from the writes to the read for( std::vector::const_iterator r = reached.begin(); r != reached.end(); ++r ) diff --git a/src/RegisterAllocator.h b/src/RegisterAllocator.h index 1e2242d..79ad1db 100644 --- a/src/RegisterAllocator.h +++ b/src/RegisterAllocator.h @@ -95,7 +95,8 @@ class RegisterAllocator // a value carried around the loop can lose its register there. This walks // the block's control flow back from every read to the writes that reach // it, and joins them into one alias live in between, as a complete walk - // would have. + // would have. The block's exit reads its outputs the same way, so each + // stays in the register its out_vf or out_vi names until then. bool completeLiveRanges( std::list::iterator begin, std::list::iterator end, BranchState* entry ); // For each VF write that leaves lanes of its variable as they were, the diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 731b8bd..8f1248b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -84,6 +84,7 @@ set(OPENVCL_TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_continuation_lifetime.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_two_address_coalescing.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_loop_liveness.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_exit_outputs.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_vsm_cost.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_software_pipeline.cpp ) diff --git a/test/integration/test_exit_outputs.cpp b/test/integration/test_exit_outputs.cpp new file mode 100644 index 0000000..122d019 --- /dev/null +++ b/test/integration/test_exit_outputs.cpp @@ -0,0 +1,199 @@ +// Regression tests: a block's outputs leave in the registers its --exit +// names. +// +// The register allocator walks the program path by path. The path that +// takes a conditional branch is a copy of the state before it, and the copy +// was made without the block's outputs, so a value assigned before the +// branch never got its out_vf register on that path. And an output's live +// range ended at its last access instead of at the exit, so a temporary +// could take its register before the program ended. + +#include "test_harness.h" +#include "openvcl_runner.h" + +#include +#include +#include + +namespace +{ + // fOut is assigned before the branch, and again on the path that does + // not take it. + const char* kBranchedOutput = + "\t.init_vf_all\n" + "\t.init_vi_all\n" + "\t.syntax new\n" + "\t.name vsmExitOutputs\n" + "\t--enter\n" + "\tin_vi iFlag(vi01)\n" + "\tin_vf fA(vf01)\n" + "\t--endenter\n" + "\tmove fOut, fA\n" + "\tibeq iFlag, vi00, lDone\n" + "\tadd fOut, fA, fA\n" + "lDone:\n" + "\t--exit\n" + "\tout_vf fOut(vf05)\n" + "\t--endexit\n"; + + // fT is needed after fOut's last access, while fOut waits for the exit. + const char* kOutputBeforeTemporary = + "\t.init_vf vf01, vf05, vf06\n" + "\t.init_vi_all\n" + "\t.syntax new\n" + "\t.name vsmExitOutputs\n" + "\t--enter\n" + "\tin_vf fA(vf01)\n" + "\t--endenter\n" + "\tadd fOut, fA, fA\n" + "\tadd fT, fA, fA\n" + "\tsq fT, 0(vi00)\n" + "\t--exit\n" + "\tout_vf fOut(vf05)\n" + "\t--endexit\n"; + + const char* kIntegerOutputBeforeTemporary = + "\t.init_vf_all\n" + "\t.init_vi vi01, vi02, vi03\n" + "\t.syntax new\n" + "\t.name vsmExitOutputs\n" + "\t--enter\n" + "\tin_vi iA(vi01)\n" + "\t--endenter\n" + "\tiaddiu iOut, iA, 1\n" + "\tiaddiu iT, iA, 2\n" + "\tisw.x iT, 0(vi00)\n" + "\t--exit\n" + "\tout_vi iOut(vi02)\n" + "\t--endexit\n"; + + // fA goes from the entry to the exit untouched, in VF01. + const char* kPassedThrough = + "\t.init_vf vf01, vf02\n" + "\t.init_vi_all\n" + "\t.syntax new\n" + "\t.name vsmExitOutputs\n" + "\t--enter\n" + "\tin_vf fA(vf01)\n" + "\t--endenter\n" + "\tlq fT, 0(vi00)\n" + "\tsq fT, 1(vi00)\n" + "\t--exit\n" + "\tout_vf fA(vf01)\n" + "\t--endexit\n"; + + // fA enters in VF01 and is wanted in VF05 at the exit; nothing moves it. + const char* kMismatchedRegisters = + "\t.init_vf_all\n" + "\t.init_vi_all\n" + "\t.syntax new\n" + "\t.name vsmExitOutputs\n" + "\t--enter\n" + "\tin_vf fA(vf01)\n" + "\t--endenter\n" + "\tlq fT, 0(vi00)\n" + "\tsq fT, 1(vi00)\n" + "\t--exit\n" + "\tout_vf fA(vf05)\n" + "\t--endexit\n"; + + ::test::RunResult compile(const char* source) + { + return ::test::run_openvcl(std::vector(), source); + } + + // The emitted instructions, upper and lower halves apart: code lines are + // indented by 20 spaces, and padding separates the two halves. + std::vector instructions(const std::string& vsm) + { + std::vector result; + std::stringstream stream(vsm); + std::string line; + while (std::getline(stream, line)) + { + if (line.compare(0, 20, std::string(20, ' ')) != 0) + continue; + std::string::size_type start = line.find_first_not_of(' '); + while (start != std::string::npos) + { + const std::string::size_type end = line.find(" ", start); + result.push_back(line.substr(start, end == std::string::npos ? std::string::npos : end - start)); + start = (end == std::string::npos) ? end : line.find_first_not_of(' ', end); + } + } + return result; + } + + // The first instruction with this opcode. + std::string find(const std::vector& code, const std::string& opcode) + { + for (std::vector::const_iterator i = code.begin(); i != code.end(); ++i) + { + if (i->compare(0, opcode.size() + 1, opcode + " ") == 0) + return *i; + } + return std::string(); + } + + bool contains(const std::vector& code, const std::string& instruction) + { + for (std::vector::const_iterator i = code.begin(); i != code.end(); ++i) + { + if (*i == instruction) + return true; + } + return false; + } + + // The register an instruction names first: what it writes, or what a + // store stores. + std::string firstRegister(const std::string& instruction) + { + const std::string::size_type space = instruction.find(' '); + return space == std::string::npos ? std::string() : instruction.substr(space + 1, 4); + } +} + +TEST_CASE("openvcl: an output assigned before a taken branch gets its register") +{ + ::test::RunResult result = compile(kBranchedOutput); + REQUIRE(result.exit_code == 0); + + const std::vector code = instructions(result.stdout_data); + CHECK(contains(code, "move VF05, VF01")); + CHECK(contains(code, "add.xyzw VF05, VF01, VF01")); +} + +TEST_CASE("openvcl: an output keeps its register until the exit") +{ + ::test::RunResult result = compile(kOutputBeforeTemporary); + REQUIRE(result.exit_code == 0); + + std::vector code = instructions(result.stdout_data); + CHECK(contains(code, "add.xyzw VF05, VF01, VF01")); + CHECK(firstRegister(find(code, "sq")) == "VF06"); + + result = compile(kIntegerOutputBeforeTemporary); + REQUIRE(result.exit_code == 0); + + code = instructions(result.stdout_data); + CHECK(contains(code, "iaddiu VI02, VI01, 1")); + CHECK(firstRegister(find(code, "isw.x")) == "VI03"); +} + +TEST_CASE("openvcl: an input passed through to an output keeps its register") +{ + ::test::RunResult result = compile(kPassedThrough); + REQUIRE(result.exit_code == 0); + + const std::vector code = instructions(result.stdout_data); + CHECK(firstRegister(find(code, "lq")) == "VF02"); + CHECK(firstRegister(find(code, "sq")) == "VF02"); +} + +TEST_CASE("openvcl: an output that wants an input's value in another register is an error") +{ + ::test::RunResult result = compile(kMismatchedRegisters); + CHECK(result.exit_code != 0); + CHECK(result.stderr_data.find("error: Register conflict: fA has to be in both VF01 and VF05") != std::string::npos); +}