From 8838b72894ebcca062e7b4d1a61e4fed53512618 Mon Sep 17 00:00:00 2001 From: Guilherme Lampert Date: Tue, 20 Oct 2026 17:26:59 +1100 Subject: [PATCH] Leave the code unchanged under -c, and comment every instruction -c writes each instruction's source line above it as a `; Line N:` comment. The comments went into the same list as the code, ahead of the instruction's padding, and the check for a bubble already in place before a branch (padForBranchPreBubble) looks at the previous line. It found the comment instead of the padding and added another nop pair, so -c changed the program: Quake II's textured_triangles got 5 more nop pairs with -c than without, and each of those branches had its comment above the extra nop. Instructions emitted as an upper/lower pair, or with waitq/waitp, got no comment at all (103 of the 1001 instruction lines there). The comments now live in their own list, each with the index of the code line it goes above, and write() interleaves them, so code generation only ever sees code. Each instruction line gets the comments of the source instructions on it, after any padding: both halves of a pair (upper first), an instruction issued with waitq/waitp, and a branch and its delay-slot filler. Only openvcl's own nop/waitq/waitp padding is left without one. Co-Authored-By: Claude Opus 5.5 --- src/CodeGenerator.cpp | 53 ++++--- src/CodeGenerator.h | 6 + test/CMakeLists.txt | 1 + test/integration/test_emit_source.cpp | 215 ++++++++++++++++++++++++++ 4 files changed, 252 insertions(+), 23 deletions(-) create mode 100644 test/integration/test_emit_source.cpp diff --git a/src/CodeGenerator.cpp b/src/CodeGenerator.cpp index 699abe7..761c249 100644 --- a/src/CodeGenerator.cpp +++ b/src/CodeGenerator.cpp @@ -1480,6 +1480,17 @@ void CodeGenerator::addNopLine() m_codeLines.push_back( formatRawPairedInstructionLine( vuInstr(VU_OP_NOP), vuInstr(VU_OP_NOP) ) ); } +void CodeGenerator::addSourceComment( const Token& token ) +{ + // with -c, the token's source line goes above the next code line pushed + if( !emitSource() ) + return; + + std::stringstream s; + s << " ; Line " << token.line().number() << ": " << token.line().content(); + m_sourceComments.push_back( std::make_pair( m_codeLines.size(), s.str() ) ); +} + void CodeGenerator::emitWaitQ() { const int nextCycle = m_currentCycle + 1; @@ -1513,6 +1524,7 @@ void CodeGenerator::emitUpperWithWait( const Token& token, bool waitQ ) const int readyCycle = waitQ ? m_latencyTracker.qReadyCycle() : m_latencyTracker.pReadyCycle(); const int issueCycle = (readyCycle > m_currentCycle) ? readyCycle : m_currentCycle; + addSourceComment(token); m_codeLines.push_back(outputLine); recordRegisterWrites(token, issueCycle); m_currentCycle = issueCycle + 1; @@ -1554,14 +1566,6 @@ void CodeGenerator::emitSingleToken( const Token& token ) std::string instruction = generateInstruction(token); std::string outputLine = ""; - // emit original sourcecode as a comment - if( emitSource() ) - { - std::stringstream s; - s << " ; Line " << token.line().number() << ": " << token.line().content(); - m_codeLines.push_back( s.str() ); - } - int instructionLength = 32; for(int d = 0; d < 20; d++) @@ -1596,6 +1600,7 @@ void CodeGenerator::emitSingleToken( const Token& token ) if( branchDelaySlots > 0 ) { padForBranchPreBubble( token ); + addSourceComment(token); m_codeLines.push_back(outputLine); m_currentCycle++; for( unsigned int i = 0; i < branchDelaySlots; ++i ) @@ -1607,6 +1612,7 @@ void CodeGenerator::emitSingleToken( const Token& token ) else { int issueCycle = m_currentCycle; + addSourceComment(token); m_codeLines.push_back(outputLine); m_currentCycle++; recordRegisterWrites(token, issueCycle); @@ -1622,13 +1628,6 @@ void CodeGenerator::emitBranchWithDelayFiller( const Token& branch, const Token& std::string fillerLine = ""; const int instructionLength = 32; - if( emitSource() ) - { - std::stringstream s; - s << " ; Line " << branch.line().number() << ": " << branch.line().content(); - m_codeLines.push_back( s.str() ); - } - for(int d = 0; d < 20; d++) branchLine += " "; @@ -1653,16 +1652,10 @@ void CodeGenerator::emitBranchWithDelayFiller( const Token& branch, const Token& } padForBranchPreBubble( branch ); + addSourceComment(branch); m_codeLines.push_back(branchLine); m_currentCycle++; - if( emitSource() ) - { - std::stringstream s; - s << " ; Line " << filler.line().number() << ": " << filler.line().content(); - m_codeLines.push_back( s.str() ); - } - for(int d = 0; d < 20; d++) fillerLine += " "; @@ -1686,6 +1679,7 @@ void CodeGenerator::emitBranchWithDelayFiller( const Token& branch, const Token& } } + addSourceComment(filler); m_codeLines.push_back(fillerLine); recordRegisterWrites(filler, m_currentCycle); m_currentCycle++; @@ -1705,6 +1699,8 @@ void CodeGenerator::emitPairedTokens( const Token& a, const Token& b ) padForBranchPreBubble(a); if( branchNeedsPreBubble(b) ) padForBranchPreBubble(b); + addSourceComment( tokenIsLowerExecutionPath(a) ? b : a ); + addSourceComment( tokenIsLowerExecutionPath(a) ? a : b ); m_codeLines.push_back(pairedLine); recordRegisterWrites(a, m_currentCycle); @@ -1728,6 +1724,8 @@ void CodeGenerator::emitPairedBranchWithDelayFiller( const Token& a, const Token padForBranchPreBubble(a); if( branchNeedsPreBubble(b) ) padForBranchPreBubble(b); + addSourceComment( tokenIsLowerExecutionPath(a) ? b : a ); + addSourceComment( tokenIsLowerExecutionPath(a) ? a : b ); m_codeLines.push_back(pairedLine); recordRegisterWrites(a, m_currentCycle); @@ -1760,6 +1758,7 @@ void CodeGenerator::emitPairedBranchWithDelayFiller( const Token& a, const Token } } + addSourceComment(filler); m_codeLines.push_back(fillerLine); recordRegisterWrites(filler, m_currentCycle); m_currentCycle++; @@ -10060,8 +10059,16 @@ std::string CodeGenerator::generateOperand(const Token& token) bool CodeGenerator::write(std::ostream& stream) { - for(std::list::const_iterator i = m_codeLines.begin(); i != m_codeLines.end(); ++i) + std::vector< std::pair >::const_iterator comment = m_sourceComments.begin(); + size_t index = 0; + + for(std::list::const_iterator i = m_codeLines.begin(); i != m_codeLines.end(); ++i, ++index) + { + for( ; (comment != m_sourceComments.end()) && (comment->first == index); ++comment ) + stream << comment->second << std::endl; + stream << (*i) << std::endl; + } return true; } diff --git a/src/CodeGenerator.h b/src/CodeGenerator.h index 1845035..fd8362f 100644 --- a/src/CodeGenerator.h +++ b/src/CodeGenerator.h @@ -68,6 +68,7 @@ class CodeGenerator std::string accumulatorArg( const Token::Argument& arg, const Token& token ); void addNopLine(); + void addSourceComment( const Token& token ); void emitWaitQ(); void emitWaitP(); void emitUpperWithWait( const Token& token, bool waitQ ); @@ -191,6 +192,11 @@ class CodeGenerator std::list m_codeLines; + // -c source comments, each with the index of the m_codeLines entry it + // precedes. They stay out of m_codeLines so code generation, which looks + // back at the lines already emitted, never sees them. + std::vector< std::pair > m_sourceComments; + bool m_emitSource; bool m_knownLoopOptimizations; bool m_genericSoftwarePipelining; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e1a162b..053aaa4 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -78,6 +78,7 @@ set(OPENVCL_TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_pairing.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_literal_register_conflict.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_flag_latency.cpp + ${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_vsm_cost.cpp diff --git a/test/integration/test_emit_source.cpp b/test/integration/test_emit_source.cpp new file mode 100644 index 0000000..93d5186 --- /dev/null +++ b/test/integration/test_emit_source.cpp @@ -0,0 +1,215 @@ +// Regression tests: -c annotates the code without changing it. +// +// -c writes each instruction's source line above it as a "; Line N:" +// comment. The comments went into the same list as the code, so the check +// for padding already in place before a branch found a comment instead, +// and added another nop pair: the program changed under -c, and the +// branch's comment ended up above that nop. Instructions emitted as a +// pair, or with waitq/waitp, got no comment. + +#include "test_harness.h" +#include "openvcl_runner.h" + +#include +#include +#include + +namespace +{ + // The branch reads a fresh ilw result, so latency padding precedes it. + const char* kLoadThenBranch = + "\t.init_vf_all\n" + "\t.init_vi_all\n" + "\t.syntax new\n" + "\t.name vsmEmitSource\n" + "\t--enter\n" + "\t--endenter\n" + "\tilw.x iFlag, 0(vi00)\n" + "\tibeq iFlag, vi00, skip\n" + "\tiaddiu iValue, vi00, 6\n" + "\tisw.x iValue, 1(vi00)\n" + "skip:\n" + "\tiaddiu iDone, vi00, 1\n" + "\tisw.x iDone, 2(vi00)\n" + "\t--exit\n" + "\t--endexit\n"; + + // Upper/lower pairs, a mulq issued with waitq, a move emitted as max. + const char* kPairedLoop = + "\t.init_vf_all\n" + "\t.init_vi_all\n" + "\t.syntax new\n" + "\t.name vsmEmitSource\n" + "\t--enter\n" + "\t--endenter\n" + "\tiaddiu iCount, vi00, 4\n" + "\tlq fA, 0(vi00)\n" + "\tlq fB, 1(vi00)\n" + "loop:\n" + "\tilw.x iFlag, 2(vi00)\n" + "\tadd.xyz fC, fA, fB\n" + "\tdiv q, vf00[w], fA[w]\n" + "\tmulq.xyz fD, fC, q\n" + "\tibeq iFlag, vi00, skip\n" + "\tmove.xyz fE, fD\n" + "\tsq.xyz fE, 3(vi00)\n" + "skip:\n" + "\tiaddi iCount, iCount, -1\n" + "\tsq.xyz fD, 4(vi00)\n" + "\tibne iCount, vi00, loop\n" + "\t--exit\n" + "\t--endexit\n"; + + // The store moves into the loop branch's delay slot. + const char* kDelaySlotLoop = + "\t.init_vf_all\n" + "\t.init_vi_all\n" + "\t--enter\n" + "\t--endenter\n" + "\tiaddiu vi01, vi00, 0\n" + "\tiaddiu vi02, vi00, 3\n" + "loop_lid:\n" + "\tlq.xyzw vf01, 0(vi00)\n" + "\tadd.xyzw vf02, vf01, vf01\n" + "\tsq.xyzw vf02, 0(vi01)\n" + "\tiaddiu vi01, vi01, 1\n" + "\tibne vi01, vi02, loop_lid\n" + "\t--exit\n" + "\t--endexit\n"; + + ::test::RunResult compile(const char* source, bool emitSource) + { + std::vector args; + if (emitSource) + args.push_back("-c"); + return ::test::run_openvcl(args, source); + } + + std::vector splitLines(const std::string& text) + { + std::vector result; + std::stringstream stream(text); + std::string line; + while (std::getline(stream, line)) + result.push_back(line); + return result; + } + + bool isComment(const std::string& line) + { + const std::string::size_type first = line.find_first_not_of(" \t"); + return first != std::string::npos && line[first] == ';'; + } + + std::string stripComments(const std::string& text) + { + std::string result; + const std::vector lines = splitLines(text); + for (std::vector::const_iterator i = lines.begin(); i != lines.end(); ++i) + { + if (!isComment(*i)) + result += *i + "\n"; + } + return result; + } + + // An emitted instruction pair other than nop/nop (code lines are indented + // by 20 spaces; labels and directives are not). + bool isInstruction(const std::string& line) + { + if (line.compare(0, 20, std::string(20, ' ')) != 0 || isComment(line)) + return false; + std::stringstream words(line); + std::string word; + while (words >> word) + { + if (word.compare(0, 3, "nop") != 0) + return true; + } + return false; + } + + unsigned int uncommentedInstructions(const std::string& text) + { + unsigned int count = 0; + const std::vector lines = splitLines(text); + for (std::vector::size_type i = 0; i < lines.size(); ++i) + { + if (isInstruction(lines[i]) && (i == 0 || !isComment(lines[i - 1]))) + ++count; + } + return count; + } + + // The comments directly above the first line that contains `code`. + std::vector commentsAbove(const std::string& text, const std::string& code) + { + std::vector result; + const std::vector lines = splitLines(text); + for (std::vector::size_type i = 0; i < lines.size(); ++i) + { + if (isComment(lines[i]) || lines[i].find(code) == std::string::npos) + continue; + for (std::vector::size_type j = i; j > 0 && isComment(lines[j - 1]); --j) + result.insert(result.begin(), lines[j - 1]); + break; + } + return result; + } + + bool contains(const std::string& text, const std::string& needle) + { + return text.find(needle) != std::string::npos; + } +} + +TEST_CASE("openvcl: -c leaves the padding before a branch as it is") +{ + ::test::RunResult plain = compile(kLoadThenBranch, false); + ::test::RunResult annotated = compile(kLoadThenBranch, true); + + REQUIRE(plain.exit_code == 0); + REQUIRE(annotated.exit_code == 0); + CHECK(stripComments(annotated.stdout_data) == plain.stdout_data); + CHECK(uncommentedInstructions(annotated.stdout_data) == 0); +} + +TEST_CASE("openvcl: -c comments both halves of a pair and instructions issued with waitq") +{ + ::test::RunResult plain = compile(kPairedLoop, false); + ::test::RunResult annotated = compile(kPairedLoop, true); + + REQUIRE(plain.exit_code == 0); + REQUIRE(annotated.exit_code == 0); + CHECK(stripComments(annotated.stdout_data) == plain.stdout_data); + CHECK(uncommentedInstructions(annotated.stdout_data) == 0); + + // add.xyz (upper) and ilw.x (lower) issue together + const std::vector pair = commentsAbove(annotated.stdout_data, "ilw.x VI02, 2(VI00)"); + REQUIRE(pair.size() == 2); + CHECK(contains(pair[0], "Line 12:") && contains(pair[0], "add.xyz fC, fA, fB")); + CHECK(contains(pair[1], "Line 11:") && contains(pair[1], "ilw.x iFlag, 2(vi00)")); + + const std::vector wait = commentsAbove(annotated.stdout_data, "waitq"); + REQUIRE(wait.size() == 1); + CHECK(contains(wait[0], "Line 14:") && contains(wait[0], "mulq.xyz fD, fC, q")); + + const std::vector move = commentsAbove(annotated.stdout_data, "max.xyz"); + REQUIRE(move.size() == 1); + CHECK(contains(move[0], "Line 16:") && contains(move[0], "move.xyz fE, fD")); +} + +TEST_CASE("openvcl: -c leaves a loop branch and its filled delay slot as they are") +{ + ::test::RunResult plain = compile(kDelaySlotLoop, false); + ::test::RunResult annotated = compile(kDelaySlotLoop, true); + + REQUIRE(plain.exit_code == 0); + REQUIRE(annotated.exit_code == 0); + CHECK(stripComments(annotated.stdout_data) == plain.stdout_data); + CHECK(uncommentedInstructions(annotated.stdout_data) == 0); + + const std::vector filler = commentsAbove(annotated.stdout_data, "sq VF02, -1(VI01)"); + REQUIRE(filler.size() == 1); + CHECK(contains(filler[0], "Line 10:") && contains(filler[0], "sq.xyzw vf02, 0(vi01)")); +}