Conversation
Backends TTs may implement print_incomplete_tasks() to print dangling tasks. This has proven helpful in debugging flow problems. Signed-off-by: Joseph Schuchart <joseph.schuchart@stonybrook.edu>
Signed-off-by: Joseph Schuchart <joseph.schuchart@stonybrook.edu>
There was a problem hiding this comment.
Pull request overview
This PR introduces a new debugging hook to print “incomplete/pending” tasks from TTG operations, plumbing it through composite TTGs and implementing it for the PaRSEC backend, plus a small tweak to DOT graph labeling when types are disabled.
Changes:
- Add
TTBase::print_incomplete_tasks()virtual API and forward it fromTTG. - Implement
print_incomplete_tasks()forttg_parsec::TTby iterating the PaRSEC task hash table. - Adjust DOT label output when
disable_typeis enabled.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
ttg/ttg/util/dot.h |
Tweaks DOT record-label formatting when type display is disabled. |
ttg/ttg/tt.h |
Adds TTG-level forwarding implementation of print_incomplete_tasks(). |
ttg/ttg/parsec/ttg.h |
Implements print_incomplete_tasks() for PaRSEC TT by iterating tasks_table. |
ttg/ttg/base/tt.h |
Adds the new print_incomplete_tasks() virtual method to the TTBase API. |
Comments suppressed due to low confidence (1)
ttg/ttg/util/dot.h:105
- Output terminal names are inserted into the DOT record label without escaping. If an output name contains
<,>,", or|, it can break DOT parsing; use the existingescape()helper for output names (in both thedisable_typeand typed branches).
if (disable_type) {
ttss << " <out" << count << ">"
<< out->get_name();
} else {
ttss << " <out" << count << ">"
<< " " << escape("<" + out->get_key_type_str() + "," + out->get_value_type_str() + ">") << " "
<< out->get_name();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| virtual void print_incomplete_tasks() const { | ||
| throw std::runtime_error("print_incomplete_tasks not implemented!"); | ||
| } |
There was a problem hiding this comment.
TTBase::print_incomplete_tasks() throws std::runtime_error but this header does not include <stdexcept>, which will fail to compile in translation units that don’t get it transitively. Add the missing include (or move the definition to a header that already includes it).
| virtual void print_incomplete_tasks() const { | ||
| throw std::runtime_error("print_incomplete_tasks not implemented!"); | ||
| } |
There was a problem hiding this comment.
Default-implementing print_incomplete_tasks() by throwing makes the new API hazardous to call on runtimes that don’t override it (and TTG::print_incomplete_tasks() will propagate those throws across mixed graphs). Consider making the base implementation a no-op (or making it pure virtual if all runtimes must support it) so this debug helper can be safely invoked generically.
| virtual void print_incomplete_tasks() const { | |
| throw std::runtime_error("print_incomplete_tasks not implemented!"); | |
| } | |
| /// Debug hook: default implementation is a no-op so it is safe to call generically. | |
| virtual void print_incomplete_tasks() const {} |
| ttss << " <in" << count << ">" | ||
| << " " << escape(in->get_key_type_str()) << " " << escape(in->get_name()); | ||
| << escape(in->get_name()); |
There was a problem hiding this comment.
When disable_type is true, the record label concatenates the port (<inN>) and the terminal name without any separator. This makes the DOT output harder to read and can be fragile for Graphviz record labels; consider keeping a leading space before the escaped name for consistency with the typed branch.
| virtual void print_incomplete_tasks() const override { | ||
| parsec_hash_table_for_all((parsec_hash_table_t*)&tasks_table, ht_iter_cb, (void*)this); | ||
| } |
There was a problem hiding this comment.
print_incomplete_tasks() is now const but uses C-style casts to drop constness for both tasks_table and this. Prefer const_cast (or make tasks_table mutable if iteration is logically-const) and pass a const ttT* through cb_data so ht_iter_cb doesn’t need to cast away const.
| for (auto& tt : tts) { | ||
| tt->print_incomplete_tasks(); |
There was a problem hiding this comment.
This forwards print_incomplete_tasks() to all contained ops; with the current TTBase default implementation throwing, this TTG-level call will throw as soon as it encounters a child TT that doesn’t override the method. If the intent is a best-effort debug print, consider making the base implementation a no-op or handling the unsupported case here to avoid aborting the whole traversal.
| for (auto& tt : tts) { | |
| tt->print_incomplete_tasks(); | |
| for (auto &tt : tts) { | |
| try { | |
| tt->print_incomplete_tasks(); | |
| } catch (...) { | |
| // Ignore unsupported or failing implementations to avoid aborting traversal | |
| } |
Also fixes a case in the Dot printer where we print a type even if we were asked not to.