From 334b6c0bc822855a7fc454ec68465abe2350ca74 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Mon, 24 Aug 2026 16:07:43 -0400 Subject: [PATCH 1/2] Add tag-based filtering to debug() / HL_DEBUG_CODEGEN Selecting a specific debug() print via HL_DEBUG_CODEGEN currently requires naming a file and line range, which drifts as soon as the surrounding code changes. A call site can now be given a stable tag (debug(verbosity, "my-tag")) selectable via a new tag:name[,name...] rule that fires regardless of verbosity. Applied to the two prints that most benefit from a stable selector: the Simplify.cpp counter-example dump and the SimplifyCorrelatedDifferences.cpp non-monotonic warning. Co-Authored-By: Claude Sonnet 5 --- README.md | 7 ++- src/Debug.cpp | 47 ++++++++++++--- src/Debug.h | 17 +++++- src/Simplify.cpp | 2 +- src/SimplifyCorrelatedDifferences.cpp | 2 +- test/correctness/debug_helpers.cpp | 84 +++++++++++++++++++-------- 6 files changed, 121 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 1bffe7d692dd..baa902bb1796 100644 --- a/README.md +++ b/README.md @@ -460,7 +460,12 @@ Higher numbers will print more detail. The output can be filtered by source location using the grammar `verbosity[,filename[:line_low[-line_high]]][@func]`, with rules separated by `;` and OR-ed together (filenames and function names are matched as suffixes). For example, `HL_DEBUG_CODEGEN=3,Simplify.cpp:100-180` -prints verbosity-3 output only from lines 100–180 of `Simplify.cpp`. See +prints verbosity-3 output only from lines 100–180 of `Simplify.cpp`. A handful +of call sites are also given a stable tag (their second `debug()` argument, e.g. +`debug(1, "counterexample")`), independent of file/line, selectable via a +`tag:name` rule (e.g. `HL_DEBUG_CODEGEN=tag:counterexample`, or +`HL_DEBUG_CODEGEN=tag:counterexample,non-monotonic` to select either); a tag +rule fires whenever one of its names matches, regardless of verbosity. See [doc/Testing.md](doc/Testing.md) for more, including the LLDB/GDB debugger helpers. `HL_DEBUG_CODEGEN` output goes to stderr by default; set `HL_DEBUG_CODEGEN_LOG_FILE=` to append it to a file instead (the file is diff --git a/src/Debug.cpp b/src/Debug.cpp index bb714a0443e0..2f052ef0500c 100644 --- a/src/Debug.cpp +++ b/src/Debug.cpp @@ -47,11 +47,28 @@ class DebugRule { int line_low = -1; int line_high = INT_MAX; std::string function_suffix = ""; + bool is_tag_rule = false; + std::vector tags; enum Complexity { VerbosityOnly, NeedsMatching } complexity = VerbosityOnly; public: static std::optional parse(const std::string &spec) { + static constexpr char tag_prefix[] = "tag:"; + if (spec.rfind(tag_prefix, 0) == 0) { + DebugRule rule; + rule.is_tag_rule = true; + for (std::string &tag : split_string(spec.substr(sizeof(tag_prefix) - 1), ",")) { + if (!tag.empty()) { + rule.tags.push_back(std::move(tag)); + } + } + if (rule.tags.empty()) { + return std::nullopt; + } + return rule; + } + DebugRule rule; const char *ptr = spec.c_str(); @@ -86,8 +103,11 @@ class DebugRule { return rule; } - bool accepts(const int verbosity, const char *file, const char *function, - const int line) const { + bool accepts(const int verbosity, const char *tag, const char *file, + const char *function, const int line) const { + if (is_tag_rule) { + return std::any_of(tags.begin(), tags.end(), [&](const std::string &t) { return t == tag; }); + } switch (complexity) { case VerbosityOnly: return verbosity <= this->verbosity; @@ -118,13 +138,19 @@ std::vector parse_rules(const std::string &env) { "Warning: Ignoring malformed HL_DEBUG_CODEGEN entry: [" + spec + "]\n" + "Expected rule format:\n" " verbosity[,filename[:line_low[-line_high]]][@func]\n" + " tag:name[,name...]\n" "Rules are separated by ';' and are OR-ed together.\n" - "Matching for filename and function uses suffix matching.\n" + "Matching for filename and function uses suffix matching. A\n" + "tag:name rule matches a debug(verbosity, \"name\") call site's\n" + "tag exactly, regardless of verbosity; a comma-separated list\n" + "of names matches any one of them.\n" "Examples:\n" " HL_DEBUG_CODEGEN=2\n" " HL_DEBUG_CODEGEN=4,CodeGen_LLVM.cpp\n" " HL_DEBUG_CODEGEN=3,Simplify.cpp:100-180\n" " HL_DEBUG_CODEGEN=2@visit\n" + " HL_DEBUG_CODEGEN=tag:counterexample\n" + " HL_DEBUG_CODEGEN=tag:counterexample,non-monotonic\n" " HL_DEBUG_CODEGEN=1;4,CodeGen_LLVM.cpp@compile\n"; issue_warning(warning.c_str()); } @@ -132,10 +158,10 @@ std::vector parse_rules(const std::string &env) { return rules; } -bool rules_accept(const std::vector &rules, const int verbosity, +bool rules_accept(const std::vector &rules, const int verbosity, const char *tag, const char *file, const char *function, const int line) { return std::any_of(rules.begin(), rules.end(), [&](const auto &rule) { - return rule.accepts(verbosity, file, function, line); + return rule.accepts(verbosity, tag, file, function, line); }); } @@ -222,13 +248,18 @@ DebugStream::~DebugStream() { bool debug_is_active_impl(const int verbosity, const char *file, const char *function, const int line) { + return debug_is_active_impl(verbosity, "", file, function, line); +} + +bool debug_is_active_impl(const int verbosity, const char *tag, const char *file, + const char *function, const int line) { static const std::vector rules = parse_rules(get_env_variable("HL_DEBUG_CODEGEN")); - return rules_accept(rules, verbosity, file, function, line); + return rules_accept(rules, verbosity, tag, file, function, line); } -bool debug_spec_accepts(const std::string &spec, const int verbosity, +bool debug_spec_accepts(const std::string &spec, const int verbosity, const char *tag, const char *file, const char *function, const int line) { - return rules_accept(parse_rules(spec), verbosity, file, function, line); + return rules_accept(parse_rules(spec), verbosity, tag, file, function, line); } } // namespace Halide::Internal diff --git a/src/Debug.h b/src/Debug.h index 281dfca83e4a..ed8ae2402382 100644 --- a/src/Debug.h +++ b/src/Debug.h @@ -34,6 +34,7 @@ struct LoweredFunc; std::ostream &operator<<(std::ostream &, const LoweredFunc &); bool debug_is_active_impl(int verbosity, const char *file, const char *function, int line); +bool debug_is_active_impl(int verbosity, const char *tag, const char *file, const char *function, int line); /** Backs the debug() macro. Buffers everything written to it in memory, then * emits the whole statement's accumulated output as a single write when the @@ -74,11 +75,21 @@ class DebugStream : public std::ostringstream { * is determined by the value of the environment variable * HL_DEBUG_CODEGEN. Output goes to stderr by default, but can be * redirected via HL_DEBUG_CODEGEN_LOG_FILE (see DebugStream above). + * + * A call site can optionally be given a stable tag, independent of its + * file/line, so it can be selected on its own via + * HL_DEBUG_CODEGEN=tag:my-tag (or a comma-separated + * HL_DEBUG_CODEGEN=tag:my-tag,other-tag to select either) regardless of the + * configured verbosity: + * + * \code + * debug(verbosity, "my-tag") << "The expression is " << expr << "\n"; + * \endcode */ -#define debug(n) \ +#define debug(...) \ /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ - if (::Halide::Internal::debug_is_active_impl((n), __FILE__, __FUNCTION__, __LINE__)) ::Halide::Internal::DebugStream().stream() + if (::Halide::Internal::debug_is_active_impl(__VA_ARGS__, __FILE__, __FUNCTION__, __LINE__)) ::Halide::Internal::DebugStream().stream() /** Allow easily printing the contents of containers, or std::vector-like containers, * in debug output. Used like so: @@ -145,7 +156,7 @@ inline StreamT &operator<<(StreamT &stream, const PrintSpanLn &wrapper) { * function-local static), so it can't be exercised against multiple specs * from a single process; this re-parses `spec` fresh on every call using the * same grammar, so the parser/matcher can be unit tested directly. */ -bool debug_spec_accepts(const std::string &spec, int verbosity, +bool debug_spec_accepts(const std::string &spec, int verbosity, const char *tag, const char *file, const char *function, int line); } // namespace Internal diff --git a/src/Simplify.cpp b/src/Simplify.cpp index 75b908ce8202..18129614aca7 100644 --- a/src/Simplify.cpp +++ b/src/Simplify.cpp @@ -445,7 +445,7 @@ bool can_prove(Expr e, const Scope &bounds) { // Take a closer look at all failed proof attempts to hunt for // simplifier weaknesses if (!is_const(e)) { - debug(1) << [&]() -> std::string { + debug(1, "counterexample") << [&]() -> std::string { struct RenameVariables : public IRMutator { using IRMutator::visit; diff --git a/src/SimplifyCorrelatedDifferences.cpp b/src/SimplifyCorrelatedDifferences.cpp index dc90aa6d7cef..f7d1049ea4f0 100644 --- a/src/SimplifyCorrelatedDifferences.cpp +++ b/src/SimplifyCorrelatedDifferences.cpp @@ -229,7 +229,7 @@ class SimplifyCorrelatedDifferences : public IRMutator { e = PartiallyCancelDifferences()(e); e = simplify(e); - debug(1) << [&]() -> std::string { + debug(1, "non-monotonic") << [&]() -> std::string { if (is_monotonic(e, loop_var) != Monotonic::Unknown) { return ""; } diff --git a/test/correctness/debug_helpers.cpp b/test/correctness/debug_helpers.cpp index 477ffb95e178..b00850716dfc 100644 --- a/test/correctness/debug_helpers.cpp +++ b/test/correctness/debug_helpers.cpp @@ -5,47 +5,83 @@ using namespace Halide::Internal; int main(int argc, char **argv) { // A bare verbosity matches purely on level, at any location. - internal_assert(debug_spec_accepts("2", 0, "any.cpp", "any", 1)); - internal_assert(debug_spec_accepts("2", 2, "any.cpp", "any", 1)); - internal_assert(!debug_spec_accepts("2", 3, "any.cpp", "any", 1)); + internal_assert(debug_spec_accepts("2", 0, "", "any.cpp", "any", 1)); + internal_assert(debug_spec_accepts("2", 2, "", "any.cpp", "any", 1)); + internal_assert(!debug_spec_accepts("2", 3, "", "any.cpp", "any", 1)); // Filenames are matched as suffixes, subject to the verbosity bound. - internal_assert(debug_spec_accepts("4,CodeGen_LLVM.cpp", 4, "src/CodeGen_LLVM.cpp", "f", 10)); - internal_assert(!debug_spec_accepts("4,CodeGen_LLVM.cpp", 4, "src/Simplify.cpp", "f", 10)); - internal_assert(!debug_spec_accepts("4,CodeGen_LLVM.cpp", 5, "src/CodeGen_LLVM.cpp", "f", 10)); + internal_assert(debug_spec_accepts("4,CodeGen_LLVM.cpp", 4, "", "src/CodeGen_LLVM.cpp", "f", 10)); + internal_assert(!debug_spec_accepts("4,CodeGen_LLVM.cpp", 4, "", "src/Simplify.cpp", "f", 10)); + internal_assert(!debug_spec_accepts("4,CodeGen_LLVM.cpp", 5, "", "src/CodeGen_LLVM.cpp", "f", 10)); // Line ranges are inclusive on both ends. - internal_assert(debug_spec_accepts("3,Simplify.cpp:100-180", 3, "src/Simplify.cpp", "f", 100)); - internal_assert(debug_spec_accepts("3,Simplify.cpp:100-180", 3, "src/Simplify.cpp", "f", 180)); - internal_assert(!debug_spec_accepts("3,Simplify.cpp:100-180", 3, "src/Simplify.cpp", "f", 99)); - internal_assert(!debug_spec_accepts("3,Simplify.cpp:100-180", 3, "src/Simplify.cpp", "f", 181)); + internal_assert(debug_spec_accepts("3,Simplify.cpp:100-180", 3, "", "src/Simplify.cpp", "f", 100)); + internal_assert(debug_spec_accepts("3,Simplify.cpp:100-180", 3, "", "src/Simplify.cpp", "f", 180)); + internal_assert(!debug_spec_accepts("3,Simplify.cpp:100-180", 3, "", "src/Simplify.cpp", "f", 99)); + internal_assert(!debug_spec_accepts("3,Simplify.cpp:100-180", 3, "", "src/Simplify.cpp", "f", 181)); // A single line means low == high. - internal_assert(debug_spec_accepts("3,Simplify.cpp:100", 3, "src/Simplify.cpp", "f", 100)); - internal_assert(!debug_spec_accepts("3,Simplify.cpp:100", 3, "src/Simplify.cpp", "f", 101)); + internal_assert(debug_spec_accepts("3,Simplify.cpp:100", 3, "", "src/Simplify.cpp", "f", 100)); + internal_assert(!debug_spec_accepts("3,Simplify.cpp:100", 3, "", "src/Simplify.cpp", "f", 101)); // Functions are also matched as suffixes. - internal_assert(debug_spec_accepts("2@visit", 2, "any.cpp", "visit", 1)); - internal_assert(debug_spec_accepts("2@visit", 2, "any.cpp", "IRVisitor::visit", 1)); - internal_assert(!debug_spec_accepts("2@visit", 2, "any.cpp", "mutate", 1)); + internal_assert(debug_spec_accepts("2@visit", 2, "", "any.cpp", "visit", 1)); + internal_assert(debug_spec_accepts("2@visit", 2, "", "any.cpp", "IRVisitor::visit", 1)); + internal_assert(!debug_spec_accepts("2@visit", 2, "", "any.cpp", "mutate", 1)); // File, line, and function qualifiers combine (all must hold). - internal_assert(debug_spec_accepts("3,Simplify.cpp:100-180@visit", 3, "Simplify.cpp", "visit", 150)); - internal_assert(!debug_spec_accepts("3,Simplify.cpp:100-180@visit", 3, "Simplify.cpp", "mutate", 150)); + internal_assert(debug_spec_accepts("3,Simplify.cpp:100-180@visit", 3, "", "Simplify.cpp", "visit", 150)); + internal_assert(!debug_spec_accepts("3,Simplify.cpp:100-180@visit", 3, "", "Simplify.cpp", "mutate", 150)); // Rules separated by ';' are OR-ed together. - internal_assert(debug_spec_accepts("1;4,CodeGen_LLVM.cpp@compile", 1, "whatever.cpp", "g", 5)); - internal_assert(debug_spec_accepts("1;4,CodeGen_LLVM.cpp@compile", 4, "CodeGen_LLVM.cpp", "compile", 5)); - internal_assert(!debug_spec_accepts("1;4,CodeGen_LLVM.cpp@compile", 4, "CodeGen_LLVM.cpp", "other", 5)); + internal_assert(debug_spec_accepts("1;4,CodeGen_LLVM.cpp@compile", 1, "", "whatever.cpp", "g", 5)); + internal_assert(debug_spec_accepts("1;4,CodeGen_LLVM.cpp@compile", 4, "", "CodeGen_LLVM.cpp", "compile", 5)); + internal_assert(!debug_spec_accepts("1;4,CodeGen_LLVM.cpp@compile", 4, "", "CodeGen_LLVM.cpp", "other", 5)); // An empty spec behaves like verbosity 0: only debug(0) prints. - internal_assert(debug_spec_accepts("", 0, "any.cpp", "f", 1)); - internal_assert(!debug_spec_accepts("", 1, "any.cpp", "f", 1)); + internal_assert(debug_spec_accepts("", 0, "", "any.cpp", "f", 1)); + internal_assert(!debug_spec_accepts("", 1, "", "any.cpp", "f", 1)); // A malformed rule is skipped (and warns on stderr); with no valid rules, // nothing matches. A valid rule alongside it still takes effect. - internal_assert(!debug_spec_accepts("garbage", 0, "any.cpp", "f", 1)); - internal_assert(debug_spec_accepts("2;garbage", 2, "any.cpp", "f", 1)); + internal_assert(!debug_spec_accepts("garbage", 0, "", "any.cpp", "f", 1)); + internal_assert(debug_spec_accepts("2;garbage", 2, "", "any.cpp", "f", 1)); + + // A tag rule matches a call site with the same tag, at any verbosity, + // regardless of file/function/line. + internal_assert(debug_spec_accepts("tag:counterexample", 1, "counterexample", "any.cpp", "any", 1)); + internal_assert(debug_spec_accepts("tag:counterexample", 100, "counterexample", "other.cpp", "other", 999)); + + // A tag rule does not match a different (or absent/untagged) tag. + internal_assert(!debug_spec_accepts("tag:counterexample", 1, "non-monotonic", "any.cpp", "any", 1)); + internal_assert(!debug_spec_accepts("tag:counterexample", 1, "", "any.cpp", "any", 1)); + + // An ordinary verbosity/file rule still matches a tagged call site: the + // tag is irrelevant to non-tag rules. + internal_assert(debug_spec_accepts("1,Simplify.cpp", 1, "counterexample", "src/Simplify.cpp", "can_prove", 448)); + + // `tag:` with no name is malformed (skipped; matches nothing). + internal_assert(!debug_spec_accepts("tag:", 1, "", "any.cpp", "f", 1)); + internal_assert(!debug_spec_accepts("tag:", 1, "counterexample", "any.cpp", "f", 1)); + + // Tag rules OR correctly alongside other rules when ';'-separated. + internal_assert(debug_spec_accepts("1;tag:counterexample", 5, "counterexample", "any.cpp", "f", 1)); + internal_assert(debug_spec_accepts("1;tag:counterexample", 1, "", "any.cpp", "f", 1)); + internal_assert(!debug_spec_accepts("1;tag:counterexample", 5, "", "any.cpp", "f", 1)); + + // A comma-separated tag:a,b,c list matches any of its names, at any + // verbosity, and rejects names outside the list. + internal_assert(debug_spec_accepts("tag:counterexample,non-monotonic", 1, "counterexample", "any.cpp", "f", 1)); + internal_assert(debug_spec_accepts("tag:counterexample,non-monotonic", 100, "non-monotonic", "any.cpp", "f", 1)); + internal_assert(!debug_spec_accepts("tag:counterexample,non-monotonic", 1, "other", "any.cpp", "f", 1)); + internal_assert(!debug_spec_accepts("tag:counterexample,non-monotonic", 1, "", "any.cpp", "f", 1)); + + // Empty entries within a comma-separated tag list are simply skipped, + // consistent with how ';'-separated rules tolerate empty entries. + internal_assert(debug_spec_accepts("tag:counterexample,,non-monotonic", 1, "non-monotonic", "any.cpp", "f", 1)); + + // A tag list that reduces to no names at all is malformed. + internal_assert(!debug_spec_accepts("tag:,", 1, "", "any.cpp", "f", 1)); printf("Success!\n"); return 0; From 1fad6811f4dd42c8c44b6d7579bde25b67deeb49 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Mon, 24 Aug 2026 16:28:38 -0400 Subject: [PATCH 2/2] Isolate debug_log_file test from ambient HL_DEBUG_CODEGEN* env vars A shell that already has HL_DEBUG_CODEGEN set to a tag-only rule (e.g. while exercising the new tag-filter feature) silently suppresses the test's untagged debug(0) call, and any inherited HL_DEBUG_CODEGEN_LOG_FILE would fight with the values the test sets per scenario. Clear both at the top of each role (HL_DEBUG_CODEGEN unconditionally; the log file only in the parent, before it starts overriding it). Co-Authored-By: Claude Sonnet 5 --- test/correctness/debug_log_file.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/correctness/debug_log_file.cpp b/test/correctness/debug_log_file.cpp index b30b81af6244..a3ea48f2d68c 100644 --- a/test/correctness/debug_log_file.cpp +++ b/test/correctness/debug_log_file.cpp @@ -47,9 +47,24 @@ void set_env(const char *name, const std::string &val) { #endif } +void unset_env(const char *name) { +#ifdef _WIN32 + _putenv_s(name, ""); +#else + unsetenv(name); +#endif +} + } // namespace int main(int argc, char **argv) { + // HL_DEBUG_CODEGEN is read once per process (cached in a function-local + // static) on first use. Clear any ambient value from the invoking shell + // before it's read anywhere below (parent or re-exec'd --child alike): + // an inherited tag-only rule would otherwise silently suppress the + // untagged debug(0) call in --child mode. + unset_env("HL_DEBUG_CODEGEN"); + // Child mode: emit one line of debug(0) output, honoring // HL_DEBUG_CODEGEN_LOG_FILE as inherited from the parent's environment. if (argc == 2 && std::string(argv[1]) == "--child") { @@ -57,6 +72,12 @@ int main(int argc, char **argv) { return 0; } + // HL_DEBUG_CODEGEN_LOG_FILE is also read once per process, but unlike + // HL_DEBUG_CODEGEN above, --child is meant to inherit whatever value the + // parent (here) sets for each scenario below, so it's only cleared in + // the parent, before the first scenario overrides it. + unset_env("HL_DEBUG_CODEGEN_LOG_FILE"); + const std::string self = fs::absolute(argv[0]).string(); const fs::path tmp = fs::temp_directory_path() / ("hldbglog_" + std::to_string(std::random_device{}()));