From 07817117a388185c609fc526f0cc0e52b6f2f7ac Mon Sep 17 00:00:00 2001 From: Guilherme Lampert Date: Wed, 21 Oct 2026 23:40:34 +1100 Subject: [PATCH] Keep live ranges inside the program when a branch ends it processBranchState ends each path's stretch of the program at the token after a branch. When the branch was the program's last token, as in the ps2gl programs that loop back with b and have no --exit block, that token was the end of the token list. Dereferencing it read past the list, and the ranges of values live across the branch ended at whatever number was there: in general.vcl, two ranges ended near 1.8 billion, a different number on each run. On macOS that number is always past every line, so the allocation came out the same, but the read is undefined behaviour and what it finds depends on the standard library and the memory layout. End the stretch at the branch itself when nothing follows it. Co-Authored-By: Claude Opus 5.5 --- src/RegisterAllocator.cpp | 12 ++-- test/CMakeLists.txt | 1 + .../test_trailing_branch_ranges.cpp | 65 +++++++++++++++++++ 3 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 test/integration/test_trailing_branch_ranges.cpp diff --git a/src/RegisterAllocator.cpp b/src/RegisterAllocator.cpp index 4d0507e..5957c6f 100644 --- a/src/RegisterAllocator.cpp +++ b/src/RegisterAllocator.cpp @@ -457,13 +457,17 @@ bool RegisterAllocator::processBranchState( BranchState* state, std::list std::list::iterator target; + // the path runs up to the token after the branch, or to the branch + // itself when it ends the program + Token& after = (curr != end) ? *curr : token; + if( token.operand()->flags() & Operand::DYNAMIC ) { if( !state->address( *branchDest, target, m_labels ) ) continue; unsigned int targetLine = (*target).lineNumber(); - unsigned int currentLine = (*curr).lineNumber(); + unsigned int currentLine = after.lineNumber(); bool isBackwardBranch = targetLine < currentLine; if( state->isBranchTaken( &*target ) ) @@ -490,7 +494,7 @@ bool RegisterAllocator::processBranchState( BranchState* state, std::list BranchState* newState = new BranchState( *state ); assert( newState ); - newState->pushTraces( &*curr, false ); + newState->pushTraces( &after, false ); newState->pushTraces( &*target, true ); newState->setCurrent( target ); @@ -505,7 +509,7 @@ bool RegisterAllocator::processBranchState( BranchState* state, std::list { // Backward branch (loop back-edge) - extend live ranges to cover loop body unsigned int targetLine = (*target).lineNumber(); - unsigned int currentLine = (*curr).lineNumber(); + unsigned int currentLine = after.lineNumber(); if( targetLine < currentLine ) state->extendLiveRanges( targetLine, currentLine ); break; @@ -513,7 +517,7 @@ bool RegisterAllocator::processBranchState( BranchState* state, std::list state->storeBranch( &*target ); - state->pushTraces( &*curr, false ); + state->pushTraces( &after, false ); state->pushTraces( &*target, true ); curr = target; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index df3c04e..1e956f3 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -79,6 +79,7 @@ set(OPENVCL_TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_instruction_info_dump.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_pairing.cpp ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_literal_register_conflict.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_trailing_branch_ranges.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 diff --git a/test/integration/test_trailing_branch_ranges.cpp b/test/integration/test_trailing_branch_ranges.cpp new file mode 100644 index 0000000..4387535 --- /dev/null +++ b/test/integration/test_trailing_branch_ranges.cpp @@ -0,0 +1,65 @@ +// Regression test: a branch that ends the program keeps live ranges inside +// the program. +// +// The register allocator ends a path's stretch of the program at the token +// after a branch. When the branch was the program's last token, as in +// ps2gl's programs that loop back with `b` and have no --exit block, that +// token was the end of the token list: dereferencing it read whatever lay +// past the list, and the ranges of values live across the branch ended at +// that number. + +#include "test_harness.h" +#include "openvcl_runner.h" + +#include +#include +#include + +namespace +{ + // fA and iN are live across the b on the last line, line 11. + const char* kTrailingBranch = + "\t.init_vf_all\n" + "\t.init_vi_all\n" + "\t.syntax new\n" + "\t--enter\n" + "\t--endenter\n" + "\tlq fA, 0(vi00)\n" + "\tiaddiu iN, vi00, 1\n" + "lLoop:\n" + "\tsq fA, 1(iN)\n" + "\tiaddiu iN, iN, 1\n" + "\tb lLoop\n"; + + const unsigned long kLastLine = 11; + + // The ends of the [start-stop] ranges --show-reg-alloc prints. + std::vector rangeEnds(const std::string& report) + { + std::vector result; + std::string::size_type open = report.find('['); + while (open != std::string::npos) + { + const std::string::size_type dash = report.find('-', open); + const std::string::size_type close = report.find(']', open); + if (dash != std::string::npos && close != std::string::npos && dash < close + && report.find_first_not_of("0123456789", open + 1) == dash) + result.push_back(std::strtoul(report.c_str() + dash + 1, NULL, 10)); + open = report.find('[', open + 1); + } + return result; + } +} + +TEST_CASE("openvcl: values live across a branch that ends the program keep ranges inside it") +{ + std::vector args; + args.push_back("--show-reg-alloc"); + ::test::RunResult result = ::test::run_openvcl(args, kTrailingBranch); + REQUIRE(result.exit_code == 0); + + const std::vector ends = rangeEnds(result.stderr_data); + REQUIRE(!ends.empty()); + for (unsigned int i = 0; i < ends.size(); ++i) + CHECK(ends[i] <= kLastLine); +}