From 95cd6d6d3a243f3858c1f8c23776db0ac03cae4f Mon Sep 17 00:00:00 2001 From: Guilherme Lampert Date: Wed, 21 Oct 2026 22:11:09 +1100 Subject: [PATCH] Keep a pipelined loop's scratch registers off values live across it The software pipeliner takes scratch registers for the values it rotates from VF31 down, skipping only the registers the loop body names. A register holding a value the body does not name, but that is read after the loop or on the next trip of an enclosing loop, counted as free, and a rotation overwrote it. In ps2gl's general program the directional light loop took the registers of material_spec and vert_to_eye, so the next light and the point lights read garbage. Work out which VF lanes are live where each loop is entered and left, over the whole program's control flow, and keep those registers out of the scratch pool along with the ones the body names. Co-Authored-By: Claude Opus 5.5 --- src/VuSchedulerAnalysis.cpp | 157 ++++++++++++++++++ src/VuSchedulerAnalysis.h | 3 + test/CMakeLists.txt | 1 + .../test_pipeline_live_registers.cpp | 138 +++++++++++++++ 4 files changed, 299 insertions(+) create mode 100644 test/integration/test_pipeline_live_registers.cpp diff --git a/src/VuSchedulerAnalysis.cpp b/src/VuSchedulerAnalysis.cpp index 4c6b82c..4db62b1 100644 --- a/src/VuSchedulerAnalysis.cpp +++ b/src/VuSchedulerAnalysis.cpp @@ -13,6 +13,7 @@ #include #include #include +#include namespace vcl { @@ -1971,6 +1972,8 @@ namespace } } + // The VF registers a pipelined loop's scratch registers must not take: the + // ones its body names, and the ones holding values live across the loop. void collectLoopVfBaseKeys( const VuLoopCandidate& loop, std::list& bases ) { for( std::vector::const_iterator i = loop.bodyTokens.begin(); i != loop.bodyTokens.end(); ++i ) @@ -1982,6 +1985,8 @@ namespace collectVfBaseKeys( reads, bases ); collectVfBaseKeys( writes, bases ); } + for( std::list::const_iterator i = loop.liveVfRegisters.begin(); i != loop.liveVfRegisters.end(); ++i ) + addUniqueString( bases, *i ); } std::string vfRegisterName( unsigned int index ) @@ -6186,6 +6191,150 @@ unsigned int vuIgnoredFlagWawResourcesForRemaining( std::list::const_iter return mask; } +namespace +{ + // lane l of VFr is bit r * 4 + l + typedef std::bitset<128> VuVfLanes; + + void addVfLanes( const std::list& keys, VuVfLanes& lanes ) + { + static const std::string names = "xyzw"; + for( std::list::const_iterator i = keys.begin(); i != keys.end(); ++i ) + { + // keys look like VF09.x + if( i->size() != 6 || i->compare( 0, 2, "VF" ) != 0 || (*i)[4] != '.' ) + continue; + const unsigned int reg = static_cast( std::atoi( i->substr( 2, 2 ).c_str() ) ); + const std::string::size_type lane = names.find( (*i)[5] ); + if( reg < 32 && lane != std::string::npos ) + lanes.set( reg * 4 + lane ); + } + } + + // bal and jalr write their return address to a register + bool linkRegisterKey( const Token& token, std::string& key ) + { + for( std::list::const_iterator a = token.arguments().begin(); a != token.arguments().end(); ++a ) + { + if( (*a).flags() & Token::Argument::ADDRESS ) + return vuRegisterKey( *a, key ); + } + return false; + } + + // The VF lanes live before each token: read on some path from it before + // they are written. Control flows on to the next token, to branch labels, + // and from jr/jalr back to the token after each bal/jalr that set their + // register; it stops at --endexit, where the block's out_vf registers are + // read. Only tokens that will be emitted count as writing. + void computeVfLiveIn( const std::vector& tokens, + const std::map& labels, + std::vector& liveIn ) + { + const unsigned int count = static_cast( tokens.size() ); + std::vector reads( count ); + std::vector writes( count ); + std::vector< std::pair > returns; + for( unsigned int n = 0; n < count; ++n ) + { + const Token& token = *tokens[n]; + std::list keys; + collectVuRegisterReadKeys( token, keys ); + addVfLanes( keys, reads[n] ); + if( isVuEmittableInstruction( token ) ) + { + keys.clear(); + collectVuRegisterWriteKeys( token, keys ); + addVfLanes( keys, writes[n] ); + } + + if( token.operand() && token.operand()->name() == "out_vf" && !token.arguments().empty() ) + { + const unsigned int reg = static_cast( token.arguments().front().regNumber() ); + for( unsigned int lane = 0; lane < 4 && reg < 32; ++lane ) + reads[n].set( reg * 4 + lane ); + } + + std::string link; + if( (n + 1 < count) && token.operand() && (token.operand()->unit() == Operand::BRU) && linkRegisterKey( token, link ) ) + returns.push_back( std::make_pair( link, n + 1 ) ); + } + + std::vector< std::vector > successors( count ); + for( unsigned int n = 0; n < count; ++n ) + { + const Token& token = *tokens[n]; + if( token.operand() && token.operand()->name() == "--endexit" ) + continue; + + bool fallsThrough = true; + if( token.operand() && token.operand()->unit() == Operand::BRU ) + { + // a conditional branch, or a call returning to the next token + std::string link; + fallsThrough = (token.operand()->flags() & Operand::DYNAMIC) || linkRegisterKey( token, link ); + + std::string label; + if( branchTargetLabel( token, label ) ) + { + std::map::const_iterator target = labels.find( label ); + if( target != labels.end() ) + successors[n].push_back( target->second ); + } + + for( std::list::const_iterator a = token.arguments().begin(); a != token.arguments().end(); ++a ) + { + std::string key; + if( !((*a).flags() & Token::Argument::BRANCH) || (*a).type() != Token::Argument::INTEGER_REGISTER || !vuRegisterKey( *a, key ) ) + continue; + + // a register no bal/jalr sets may return anywhere + bool matched = false; + for( unsigned int r = 0; r < returns.size(); ++r ) + matched = matched || (returns[r].first == key); + for( unsigned int r = 0; r < returns.size(); ++r ) + { + if( !matched || (returns[r].first == key) ) + successors[n].push_back( returns[r].second ); + } + } + } + + if( fallsThrough && (n + 1 < count) ) + successors[n].push_back( n + 1 ); + } + + liveIn.assign( count, VuVfLanes() ); + bool changed = true; + while( changed ) + { + changed = false; + for( unsigned int n = count; n-- > 0; ) + { + VuVfLanes out; + for( unsigned int s = 0; s < successors[n].size(); ++s ) + out |= liveIn[ successors[n][s] ]; + + const VuVfLanes in = reads[n] | (out & ~writes[n]); + if( in != liveIn[n] ) + { + liveIn[n] = in; + changed = true; + } + } + } + } + + void addLiveVfRegisters( const VuVfLanes& lanes, std::list& registers ) + { + for( unsigned int reg = 0; reg < 32; ++reg ) + { + if( ((lanes >> (reg * 4)) & VuVfLanes( 0xf )).any() ) + addUniqueString( registers, vfRegisterName( reg ) ); + } + } +} + std::vector findVuLoopCandidates( const std::list& tokens ) { std::vector indexedTokens; @@ -6200,6 +6349,7 @@ std::vector findVuLoopCandidates( const std::list& token } std::vector result; + std::vector liveIn; for( unsigned int branchIndex = 0; branchIndex < indexedTokens.size(); ++branchIndex ) { const Token& token = *indexedTokens[branchIndex]; @@ -6211,6 +6361,9 @@ std::vector findVuLoopCandidates( const std::list& token if( target == labels.end() || target->second >= branchIndex ) continue; + if( liveIn.empty() ) + computeVfLiveIn( indexedTokens, labels, liveIn ); + VuLoopCandidate candidate; candidate.label = label; candidate.labelTokenIndex = target->second; @@ -6231,6 +6384,10 @@ std::vector findVuLoopCandidates( const std::list& token candidate.bodyTokens.push_back( indexedTokens[body] ); analyzeLoopCandidateResources( candidate ); + addLiveVfRegisters( liveIn[candidate.labelTokenIndex], candidate.liveVfRegisters ); + if( branchIndex + 1 < indexedTokens.size() ) + addLiveVfRegisters( liveIn[branchIndex + 1], candidate.liveVfRegisters ); + result.push_back( candidate ); } diff --git a/src/VuSchedulerAnalysis.h b/src/VuSchedulerAnalysis.h index e51b7f2..79d6e63 100644 --- a/src/VuSchedulerAnalysis.h +++ b/src/VuSchedulerAnalysis.h @@ -161,6 +161,9 @@ struct VuLoopCandidate std::list inductionRegisters; std::vector inductionUpdates; std::list loopReadWriteRegisters; + // VF registers live where the loop is entered or left: the code around the + // loop still needs their values, whether or not the body names them + std::list liveVfRegisters; std::vector bodyTokens; const Token* branchToken; }; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index df3c04e..494e387 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -85,6 +85,7 @@ set(OPENVCL_TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_two_address_coalescing.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_vsm_cost.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_software_pipeline.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_pipeline_live_registers.cpp ) add_executable(openvcl_unit_tests diff --git a/test/integration/test_pipeline_live_registers.cpp b/test/integration/test_pipeline_live_registers.cpp new file mode 100644 index 0000000..f629acf --- /dev/null +++ b/test/integration/test_pipeline_live_registers.cpp @@ -0,0 +1,138 @@ +// Regression tests: a pipelined loop leaves alone the registers of values +// live across it. +// +// The software pipeliner takes scratch registers for the values it rotates +// from VF31 down, skipping the registers the loop body names. A value the +// body does not name, but that is read after the loop or on the next trip of +// an enclosing loop, lost its register to a scratch. ps2gl's general program +// lost material_spec and vert_to_eye that way in its directional light loop. + +#include "test_harness.h" +#include "openvcl_runner.h" + +#include +#include +#include + +namespace +{ + // The pipeliner rotates vf03 through a scratch register. + const char* kLoop = + "loop_lid:\n" + "\t--LoopCS 1,1\n" + "\tlq.xyzw vf01, 0(vi01)\n" + "\tmul.xyzw vf03, vf01, vf02\n" + "\tdiv q, vf00w, vf03w\n" + "\tmulq.xyz vf03, vf03, q\n" + "\tsq.xyz vf03, 0(vi02)\n" + "\tadd.xyz vf10, vf00, vf00\n" + "\tadd.xyz vf11, vf00, vf00\n" + "\tadd.xyz vf12, vf00, vf00\n" + "\tadd.xyz vf13, vf00, vf00\n" + "\tadd.xyz vf14, vf00, vf00\n" + "\tiaddiu vi01, vi01, 1\n" + "\tiaddiu vi02, vi02, 1\n" + "\tibne vi01, vi04, loop_lid\n"; + + // vf31 is set before the loop and stored after it. + std::string readAfterLoopSource() + { + return std::string( + "\t.init_vf_all\n" + "\t.init_vi_all\n" + "\t--enter\n" + "\t--endenter\n" + "\tiaddiu vi01, vi00, 0\n" + "\tiaddiu vi02, vi00, 0\n" + "\tiaddiu vi04, vi00, 3\n" + "\tmove.xyzw vf02, vf00\n" + "\tlq.xyzw vf31, 7(vi00)\n") + + kLoop + + "\tsq.xyzw vf31, 8(vi00)\n" + "\t--exit\n" + "\t--endexit\n"; + } + + // vf31 is set before the outer loop and stored on each of its trips, + // before the inner loop. + std::string readInOuterLoopSource() + { + return std::string( + "\t.init_vf_all\n" + "\t.init_vi_all\n" + "\t--enter\n" + "\t--endenter\n" + "\tlq.xyzw vf31, 7(vi00)\n" + "\tiaddiu vi05, vi00, 2\n" + "outer_lid:\n" + "\tsq.xyzw vf31, 8(vi05)\n" + "\tiaddiu vi01, vi00, 0\n" + "\tiaddiu vi02, vi00, 0\n" + "\tiaddiu vi04, vi00, 3\n" + "\tmove.xyzw vf02, vf00\n") + + kLoop + + "\tiaddi vi05, vi05, -1\n" + "\tibne vi05, vi00, outer_lid\n" + "\t--exit\n" + "\t--endexit\n"; + } + + // 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 instructions that write `reg`: it is their first operand, and they + // are not stores. + std::vector writing(const std::vector& code, const std::string& reg) + { + std::vector result; + for (std::vector::const_iterator i = code.begin(); i != code.end(); ++i) + { + const std::string::size_type space = i->find(' '); + if (space != std::string::npos && i->compare(space + 1, reg.size() + 1, reg + ",") == 0 + && i->compare(0, 2, "sq") != 0) + result.push_back(*i); + } + return result; + } +} + +TEST_CASE("Software pipeline: a value read after a pipelined loop keeps its register") +{ + ::test::RunResult result = ::test::run_openvcl(std::vector(), readAfterLoopSource()); + REQUIRE(result.exit_code == 0); + CHECK(result.stdout_data.find("loop_lid__PROLOG:") != std::string::npos); + + const std::vector writes = writing(instructions(result.stdout_data), "VF31"); + REQUIRE(writes.size() == 1); + CHECK(writes[0] == "lq VF31, 7(VI00)"); +} + +TEST_CASE("Software pipeline: a value read on each trip of an outer loop keeps its register") +{ + ::test::RunResult result = ::test::run_openvcl(std::vector(), readInOuterLoopSource()); + REQUIRE(result.exit_code == 0); + CHECK(result.stdout_data.find("loop_lid__PROLOG:") != std::string::npos); + + const std::vector writes = writing(instructions(result.stdout_data), "VF31"); + REQUIRE(writes.size() == 1); + CHECK(writes[0] == "lq VF31, 7(VI00)"); +}