From 6724d506f37ebfce8d20ebbf85430e30f79ad089 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 2 Sep 2026 00:33:32 +0000 Subject: [PATCH] Add cycle detection to instruction fingerprinting. (#7691) Use a version of Brent's algorithm, suitably adapted to work for our worklist-based graph traversal, to very cheaply detect if instruction fingerprinting fell into a cycle and terminate cleanly with a dump of the cycle. The algorithm does not immediately catch when we enter a cycle, but is guaranteed to catch it eventually (generally after running through the cycle no more than twice). --- toolchain/sem_ir/BUILD | 1 + toolchain/sem_ir/dump.cpp | 13 +++++++ toolchain/sem_ir/dump.h | 2 ++ toolchain/sem_ir/inst_fingerprinter.cpp | 46 +++++++++++++++++++++++-- 4 files changed, 59 insertions(+), 3 deletions(-) diff --git a/toolchain/sem_ir/BUILD b/toolchain/sem_ir/BUILD index c90d84eb5fb4..29ddb43f0f39 100644 --- a/toolchain/sem_ir/BUILD +++ b/toolchain/sem_ir/BUILD @@ -227,6 +227,7 @@ cc_library( "inst_namer.h", ], deps = [ + ":dump", ":file", ":typed_insts", "//common:concepts", diff --git a/toolchain/sem_ir/dump.cpp b/toolchain/sem_ir/dump.cpp index 074434ce2252..4c14eb1ba672 100644 --- a/toolchain/sem_ir/dump.cpp +++ b/toolchain/sem_ir/dump.cpp @@ -229,6 +229,19 @@ LLVM_DUMP_METHOD auto Dump(const File& file, ConstantId const_id) return out.TakeStr(); } +LLVM_DUMP_METHOD auto Dump(const File& file, CppOverloadSetId overload_set_id) + -> std::string { + RawStringOstream out; + out << overload_set_id; + if (overload_set_id.has_value()) { + const auto& overload_set = file.cpp_overload_sets().Get(overload_set_id); + out << ": " << overload_set; + // TODO: Consider also including a dump of the functions in the overload + // set. Printing the set just includes the name and parent scope. + } + return out.TakeStr(); +} + LLVM_DUMP_METHOD auto Dump(const File& file, EntityNameId entity_name_id) -> std::string { RawStringOstream out; diff --git a/toolchain/sem_ir/dump.h b/toolchain/sem_ir/dump.h index 20845b8625f5..2a3f965c88b8 100644 --- a/toolchain/sem_ir/dump.h +++ b/toolchain/sem_ir/dump.h @@ -14,6 +14,7 @@ #ifndef CARBON_TOOLCHAIN_SEM_IR_DUMP_H_ #define CARBON_TOOLCHAIN_SEM_IR_DUMP_H_ +#include "toolchain/sem_ir/ids.h" #ifndef NDEBUG #include "toolchain/sem_ir/file.h" @@ -24,6 +25,7 @@ auto Dump(const File& file) -> std::string; auto Dump(const File& file, RawBundleId bundle_id) -> std::string; auto Dump(const File& file, ClassId class_id) -> std::string; auto Dump(const File& file, ConstantId const_id) -> std::string; +auto Dump(const File& file, CppOverloadSetId overload_set_id) -> std::string; auto Dump(const File& file, EntityNameId entity_name_id) -> std::string; auto Dump(const File& file, DeclaredFacetTypeId declared_facet_type_id) -> std::string; diff --git a/toolchain/sem_ir/inst_fingerprinter.cpp b/toolchain/sem_ir/inst_fingerprinter.cpp index e9c2e590b5c4..ac23f06a1c76 100644 --- a/toolchain/sem_ir/inst_fingerprinter.cpp +++ b/toolchain/sem_ir/inst_fingerprinter.cpp @@ -11,6 +11,7 @@ #include "common/concepts.h" #include "common/ostream.h" +#include "common/raw_string_ostream.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" @@ -22,6 +23,7 @@ #include "toolchain/base/kind_switch.h" #include "toolchain/base/value_ids.h" #include "toolchain/sem_ir/cpp_overload_set.h" +#include "toolchain/sem_ir/dump.h" #include "toolchain/sem_ir/entity_with_params_base.h" #include "toolchain/sem_ir/ids.h" #include "toolchain/sem_ir/name_scope.h" @@ -673,12 +675,50 @@ struct Worklist { // Ensure all the instructions on the todo list have fingerprints. To avoid a // re-lookup, returns the fingerprint of the first instruction on the todo // list, and requires the todo list to be non-empty. + // + // To avoid runaway fingerprinting, we use a cycle detector based on Brent's + // algorithm. auto Run() -> ResultType { CARBON_CHECK(!todo.empty()); + + // The index of an enclosing item we are visiting. If we see this again at + // an index in + // [cycle_detector_index + 1, 2 * cycle_detector_index), + // we have found a cycle, and if we go deeper than that, we pick a new index + // and watch it for longer. + int cycle_detector_index = todo.size() - 1; + while (true) { - const size_t init_size = todo.size(); + const int init_size = todo.size(); auto [next_sem_ir, next] = todo.back(); + // Check that we're not in a cycle. + if (cycle_detector_index < init_size - 1 && + init_size - 1 < cycle_detector_index * 2) { + CARBON_CHECK( + todo[init_size - 1] != todo[cycle_detector_index], + "Fingerprinting got stuck in a cycle" +#ifndef NDEBUG + ":{0}", + [&]() -> std::string { + RawStringOstream out; + for (auto [next_sem_ir, next] : llvm::ArrayRef(todo).slice( + cycle_detector_index, + init_size - cycle_detector_index)) { + out << "\n"; + std::visit([&](auto id) { out << Dump(*next_sem_ir, id); }, + next); + } + return out.TakeStr(); + }() +#endif + ); + } else { + // We've left the region of the stack in which we're looking for this + // item. Switch to looking for the current item. + cycle_detector_index = init_size - 1; + } + sem_ir = next_sem_ir; store->Prepare(); @@ -706,7 +746,7 @@ struct Worklist { // the fingerprint for things other than `InstId`, but we really only // expect other `next` types to be at the bottom of the `todo` stack // since they are not added to `todo` during Run(). - if (todo.size() == init_size) { + if (static_cast(todo.size()) == init_size) { auto fingerprint = Finish(); todo.pop_back(); CARBON_CHECK(todo.empty(), @@ -758,7 +798,7 @@ struct Worklist { // If we didn't add any work, we have a fingerprint for this instruction; // pop it from the todo list. Otherwise, we leave it on the todo list so // we can compute its fingerprint once we've finished the work we added. - if (todo.size() == init_size) { + if (static_cast(todo.size()) == init_size) { ResultType fingerprint = Finish(); SetFingerprint(next_sem_ir, next_inst_id, fingerprint); todo.pop_back();