Keep a pipelined loop's scratch registers off values live across it.

src/VuSchedulerAnalysis.cpp

@@ -13,6 +13,7 @@
 #include <sstream>
 #include <string>
 #include <algorithm>
+#include <bitset>
 
 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<std::string>& bases )
 	{
 		for( std::vector<const Token*>::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<std::string>::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<Token>::const_iter
 	return mask;
 }
 
+namespace
+{
+	// lane l of VFr is bit r * 4 + l
+	typedef std::bitset<128> VuVfLanes;
+
+	void addVfLanes( const std::list<std::string>& keys, VuVfLanes& lanes )
+	{
+		static const std::string names = "xyzw";
+		for( std::list<std::string>::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<unsigned int>( 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<Token::Argument>::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<const Token*>& tokens,
+	                      const std::map<std::string, unsigned int>& labels,
+	                      std::vector<VuVfLanes>& liveIn )
+	{
+		const unsigned int count = static_cast<unsigned int>( tokens.size() );
+		std::vector<VuVfLanes> reads( count );
+		std::vector<VuVfLanes> writes( count );
+		std::vector< std::pair<std::string, unsigned int> > returns;
+		for( unsigned int n = 0; n < count; ++n )
+		{
+			const Token& token = *tokens[n];
+			std::list<std::string> 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<unsigned int>( 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<unsigned int> > 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<std::string, unsigned int>::const_iterator target = labels.find( label );
+					if( target != labels.end() )
+						successors[n].push_back( target->second );
+				}
+
+				for( std::list<Token::Argument>::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<std::string>& registers )
+	{
+		for( unsigned int reg = 0; reg < 32; ++reg )
+		{
+			if( ((lanes >> (reg * 4)) & VuVfLanes( 0xf )).any() )
+				addUniqueString( registers, vfRegisterName( reg ) );
+		}
+	}
+}
+
 std::vector<VuLoopCandidate> findVuLoopCandidates( const std::list<Token>& tokens )
 {
 	std::vector<const Token*> indexedTokens;
@@ -6200,6 +6349,7 @@ std::vector<VuLoopCandidate> findVuLoopCandidates( const std::list<Token>& token
 	}
 
 	std::vector<VuLoopCandidate> result;
+	std::vector<VuVfLanes> liveIn;
 	for( unsigned int branchIndex = 0; branchIndex < indexedTokens.size(); ++branchIndex )
 	{
 		const Token& token = *indexedTokens[branchIndex];
@@ -6211,6 +6361,9 @@ std::vector<VuLoopCandidate> findVuLoopCandidates( const std::list<Token>& 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<VuLoopCandidate> findVuLoopCandidates( const std::list<Token>& 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 );
 	}
 

src/VuSchedulerAnalysis.h

@@ -161,6 +161,9 @@ struct VuLoopCandidate
 	std::list<std::string> inductionRegisters;
 	std::vector<VuLoopInductionUpdate> inductionUpdates;
 	std::list<std::string> 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<std::string> liveVfRegisters;
 	std::vector<const Token*> bodyTokens;
 	const Token* branchToken;
 };

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

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 <sstream>
+#include <string>
+#include <vector>
+
+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<std::string> instructions(const std::string& vsm)
+    {
+        std::vector<std::string> 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<std::string> writing(const std::vector<std::string>& code, const std::string& reg)
+    {
+        std::vector<std::string> result;
+        for (std::vector<std::string>::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<std::string>(), readAfterLoopSource());
+    REQUIRE(result.exit_code == 0);
+    CHECK(result.stdout_data.find("loop_lid__PROLOG:") != std::string::npos);
+
+    const std::vector<std::string> 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<std::string>(), readInOuterLoopSource());
+    REQUIRE(result.exit_code == 0);
+    CHECK(result.stdout_data.find("loop_lid__PROLOG:") != std::string::npos);
+
+    const std::vector<std::string> writes = writing(instructions(result.stdout_data), "VF31");
+    REQUIRE(writes.size() == 1);
+    CHECK(writes[0] == "lq VF31, 7(VI00)");
+}