Keep a block's outputs in the registers its --exit names until the exit.

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<std::string,unsigned int>& floatOutputs() const;
+	const std::map<std::string,unsigned int>& integerOutputs() const;
+
 	BranchState* clone();
 
 private:

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<std::string,unsigned int>& BranchState::floatOutputs() const
+{
+	return m_floatOutputs;
+}
+
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
+inline const std::map<std::string,unsigned int>& BranchState::integerOutputs() const
+{
+	return m_integerOutputs;
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

src/RegisterAllocator.cpp

@@ -581,6 +581,46 @@ namespace
 		std::vector<unsigned int> 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<std::string,unsigned int>& outputs,
+	                   std::list<Token>::iterator exitToken, unsigned int exit, std::map< Variable, VariableAccesses >& variables, std::vector<ExitRead>& exitReads )
+	{
+		for( std::map<std::string,unsigned int>::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<Token>::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<ExitRead> 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<unsigned int> exitPredecessors( 1, count - 1 );
+
 	std::vector<ReachedRead> reached;
 	std::map< Variable, std::vector<Dependency*> > inputReads;
 
@@ -875,8 +926,9 @@ bool RegisterAllocator::completeLiveRanges( std::list<Token>::iterator begin, st
 			bool reachesEntry = ((*r).node == 0);
 
 			std::vector< std::pair<unsigned int, unsigned int> > 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<unsigned int>& 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<Token>::iterator begin, st
 		}
 	}
 
+	// each output leaves in the register its out_vf or out_vi names
+
+	for( std::vector<ExitRead>::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<ReachedRead>::const_iterator r = reached.begin(); r != reached.end(); ++r )

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<Token>::iterator begin, std::list<Token>::iterator end, BranchState* entry );
 
 	// For each VF write that leaves lanes of its variable as they were, the

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
 )

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 <sstream>
+#include <string>
+#include <vector>
+
+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<std::string>(), source);
+    }
+
+    // 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 first instruction with this opcode.
+    std::string find(const std::vector<std::string>& code, const std::string& opcode)
+    {
+        for (std::vector<std::string>::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<std::string>& code, const std::string& instruction)
+    {
+        for (std::vector<std::string>::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<std::string> 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<std::string> 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<std::string> 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);
+}