From ff25bee2e77e9d2867dff89222d358a3f505897b Mon Sep 17 00:00:00 2001 From: Guilherme Lampert Date: Tue, 20 Oct 2026 23:07:12 +1100 Subject: [PATCH] Keep values carried around a loop live past its inner branches The register allocator walks each path through a code block, and stops a path at a branch the path has already taken. A loop's second trip is never walked past its first inner branch, so a value written in one trip and read after that branch in the next has no range around the back edge, and a later temporary can take its register: iaddiu VI01, VI01, 1 ; iPtr, read again after the inner branch iaddiu VI01, VI00, 7 ; iT2 Values loaded before a --cont loop and read after an inner branch lose their registers the same way. Tyra's static pipeline programs load their GIF tags once and continue with MSCNT for each further buffer, so from the second buffer on the tags were overwritten by vertex colours. After the walk, follow each block's control flow back from every read to the writes that reach it, lane by lane, and join them into one alias live over the tokens in between, as a complete walk would have. Where the walk was complete this adds nothing, and the code is unchanged. Co-Authored-By: Claude Opus 5.5 --- src/RegisterAllocator.cpp | 340 ++++++++++++++++++++++++ src/RegisterAllocator.h | 8 + test/CMakeLists.txt | 1 + test/integration/test_loop_liveness.cpp | 244 +++++++++++++++++ 4 files changed, 593 insertions(+) create mode 100644 test/integration/test_loop_liveness.cpp diff --git a/src/RegisterAllocator.cpp b/src/RegisterAllocator.cpp index 4d0507e..4746568 100644 --- a/src/RegisterAllocator.cpp +++ b/src/RegisterAllocator.cpp @@ -250,7 +250,17 @@ bool RegisterAllocator::process( std::list& tokens ) return false; } + // one entry state per code block so far; record where each block starts + // before processing moves the states on + std::vector< std::pair< std::list::iterator, BranchState* > > blocks; + std::list::iterator j; + for( j = m_states.begin(); j != m_states.end(); j++ ) + { + if( *j ) + blocks.push_back( std::make_pair( (*j)->current(), *j ) ); + } + for( j = m_states.begin(); j != m_states.end(); j++ ) { if( !(*j) ) @@ -262,6 +272,12 @@ bool RegisterAllocator::process( std::list& tokens ) return false; } + for( std::vector< std::pair< std::list::iterator, BranchState* > >::iterator b = blocks.begin(); b != blocks.end(); ++b ) + { + if( !completeLiveRanges( b->first, b->second->exitPoint(), b->second ) ) + return false; + } + collectLiteralRegisterUsage( tokens ); extendContinuationLiveRanges( tokens ); extendLoopDirectiveLiveRanges( tokens ); @@ -535,6 +551,330 @@ bool RegisterAllocator::processBranchState( BranchState* state, std::list /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +namespace +{ + // A read or write of a named VF or VI register in a code block. + struct VariableAccess + { + unsigned int node; // the token's position in the block + unsigned int lanes; // the fields of a VF access; 1 for VI + Dependency* dependency; // NULL where processBranchState never got to + }; + + struct VariableAccesses + { + std::vector reads; + std::vector writes; + }; + + typedef std::pair Variable; + + // A read, the writes that reach it, and the block's tokens in between. + struct ReachedRead + { + std::string name; + Dependency* read; + std::vector writes; + std::vector live; + }; + + bool sameRegister( const Token::Argument& a, const Token::Argument& b ) + { + if( a.content() != b.content() ) + return false; + if( a.content() == Token::Argument::ALIAS ) + return a.alias() == b.alias(); + return a.regNumber() == b.regNumber(); + } + + void reportRegisterConflict( const std::string& name, const Register* a, const Register* b, const Token& token ) + { + Error::Display( Error( "Register conflict: " + name + " has to be in both " + a->name() + " and " + b->name(), token ) ); + } + + bool joinWebs( Dependency* use, Dependency* def, const std::string& name ) + { + Alias* useAlias = use->alias(); + Alias* defAlias = def->alias(); + if( useAlias == defAlias ) + return true; + + if( useAlias->allocatedRegister() && defAlias->allocatedRegister() && (useAlias->allocatedRegister() != defAlias->allocatedRegister()) ) + { + reportRegisterConflict( name, useAlias->allocatedRegister(), defAlias->allocatedRegister(), use->token() ); + return false; + } + + use->depend( def ); + return true; + } +} + +bool RegisterAllocator::completeLiveRanges( std::list::iterator begin, std::list::iterator end, BranchState* entry ) +{ + std::vector< std::list::iterator > nodes; + std::map< const Token*, unsigned int > index; + for( std::list::iterator t = begin; t != end; ++t ) + { + index[ &*t ] = static_cast( nodes.size() ); + nodes.push_back( t ); + } + const unsigned int count = static_cast( nodes.size() ); + if( !count ) + return true; + + // control flow: fall-through, branches to labels, and jr/jalr to the + // instruction after each bal/jalr that sets their register + + std::vector< std::pair > returns; + for( unsigned int n = 0; n + 1 < count; ++n ) + { + const Token& token = *nodes[n]; + if( !token.operand() || token.operand()->unit() != Operand::BRU ) + continue; + for( std::list::const_iterator a = token.arguments().begin(); a != token.arguments().end(); ++a ) + { + if( (*a).flags() & Token::Argument::ADDRESS ) + returns.push_back( std::make_pair( &*a, n + 1 ) ); + } + } + + std::vector< std::vector > successors( count ); + for( unsigned int n = 0; n < count; ++n ) + { + const Token& token = *nodes[n]; + bool fallsThrough = true; + + if( token.operand() && token.operand()->unit() == Operand::BRU ) + { + fallsThrough = (token.operand()->flags() & Operand::DYNAMIC) != 0; + + for( std::list::const_iterator a = token.arguments().begin(); a != token.arguments().end(); ++a ) + { + if( !((*a).flags() & Token::Argument::BRANCH) ) + continue; + + if( (*a).type() == Token::Argument::IMMEDIATE ) + { + std::map< std::string, std::list::iterator >::const_iterator label = m_labels.find( (*a).immediate() ); + if( label == m_labels.end() ) + continue; + std::map< const Token*, unsigned int >::const_iterator target = index.find( &*label->second ); + if( target != index.end() ) + successors[n].push_back( target->second ); + } + else if( (*a).type() == Token::Argument::INTEGER_REGISTER ) + { + for( unsigned int r = 0; r < returns.size(); ++r ) + { + if( sameRegister( *returns[r].first, *a ) ) + successors[n].push_back( returns[r].second ); + } + } + } + } + + if( fallsThrough && n + 1 < count ) + successors[n].push_back( n + 1 ); + } + + std::vector< std::vector > predecessors( count ); + for( unsigned int n = 0; n < count; ++n ) + { + for( unsigned int s = 0; s < successors[n].size(); ++s ) + predecessors[ successors[n][s] ].push_back( n ); + } + + // every read and write of a named register, by variable + + std::map< Variable, VariableAccesses > variables; + for( unsigned int n = 0; n < count; ++n ) + { + for( std::list::const_iterator a = nodes[n]->arguments().begin(); a != nodes[n]->arguments().end(); ++a ) + { + if( (*a).content() != Token::Argument::ALIAS ) + continue; + if( ((*a).type() != Token::Argument::FLOAT_REGISTER) && ((*a).type() != Token::Argument::INTEGER_REGISTER) ) + continue; + + const bool isFloat = ((*a).type() == Token::Argument::FLOAT_REGISTER); + + // a broadcast named by the opcode (maddaw, mulz) leaves its operand + // without fields: it reads the token's broadcast lane + unsigned int lanes = 1; + if( isFloat ) + { + lanes = (*a).fields(); + if( !lanes && ((*a).flags() & Token::Argument::BROADCAST) ) + lanes = nodes[n]->broadcast(); + if( !lanes ) + lanes = Token::X|Token::Y|Token::Z|Token::W; + } + + VariableAccess access; + access.node = n; + access.lanes = lanes; + access.dependency = (*a).dependency(); + + VariableAccesses& accesses = variables[ Variable( isFloat ? Alias::FLOAT : Alias::INTEGER, (*a).alias() ) ]; + if( (*a).flags() & Token::Argument::WRITE ) + accesses.writes.push_back( access ); + else if( access.dependency ) + accesses.reads.push_back( access ); + } + } + + std::vector reached; + std::map< Variable, std::vector > inputReads; + + for( std::map< Variable, VariableAccesses >::const_iterator v = variables.begin(); v != variables.end(); ++v ) + { + const std::vector& writes = v->second.writes; + const bool isInput = (entry->inputRegister( v->first.first, v->first.second ) != NULL); + + // where a write, or the block entry for an input, may have set the variable + + std::vector set( count, false ); + std::vector work; + for( unsigned int w = 0; w < writes.size(); ++w ) + work.push_back( writes[w].node ); + if( isInput ) + work.push_back( 0 ); + while( !work.empty() ) + { + const unsigned int n = work.back(); + work.pop_back(); + if( set[n] ) + continue; + set[n] = true; + for( unsigned int s = 0; s < successors[n].size(); ++s ) + work.push_back( successors[n][s] ); + } + + for( std::vector::const_iterator r = v->second.reads.begin(); r != v->second.reads.end(); ++r ) + { + // back from the read until writes cover the lanes it reads + + std::vector followed( count, 0 ); + std::vector reaching( writes.size(), false ); + 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 ) ); + + while( !back.empty() ) + { + const unsigned int n = back.back().first; + unsigned int lanes = back.back().second & ~followed[n]; + back.pop_back(); + if( !lanes ) + continue; + followed[n] |= lanes; + + unsigned int written = 0; + for( unsigned int w = 0; w < writes.size(); ++w ) + { + if( (writes[w].node == n) && (writes[w].lanes & lanes) ) + { + reaching[w] = true; + written |= writes[w].lanes; + } + } + lanes &= ~written; + if( !lanes ) + continue; + + if( n == 0 ) + reachesEntry = true; + for( unsigned int p = 0; p < predecessors[n].size(); ++p ) + back.push_back( std::make_pair( predecessors[n][p], lanes ) ); + } + + ReachedRead read; + read.name = v->first.second; + read.read = (*r).dependency; + for( unsigned int w = 0; w < writes.size(); ++w ) + { + if( reaching[w] && writes[w].dependency ) + read.writes.push_back( writes[w].dependency ); + } + // a path from the entry without a write leaves the variable unset + // there: that stretch is not part of its range + for( unsigned int n = 0; n < count; ++n ) + { + if( (n == (*r).node) || (followed[n] && set[n]) ) + read.live.push_back( n ); + } + reached.push_back( read ); + + if( reachesEntry && isInput ) + inputReads[ v->first ].push_back( (*r).dependency ); + } + } + + // join each read with the writes that reach it, and the reads of an input + // that see its value from the entry + + for( std::vector::const_iterator r = reached.begin(); r != reached.end(); ++r ) + { + for( std::vector::const_iterator w = (*r).writes.begin(); w != (*r).writes.end(); ++w ) + { + if( !joinWebs( (*r).read, *w, (*r).name ) ) + return false; + } + } + + for( std::map< Variable, std::vector >::const_iterator i = inputReads.begin(); i != inputReads.end(); ++i ) + { + const std::vector& reads = i->second; + for( unsigned int k = 1; k < reads.size(); ++k ) + { + if( !joinWebs( reads[0], reads[k], i->first.second ) ) + return false; + } + + const Register* input = entry->inputRegister( i->first.first, i->first.second ); + Alias* alias = reads[0]->alias(); + if( !alias->allocatedRegister() ) + alias->setAllocatedRegister( input ); + else if( alias->allocatedRegister() != input ) + { + reportRegisterConflict( i->first.second, alias->allocatedRegister(), input, reads[0]->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 ) + { + Alias* alias = (*r).read->alias(); + const std::vector& live = (*r).live; + for( unsigned int k = 0; k < live.size(); ++k ) + { + unsigned int start = nodes[ live[k] ]->lineNumber(); + unsigned int stop = start; + + // no other token sits between two live tokens in a row + if( (k + 1 < live.size()) && (live[k + 1] == live[k] + 1) ) + { + const unsigned int next = nodes[ live[k + 1] ]->lineNumber(); + if( next < start ) + start = next; + else + stop = next; + } + + alias->addRange( start, stop ); + } + } + + return true; +} + +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + bool RegisterAllocator::collectLabels( std::list::iterator start, std::list::iterator end ) { m_labels.clear(); diff --git a/src/RegisterAllocator.h b/src/RegisterAllocator.h index 0da00da..b72a041 100644 --- a/src/RegisterAllocator.h +++ b/src/RegisterAllocator.h @@ -86,6 +86,14 @@ class RegisterAllocator bool collectLabels( std::list::iterator start, std::list::iterator end ); bool processBranchState( BranchState* state, std::list::iterator end ); + // processBranchState stops a path at a branch the path has already taken, + // so a loop's second trip is never walked past its first inner branch, and + // 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. + bool completeLiveRanges( std::list::iterator begin, std::list::iterator end, BranchState* entry ); + bool processCommonDirective( Token& token ); bool processAliases( Alias*& unallocated ); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index df3c04e..731b8bd 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -83,6 +83,7 @@ set(OPENVCL_TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_emit_source.cpp ${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_vsm_cost.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_software_pipeline.cpp ) diff --git a/test/integration/test_loop_liveness.cpp b/test/integration/test_loop_liveness.cpp new file mode 100644 index 0000000..77da59e --- /dev/null +++ b/test/integration/test_loop_liveness.cpp @@ -0,0 +1,244 @@ +// Regression tests: values live around a loop keep their registers. +// +// The register allocator walks the program path by path, and stops a path +// at a branch the path has already taken. A loop's second trip was never +// walked past its first inner branch, so a value written in one trip and +// read after that branch in the next lost its register there, and a later +// temporary could take it. Values loaded before a --cont loop and read +// after an inner branch lost theirs the same way, which Tyra's programs do. + +#include "test_harness.h" +#include "openvcl_runner.h" + +#include +#include +#include + +namespace +{ + // iPtr is read after the inner branch, then advanced for the next trip. + const char* kLoopPointer = + "\t.init_vf_all\n" + "\t.init_vi_all\n" + "\t.syntax new\n" + "\t.name vsmLoopLiveness\n" + "\t--enter\n" + "\t--endenter\n" + "\tiaddiu iPtr, vi00, 100\n" + "\tiaddiu iCount, vi00, 10\n" + "lLoop:\n" + "\tilw.x iCond, 0(iCount)\n" + "\tibeq iCond, vi00, lSkip\n" + "\tiaddiu iTemp, vi00, 5\n" + "\tisw.x iTemp, 1(vi00)\n" + "lSkip:\n" + "\tisw.x iCount, 0(iPtr)\n" + "\tiaddiu iPtr, iPtr, 1\n" + "\tiaddiu iT2, vi00, 7\n" + "\tiaddiu iT3, iT2, 1\n" + "\tisw.x iT3, 2(vi00)\n" + "\tiaddi iCount, iCount, -1\n" + "\tibne iCount, vi00, lLoop\n" + "\t--exit\n" + "\t--endexit\n"; + + // fTag is loaded once; each call continued after --cont runs from begin + // again, and stores fTag after the inner branch. + const char* kContinuedLoop = + "\t.init_vf_all\n" + "\t.init_vi_all\n" + "\t.syntax new\n" + "\t.name vsmLoopLiveness\n" + "\t--enter\n" + "\t--endenter\n" + "\tlq fTag, 0(vi00)\n" + "\tilw.x iMode, 1(vi00)\n" + "begin:\n" + "\txtop iBuf\n" + "\tiblez iMode, lColor\n" + "\tlq fC, 0(iBuf)\n" + "\tb lStore\n" + "lColor:\n" + "\tlq fC, 1(iBuf)\n" + "lStore:\n" + "\tsq fTag, 2(iBuf)\n" + "\tlq fA, 3(iBuf)\n" + "\tadd fB, fA, fC\n" + "\tsq fB, 4(iBuf)\n" + "\txgkick iBuf\n" + "\t--barrier\n" + "\t--cont\n" + "\tb begin\n" + "\t--exit\n" + "\t--endexit\n"; + + // iVal is set in the first loop and read in every trip of the second. + const char* kSiblingLoops = + "\t.init_vi vi01, vi02, vi03, vi04\n" + "\t.init_vf_all\n" + "\t.syntax new\n" + "\t.name vsmLoopLiveness\n" + "\t--enter\n" + "\t--endenter\n" + "\tiaddiu iCount, vi00, 3\n" + "lA:\n" + "\tiaddiu iVal, iCount, 10\n" + "\tiaddi iCount, iCount, -1\n" + "\tibne iCount, vi00, lA\n" + "\tiaddiu iCount2, vi00, 3\n" + "lB:\n" + "\tilw.x iCond, 0(iCount2)\n" + "\tibeq iCond, vi00, lBSkip\n" + "\tiaddiu iTmp, vi00, 1\n" + "\tisw.x iTmp, 0(vi00)\n" + "lBSkip:\n" + "\tisw.x iVal, 1(vi00)\n" + "\tiaddiu iTmp2, vi00, 5\n" + "\tiaddiu iTmp3, iTmp2, 5\n" + "\tisw.x iTmp3, 2(vi00)\n" + "\tiaddi iCount2, iCount2, -1\n" + "\tibne iCount2, vi00, lB\n" + "\t--exit\n" + "\t--endexit\n"; + + // mulw reads only the w lane of fT, which mul.w has just written: fT is + // not live around the loop, and three VF registers are enough. + const char* kBroadcastLane = + "\t.init_vf vf01, vf02, vf03\n" + "\t.init_vi_all\n" + "\t.name vsmLoopLiveness\n" + "\t--enter\n" + "\t--endenter\n" + "\tiaddiu iN, vi00, 4\n" + "\tlq fA, 0(vi00)\n" + "lLoop:\n" + "\tlq fB, 1(vi00)\n" + "\tmul.w fT, fB, fB\n" + "\tmulw.xyz fC, fA, fT\n" + "\tsq.xyz fC, 2(vi00)\n" + "\tlq fD, 3(vi00)\n" + "\tadd fE, fD, fA\n" + "\tsq fE, 4(vi00)\n" + "\tiaddi iN, iN, -1\n" + "\tibne iN, vi00, lLoop\n" + "\t--exit\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 that contains `part`. + std::string find(const std::vector& code, const std::string& opcode, const std::string& part) + { + for (std::vector::const_iterator i = code.begin(); i != code.end(); ++i) + { + if (i->compare(0, opcode.size() + 1, opcode + " ") == 0 && i->find(part) != std::string::npos) + return *i; + } + return std::string(); + } + + // 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); + } + + // The register of an address operand, as in 0(VI01). + std::string addressRegister(const std::string& instruction) + { + const std::string::size_type open = instruction.rfind('('); + return open == std::string::npos ? std::string() : instruction.substr(open + 1, 4); + } + + std::vector mentioning(const std::vector& code, const std::string& reg) + { + std::vector result; + for (std::vector::const_iterator i = code.begin(); i != code.end(); ++i) + { + if (i->find(reg) != std::string::npos) + result.push_back(*i); + } + return result; + } +} + +TEST_CASE("openvcl: a pointer carried around a loop keeps its register past an inner branch") +{ + ::test::RunResult result = compile(kLoopPointer); + REQUIRE(result.exit_code == 0); + const std::vector code = instructions(result.stdout_data); + + // isw.x iCount, 0(iPtr), and iaddiu iT2, vi00, 7 + const std::string pointer = addressRegister(find(code, "isw.x", ", 0(")); + const std::string temporary = firstRegister(find(code, "iaddiu", ", VI00, 7")); + REQUIRE(pointer.size() == 4); + REQUIRE(temporary.size() == 4); + + CHECK(temporary != pointer); + CHECK(!find(code, "iaddiu", pointer + ", " + pointer + ", 1").empty()); +} + +TEST_CASE("openvcl: a value loaded before a --cont loop keeps its register through the loop") +{ + ::test::RunResult result = compile(kContinuedLoop); + REQUIRE(result.exit_code == 0); + const std::vector code = instructions(result.stdout_data); + + // only lq fTag and sq fTag name its register + const std::string tag = firstRegister(find(code, "lq", ", 0(VI00)")); + REQUIRE(tag.size() == 4); + + const std::vector uses = mentioning(code, tag); + REQUIRE(uses.size() == 2); + CHECK(uses[0].compare(0, 3, "lq ") == 0); + CHECK(uses[1].compare(0, 3, "sq ") == 0); +} + +TEST_CASE("openvcl: a value set in one loop keeps its register through a sibling loop") +{ + ::test::RunResult result = compile(kSiblingLoops); + REQUIRE(result.exit_code == 0); + const std::vector code = instructions(result.stdout_data); + + // only iaddiu iVal and isw.x iVal name its register + const std::string value = firstRegister(find(code, "iaddiu", ", 10")); + REQUIRE(value.size() == 4); + + const std::vector uses = mentioning(code, value); + REQUIRE(uses.size() == 2); + CHECK(uses[0].compare(0, 7, "iaddiu ") == 0); + CHECK(uses[1].compare(0, 6, "isw.x ") == 0); +} + +TEST_CASE("openvcl: a broadcast operand keeps only its lane live") +{ + ::test::RunResult result = compile(kBroadcastLane); + CHECK(result.exit_code == 0); +}