From 0aec063ac5b5d861fc3462bc9ad35b830939eceb Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Wed, 19 Aug 2026 15:52:04 -0700 Subject: [PATCH 1/6] Profiler: add warnings and recompute to JSON, clarify struct aggregation The JSON output (HL_PROFILER_JSON_OUTPUT) now includes, per pipeline, a "warnings" array holding the full text of each performance warning that fired, and, per Func, the recompute ratio shown in the report's recompute column. The warning text is the same plain-English message the report prints, so consumers (LLMs especially) don't have to parse the table. It's collected into a small growable string per pipeline while the text report renders it, then emitted in the JSON pass. Also document, in HalideRuntime.h, how each profiler struct field aggregates across runs: peaks are maxima, time is summed over billed_runs, the remaining counters are summed over runs (divide by runs for a per-run value), the active-threads pair is a ready-made average, and memory_current is a live snapshot. Several consumers were dividing the wrong fields or not dividing the counters at all. Co-Authored-By: Claude Opus 4.8 --- src/runtime/HalideRuntime.h | 81 ++++++++++++++++++--------- src/runtime/profiler_common.cpp | 99 +++++++++++++++++++++++++++++++-- 2 files changed, 151 insertions(+), 29 deletions(-) diff --git a/src/runtime/HalideRuntime.h b/src/runtime/HalideRuntime.h index e1610b9028c8..d7b7333daca9 100644 --- a/src/runtime/HalideRuntime.h +++ b/src/runtime/HalideRuntime.h @@ -2018,6 +2018,21 @@ enum halide_profiler_func_kind { }; /** Per-Func state tracked by the sampling profiler. */ +// These fields fall into groups that aggregate differently across the +// pipeline's runs (see halide_profiler_pipeline_stats::runs). Read them +// accordingly -- in particular the counters are *totals over all runs*, so +// most consumers want to divide them by the run count: +// - Identity (name, parent, canonical_id, kind, buffer_func_id): fixed. +// - Peaks (memory_peak, stack_peak): the maximum over all runs. A peak, +// not a sum -- do not divide by runs. +// - Counters (everything from memory_total to the end of the struct, and +// time): summed over every run. Divide by the pipeline's `runs` to get a +// per-run value. `time` is the exception: it is summed only over runs +// that produced a sample, so divide it by `billed_runs` instead. +// active_threads_{numerator,denominator} are summed as a pair; their +// ratio is already an average, so do not divide it further. +// - memory_current is a live snapshot (roughly zero once a run finishes), +// not aggregated. struct HALIDE_ATTRIBUTE_ALIGN(8) halide_profiler_func_stats { /** The name of this Func. A global constant string. */ const char *name; @@ -2040,38 +2055,44 @@ struct HALIDE_ATTRIBUTE_ALIGN(8) halide_profiler_func_stats { int buffer_func_id; /** A bitmask flagging which of this Func's aggregated counters are - * conservative upper bounds rather than exact values. The bits index the - * counters passed to halide_profiler_update_counters, in that order: - * bit 0 = memory_total, 1 = num_allocs, 2 = parallel_loops, - * 3 = parallel_tasks, 4 = points_required_at_root, 5 = points_computed. - * (active_threads_numerator/denominator are sampled at runtime rather - * than summed over loops, so they are never approximated and have no - * bit.) A set bit only happens on GPU, where a guarded contribution - * can't be summed exactly and is bounded instead; the reporter marks - * such columns with a leading '<'. Must stay in sync with the counter - * enum in src/Profiling.cpp. */ + * conservative upper bounds rather than exact values. Bit i corresponds + * to the i'th counter passed to halide_profiler_update_counters, in the + * order of the counter enum in src/Profiling.cpp (bit 0 = memory_total, + * 1 = num_allocs, ... covering all the summed counters below, from + * memory_total through productions_if_inwards). active_threads_numerator + * and _denominator are sampled at runtime rather than summed over loops, + * so they are never approximated and have no bit. A set bit only happens + * on GPU, where a guarded contribution can't be summed exactly and is + * bounded instead; the reporter marks such columns with a leading '<'. */ uint32_t counters_approximated; - /** Total time taken evaluating this Func (in nanoseconds). */ + /** Total time (nanoseconds) spent evaluating this Func, summed across + * all runs that produced a profiler sample. Divide by the pipeline's + * billed_runs (not runs) for a per-run time. */ uint64_t HALIDE_ATTRIBUTE_ALIGN(8) time; - /** The current memory allocation of this Func. */ + /** This Func's live heap allocation at the instant the report is taken; + * normally near zero once a run has finished. A snapshot, not aggregated. */ uint64_t memory_current; - /** The peak memory allocation of this Func. */ + /** The peak heap allocation of this Func: the maximum over all runs (a + * peak, not a sum -- do not divide by runs). */ uint64_t memory_peak; - /** The peak stack allocation of this Func's threads. */ + /** The peak stack allocation of this Func's threads, as a maximum over + * all runs (like memory_peak). */ uint64_t stack_peak; - // Everything field after this point is a counter. They are aggregated by - // blindly adding. + // Every field after this point is a counter: summed across every run + // (and across a Func's instances). Divide by the pipeline's `runs` for a + // per-run value. /** The total memory allocation of this Func. */ uint64_t memory_total; - /** The average number of thread pool worker threads active while computing - * this Func. */ + /** The average number of thread pool worker threads active while + * computing this Func is numerator / denominator. Both are summed across + * runs, so the ratio is already an average -- do not divide it by runs. */ uint64_t active_threads_numerator, active_threads_denominator; /** The total number of times heap storage for this Func was allocated. */ @@ -2127,22 +2148,32 @@ struct HALIDE_ATTRIBUTE_ALIGN(8) halide_profiler_func_stats { }; /** Per-pipeline state tracked by the sampling profiler. These exist - * in a linked list. */ + * in a linked list. The fields aggregate across `runs` the same way as in + * halide_profiler_func_stats: `time` is summed over `billed_runs` (divide by + * that for per-run), `memory_peak` is a maximum (not a sum), the remaining + * counters are summed over `runs` (divide by `runs` for per-run), the + * active_threads pair is a ready-made average, and `memory_current` is a + * live snapshot. */ struct HALIDE_ATTRIBUTE_ALIGN(8) halide_profiler_pipeline_stats { - /** Total time spent in this pipeline (in nanoseconds) */ + /** Total time (nanoseconds) spent in this pipeline, summed across the + * runs that produced a sample. Divide by billed_runs for a per-run time. */ uint64_t time; - /** The current memory allocation of funcs in this pipeline. */ + /** Live heap allocation across this pipeline's funcs at report time; a + * snapshot, not aggregated. */ uint64_t memory_current; - /** The peak memory allocation of funcs in this pipeline. */ + /** The peak heap allocation of funcs in this pipeline: the maximum over + * all runs (a peak, not a sum). */ uint64_t memory_peak; - /** The total memory allocation of funcs in this pipeline. */ + /** The total memory allocated by funcs in this pipeline, summed across + * all runs. Divide by `runs` for a per-run value. */ uint64_t memory_total; - /** The average number of thread pool worker threads doing useful - * work while computing this pipeline. */ + /** The average number of thread pool worker threads doing useful work + * while computing this pipeline is numerator / denominator; the ratio is + * already an average (do not divide by runs). */ uint64_t active_threads_numerator, active_threads_denominator; /** The native vector width for the target this pipeline ran on, in diff --git a/src/runtime/profiler_common.cpp b/src/runtime/profiler_common.cpp index 0d0d27e45593..8baf27788fff 100644 --- a/src/runtime/profiler_common.cpp +++ b/src/runtime/profiler_common.cpp @@ -513,6 +513,21 @@ ALWAYS_INLINE bool counter_is_approximate(const halide_profiler_func_stats *fs, WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_state *s) { StringStreamPrinter<1024> sstr(user_context); + // When JSON output is requested, we render each pipeline's warnings into + // a "[...]" array string here (in pipeline-list order) as we print the + // report below, and emit them in the JSON pass. Null means no warnings. + const char *json_path = getenv("HL_PROFILER_JSON_OUTPUT"); + int num_pipelines = 0; + for (halide_profiler_pipeline_stats *p = s->pipelines; p; + p = (halide_profiler_pipeline_stats *)(p->next)) { + num_pipelines++; + } + char **pipeline_warnings = nullptr; + if (json_path && num_pipelines) { + pipeline_warnings = (char **)malloc(num_pipelines * sizeof(char *)); + __builtin_memset(pipeline_warnings, 0, num_pipelines * sizeof(char *)); + } + // Emit ANSI color escapes only when the report is going to an actual // color-capable terminal. Checking TERM alone isn't enough: CI and other // redirected environments often set TERM=xterm-256color while stdout is a @@ -715,8 +730,12 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st constexpr const char *column_legend_row_2 = " | |threads| loops| tasks|allocs| mem | mem | ratio | |"; + int pipeline_pos = -1; for (halide_profiler_pipeline_stats *p = s->pipelines; p; p = (halide_profiler_pipeline_stats *)(p->next)) { + // Position in the pipeline list, matched by the JSON pass below. + // Incremented before any `continue` so the two passes stay aligned. + pipeline_pos++; if (!p->runs) { continue; } @@ -1635,14 +1654,50 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st support_colors = old; } + // Render this pipeline's warnings as a JSON array of message strings + // for the JSON pass below. Kept separate from the text formatting + // above, and bounded: if the messages don't fit, the rest are + // dropped rather than producing a truncated (invalid) array. + if (pipeline_warnings && num_warnings) { + StringStreamPrinter<16384> wjson(user_context); + wjson << "["; + bool first = true; + for (int w = 0; w < num_warnings; w++) { + sstr.clear(); + int cid = warnings[w].canonical_id; + rule(&canon_fs[cid], &canon_cs[cid], (WarningKind)warnings[w].rule_id, /*emit=*/true); + const char *msg = sstr.str(); + // Bound the array so it never overflows wjson mid-string: + // reserve room for the escaped message (worst case 2x) plus + // the quotes, separator, and closing bracket. + if (wjson.size() + 2 * strlen(msg) + 8 >= 16384) { + break; + } + wjson << (first ? "\"" : ", \""); + first = false; + char one[2] = {0, 0}; + for (const char *c = msg; *c; c++) { + if (*c == '"' || *c == '\\') { + wjson << "\\"; + } + one[0] = *c; + wjson << one; + } + wjson << "\""; + } + wjson << "]"; + pipeline_warnings[pipeline_pos] = (char *)malloc(wjson.size() + 1); + memcpy(pipeline_warnings[pipeline_pos], wjson.str(), wjson.size() + 1); + } + sstr.clear(); emit_dim(horiz_rule); halide_print(user_context, sstr.str()); } - if (const char *raw_str = getenv("HL_PROFILER_JSON_OUTPUT")) { + if (json_path) { // Dump the raw stats to a JSON file for offline analysis. - void *f = halide_fopen(raw_str, "w"); + void *f = halide_fopen(json_path, "w"); if (f) { StringStreamPrinter<4096> json(user_context); @@ -1687,11 +1742,19 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st str(v); json << (last ? "\n" : ",\n"); }; + auto field_float = [&](const char *indent, const char *name, float v, bool last = false) { + json << indent; + str(name); + json << ": " << v; + json << (last ? "\n" : ",\n"); + }; json << "{\n \"pipelines\": ["; bool first_pipeline = true; + int json_pipeline_pos = -1; for (halide_profiler_pipeline_stats *pp = s->pipelines; pp; pp = (halide_profiler_pipeline_stats *)(pp->next)) { + json_pipeline_pos++; json << (first_pipeline ? "\n" : ",\n"); first_pipeline = false; @@ -1745,7 +1808,15 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st field_u64(" ", "points_required_at_realization", fs->points_required_at_realization); field_u64(" ", "points_required_at_production", fs->points_required_at_production); field_u64(" ", "points_required_inwards", fs->points_required_inwards); - field_u64(" ", "productions_if_inwards", fs->productions_if_inwards, true); + field_u64(" ", "productions_if_inwards", fs->productions_if_inwards); + // The recompute ratio shown in the report's recompute + // column: points computed / points required at root + // (billed to the canonical entry). + { + uint64_t at_root = pp->funcs[fs->canonical_id].points_required_at_root; + float recompute = at_root ? (float)fs->points_computed / at_root : 0.0f; + field_float(" ", "recompute", recompute, /*last=*/true); + } json << " }"; // Flush periodically so we don't overflow the buffer for @@ -1754,7 +1825,20 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st flush(); } } - json << "\n ]\n"; + json << "\n ],\n"; + // The full text of the performance warnings that fired, one + // string per warning, so consumers don't have to parse the + // report table. Rendered while the report above was printed. + // Written straight to the file (it can exceed json's buffer). + json << " \"warnings\": "; + flush(); + const char *pw = pipeline_warnings[json_pipeline_pos]; + if (pw) { + fwrite(pw, strlen(pw), 1, f); + } else { + fwrite("[]", 2, 1, f); + } + json << "\n"; json << " }"; } json << (first_pipeline ? "" : "\n") << " ]\n}\n"; @@ -1762,6 +1846,13 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st fclose(f); } } + + if (pipeline_warnings) { + for (int i = 0; i < num_pipelines; i++) { + free(pipeline_warnings[i]); + } + free(pipeline_warnings); + } } WEAK void halide_profiler_report(void *user_context) { From 607b6c1ccb2f325fa22bfdf3bde2c4d2b973b658 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Tue, 25 Aug 2026 10:15:53 -0700 Subject: [PATCH 2/6] Profiler: add per-Func cumulative (subtree) stats to the JSON output The report aggregates each Func's counters with those of its descendants for its parent rows; expose that same subtree rollup for every counter in the JSON, so agents consuming the JSON don't have to walk the parent pointers to reproduce it. Every field is summed, including the memory and stack peaks (a Func and its descendants can be live at once, so summing is a pessimistic bound on the subtree's peak footprint). The subtree rollup is now computed once per pipeline while printing the report (which needs it anyway) and persisted for the JSON pass, rather than recomputed. parallel_tasks, which the report latches downward for its "realized inside N tasks" warning, is computed into a small side array so the report keeps its latched value while the JSON gets a uniform subtree sum. Co-Authored-By: Claude Opus 4.8 --- src/runtime/profiler_common.cpp | 156 ++++++++++++++++++++++++-------- 1 file changed, 120 insertions(+), 36 deletions(-) diff --git a/src/runtime/profiler_common.cpp b/src/runtime/profiler_common.cpp index 8baf27788fff..ce6eef07aeab 100644 --- a/src/runtime/profiler_common.cpp +++ b/src/runtime/profiler_common.cpp @@ -523,9 +523,16 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st num_pipelines++; } char **pipeline_warnings = nullptr; + // Per-pipeline subtree-cumulative func stats, computed once while printing + // the report (which needs them anyway) and consumed by the JSON pass. + halide_profiler_func_stats **pipeline_cumulative = nullptr; if (json_path && num_pipelines) { pipeline_warnings = (char **)malloc(num_pipelines * sizeof(char *)); __builtin_memset(pipeline_warnings, 0, num_pipelines * sizeof(char *)); + pipeline_cumulative = (halide_profiler_func_stats **)malloc( + num_pipelines * sizeof(halide_profiler_func_stats *)); + __builtin_memset(pipeline_cumulative, 0, + num_pipelines * sizeof(halide_profiler_func_stats *)); } // Emit ANSI color escapes only when the report is going to an actual @@ -853,49 +860,79 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st } } - // Use the tree order to compute some cumulative stats - struct CumulativeStats { - // Time taken by this func and all children - uint64_t time; - // Average threads active for this func and all children - uint64_t active_threads_numerator; - uint64_t active_threads_denominator; - - // Number of tasks for all containing parallel loops. Note this is - // cumulative in the opposite direction - it incorporates - // information from parents, not children. - uint64_t parallel_tasks; - }; - size_t cum_stats_size = p->num_funcs * sizeof(CumulativeStats); - CumulativeStats *cum_stats = (CumulativeStats *)__builtin_alloca(cum_stats_size); - __builtin_memset(cum_stats, 0, cum_stats_size); - // Propagation to parents + // Subtree-cumulative stats: for each Func, its own counters plus those + // of all its descendants, folded in via tree order. Every counter is + // summed — including the memory and stack peaks, since a Func and its + // descendants can be live at once, so summing their peaks is a + // pessimistic bound on the subtree's peak footprint. parallel_tasks is + // the exception: it latches downward (a Func realized inside a parent's + // parallel loop inherits the parent's task count) to match how the + // report attributes tasks. Held in a halide_profiler_func_stats so the + // same values feed both the report below and the JSON output. + constexpr size_t cum_counter_offset = + __builtin_offsetof(halide_profiler_func_stats, memory_total); + constexpr int cum_counter_words = + (int)((sizeof(halide_profiler_func_stats) - cum_counter_offset) / + sizeof(uint64_t)); + halide_profiler_func_stats *cum_stats = + (halide_profiler_func_stats *)__builtin_alloca( + p->num_funcs * sizeof(halide_profiler_func_stats)); + __builtin_memset(cum_stats, 0, + p->num_funcs * sizeof(halide_profiler_func_stats)); + // Propagation to parents (children already folded in by tree order). for (int i = p->num_funcs - 1; i >= 0; i--) { int j = tree_order[i]; cum_stats[j].time += p->funcs[j].time; - cum_stats[j].active_threads_numerator += p->funcs[j].active_threads_numerator; - cum_stats[j].active_threads_denominator += p->funcs[j].active_threads_denominator; + cum_stats[j].memory_peak += p->funcs[j].memory_peak; + cum_stats[j].stack_peak += p->funcs[j].stack_peak; + uint64_t *jc = (uint64_t *)((char *)&cum_stats[j] + cum_counter_offset); + const uint64_t *sc = + (const uint64_t *)((const char *)&p->funcs[j] + cum_counter_offset); + for (int w = 0; w < cum_counter_words; w++) { + jc[w] += sc[w]; + } int parent = p->funcs[j].parent; if (parent >= 0) { cum_stats[parent].time += cum_stats[j].time; - cum_stats[parent].active_threads_numerator += cum_stats[j].active_threads_numerator; - cum_stats[parent].active_threads_denominator += cum_stats[j].active_threads_denominator; + cum_stats[parent].memory_peak += cum_stats[j].memory_peak; + cum_stats[parent].stack_peak += cum_stats[j].stack_peak; + uint64_t *pc = + (uint64_t *)((char *)&cum_stats[parent] + cum_counter_offset); + for (int w = 0; w < cum_counter_words; w++) { + pc[w] += jc[w]; + } } } - // Propagation to children: parallel_tasks latches downward — a Func - // realized inside its parent's parallel loop "inherits" the parent's - // task count if it doesn't have one of its own. + // parallel_tasks doesn't sum meaningfully across a subtree. The report + // latches it downward instead — a Func realized inside a parent's + // parallel loop inherits the parent's task count. Compute that latched + // value separately; cum_stats keeps the plain subtree sum so the JSON + // output sums every counter uniformly. + uint64_t *latched_tasks = + (uint64_t *)__builtin_alloca(p->num_funcs * sizeof(uint64_t)); + __builtin_memset(latched_tasks, 0, p->num_funcs * sizeof(uint64_t)); for (int i = 0; i < p->num_funcs; i++) { int j = tree_order[i]; int parent = p->funcs[j].parent; if (parent >= 0) { if (p->funcs[j].parallel_tasks == 0) { - cum_stats[j].parallel_tasks = cum_stats[parent].parallel_tasks; + latched_tasks[j] = latched_tasks[parent]; } else { - cum_stats[j].parallel_tasks = p->funcs[j].parallel_tasks; + latched_tasks[j] = p->funcs[j].parallel_tasks; } } } + // Persist a copy for the JSON pass, which runs after all reports print. + if (pipeline_cumulative) { + halide_profiler_func_stats *copy = + (halide_profiler_func_stats *)malloc( + p->num_funcs * sizeof(halide_profiler_func_stats)); + if (copy) { + memcpy(copy, cum_stats, + p->num_funcs * sizeof(halide_profiler_func_stats)); + } + pipeline_cumulative[pipeline_pos] = copy; + } // Rows to print, in tree-DFS order, skipping bookkeeping slots // that would be noise (no time, no allocs). @@ -944,11 +981,11 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st constexpr int num_counter_words = (int)(counter_bytes / sizeof(uint64_t)); size_t canon_fs_size = p->num_funcs * sizeof(halide_profiler_func_stats); - size_t canon_cs_size = p->num_funcs * sizeof(CumulativeStats); + size_t canon_cs_size = p->num_funcs * sizeof(halide_profiler_func_stats); halide_profiler_func_stats *canon_fs = (halide_profiler_func_stats *)__builtin_alloca(canon_fs_size); - CumulativeStats *canon_cs = - (CumulativeStats *)__builtin_alloca(canon_cs_size); + halide_profiler_func_stats *canon_cs = + (halide_profiler_func_stats *)__builtin_alloca(canon_cs_size); __builtin_memset(canon_fs, 0, canon_fs_size); __builtin_memset(canon_cs, 0, canon_cs_size); // canonical_id <= i for every instance, so a single forward pass @@ -983,11 +1020,11 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st dst_counters[j] += src_counters[j]; } - CumulativeStats &dst_cs = canon_cs[c]; + halide_profiler_func_stats &dst_cs = canon_cs[c]; dst_cs.time += cum_stats[i].time; dst_cs.active_threads_numerator += cum_stats[i].active_threads_numerator; dst_cs.active_threads_denominator += cum_stats[i].active_threads_denominator; - dst_cs.parallel_tasks += cum_stats[i].parallel_tasks; + dst_cs.parallel_tasks += latched_tasks[i]; } // ---- Heuristic warnings ----------------------------------------- @@ -1042,7 +1079,7 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st // also writes the warning message to sstr (using the same metrics // the trigger condition reads). auto rule = [&](const halide_profiler_func_stats *fs, - const CumulativeStats *cs, + const halide_profiler_func_stats *cs, WarningKind w, bool emit) -> bool { float threads_avg = cs->active_threads_numerator / @@ -1352,7 +1389,7 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st continue; } const halide_profiler_func_stats *agg_fs = &canon_fs[idx]; - const CumulativeStats *agg_cs = &canon_cs[idx]; + const halide_profiler_func_stats *agg_cs = &canon_cs[idx]; for (int w = 0; w < num_warning_kinds; w++) { if (rule(agg_fs, agg_cs, (WarningKind)w, /*emit=*/false) && num_warnings < max_warnings) { @@ -1407,7 +1444,7 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st }; auto print_func_row = [&](const halide_profiler_func_stats *fs, - const CumulativeStats *cs) { + const halide_profiler_func_stats *cs) { sstr.clear(); const char *row_template = func_row; if (fs->kind == halide_profiler_func_kind_allocation) { @@ -1569,7 +1606,7 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st for (int i = 0; i < f_stats_count; i++) { const halide_profiler_func_stats *fs = f_stats[i]; - const CumulativeStats *cs = cum_stats + (fs - p->funcs); + const halide_profiler_func_stats *cs = cum_stats + (fs - p->funcs); print_func_row(fs, cs); } @@ -1773,6 +1810,13 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st field_u64(" ", "native_vector_bytes", pp->native_vector_bytes); json << " \"funcs\": ["; + // The subtree-cumulative stats, computed once while printing the + // text report above (see pipeline_cumulative). Null only if the + // allocation failed, in which case the cumulative block is + // omitted. + const halide_profiler_func_stats *cumulative = + pipeline_cumulative[json_pipeline_pos]; + for (int i = 0; i < pp->num_funcs; i++) { json << (i == 0 ? "\n" : ",\n"); const halide_profiler_func_stats *fs = &pp->funcs[i]; @@ -1815,7 +1859,41 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st { uint64_t at_root = pp->funcs[fs->canonical_id].points_required_at_root; float recompute = at_root ? (float)fs->points_computed / at_root : 0.0f; - field_float(" ", "recompute", recompute, /*last=*/true); + field_float(" ", "recompute", recompute, + /*last=*/(cumulative == nullptr)); + } + if (cumulative) { + // The cumulative block is many fields; flush first so + // it can't overflow json's fixed-size buffer. + flush(); + const halide_profiler_func_stats *cum = &cumulative[i]; + json << " \"cumulative\": {\n"; + field_u64(" ", "time_ns", cum->time); + field_u64(" ", "memory_peak", cum->memory_peak); + field_u64(" ", "stack_peak", cum->stack_peak); + field_u64(" ", "memory_total", cum->memory_total); + field_u64(" ", "active_threads_numerator", cum->active_threads_numerator); + field_u64(" ", "active_threads_denominator", cum->active_threads_denominator); + field_u64(" ", "num_allocs", cum->num_allocs); + field_u64(" ", "parallel_loops", cum->parallel_loops); + field_u64(" ", "parallel_tasks", cum->parallel_tasks); + field_u64(" ", "points_required_at_root", cum->points_required_at_root); + field_u64(" ", "points_computed", cum->points_computed); + field_u64(" ", "scalar_loads", cum->scalar_loads); + field_u64(" ", "vector_loads", cum->vector_loads); + field_u64(" ", "gathers", cum->gathers); + field_u64(" ", "bytes_loaded", cum->bytes_loaded); + field_u64(" ", "scalar_stores", cum->scalar_stores); + field_u64(" ", "vector_stores", cum->vector_stores); + field_u64(" ", "scatters", cum->scatters); + field_u64(" ", "bytes_stored", cum->bytes_stored); + field_u64(" ", "realizations", cum->realizations); + field_u64(" ", "productions", cum->productions); + field_u64(" ", "points_required_at_realization", cum->points_required_at_realization); + field_u64(" ", "points_required_at_production", cum->points_required_at_production); + field_u64(" ", "points_required_inwards", cum->points_required_inwards); + field_u64(" ", "productions_if_inwards", cum->productions_if_inwards, /*last=*/true); + json << " }\n"; } json << " }"; @@ -1853,6 +1931,12 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st } free(pipeline_warnings); } + if (pipeline_cumulative) { + for (int i = 0; i < num_pipelines; i++) { + free(pipeline_cumulative[i]); + } + free(pipeline_cumulative); + } } WEAK void halide_profiler_report(void *user_context) { From 44365614eb67cafb4ad0fac6f876f62bd2340a89 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Tue, 25 Aug 2026 11:55:13 -0700 Subject: [PATCH 3/6] Profiler: include pipeline-level warnings in the JSON output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The JSON warnings array only carried the per-Func (numbered) warnings; the pipeline-level ones the report prints as bullets — too many auto-named Funcs, too few samples, and expensive frees — were omitted, and the array wasn't emitted at all unless a per-Func warning also fired. Emit the pipeline-level warnings too (in the same order as the report), and build the array whenever any warning fires. The message text is factored into a shared helper so the report and JSON can't drift. Co-Authored-By: Claude Opus 4.8 --- src/runtime/profiler_common.cpp | 92 ++++++++++++++++++++++----------- 1 file changed, 62 insertions(+), 30 deletions(-) diff --git a/src/runtime/profiler_common.cpp b/src/runtime/profiler_common.cpp index ce6eef07aeab..a8ca870df87b 100644 --- a/src/runtime/profiler_common.cpp +++ b/src/runtime/profiler_common.cpp @@ -1650,34 +1650,48 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st p->memory_peak > 100 * 1000 * 1000 && free_time * 10 > p->time; - if (num_warnings || too_few_samples || too_many_anon_funcs || expensive_free) { + // Text of the pipeline-level (non-Func-specific) warnings, appended to + // sstr. Factored out so the report and the JSON output below render + // identical wording. k: 0 = anon names, 1 = too few samples, 2 = free. + auto append_pipeline_warning = [&](int k) { + if (k == 0) { + sstr << anon_funcs << " Funcs have auto-generated names and " + << "collectively take up a significant fraction of the total runtime. " + << "Consider giving them explicit names by passing a string to the " + << "Func constructor. This will make this profile easier to read."; + } else if (k == 1) { + sstr << "Only " << p->samples + << " profiling samples taken. Consider running the " + << "pipeline more times in a loop for more accurate results."; + } else { + sstr << "The pipeline allocates a significant amount of memory, and a " + << "lot of time is spent freeing it. Either fuse stages more aggressively " + << "to use less memory, or consider a using caching allocator with " + << "retention enabled to make freeing it cheaper."; + } + }; + auto pipeline_warning_fired = [&](int k) { + return (k == 0 && too_many_anon_funcs) || + (k == 1 && too_few_samples) || + (k == 2 && expensive_free); + }; + const bool any_pipeline_warning = + too_few_samples || too_many_anon_funcs || expensive_free; + if (num_warnings || any_pipeline_warning) { halide_print(user_context, " Performance warnings:\n"); int max_cols = (int)strlen(func_row); // print_wrapped doesn't understand non-printing characters. bool old = support_colors; support_colors = false; - if (too_many_anon_funcs) { - sstr.clear(); - sstr << " - " << anon_funcs << " Funcs have auto-generated names and " - << "collectively take up a significant fraction of the total runtime. " - << "Consider giving them explicit names by passing a string to the " - << "Func constructor. This will make this profile easier to read.\n"; - print_wrapped(user_context, 4, max_cols, sstr.str()); - } - if (too_few_samples) { - sstr.clear(); - sstr << " - Only " << p->samples - << " profiling samples taken. Consider running the " - << "pipeline more times in a loop for more accurate results.\n"; - print_wrapped(user_context, 4, max_cols, sstr.str()); - } - if (expensive_free) { + for (int k = 0; k < 3; k++) { + if (!pipeline_warning_fired(k)) { + continue; + } sstr.clear(); - sstr << " - The pipeline allocates a significant amount of memory, and a " - << "lot of time is spent freeing it. Either fuse stages more aggressively " - << "to use less memory, or consider a using caching allocator with " - << "retention enabled to make freeing it cheaper.\n"; + sstr << " - "; + append_pipeline_warning(k); + sstr << "\n"; print_wrapped(user_context, 4, max_cols, sstr.str()); } for (int w = 0; w < num_warnings; w++) { @@ -1695,20 +1709,17 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st // for the JSON pass below. Kept separate from the text formatting // above, and bounded: if the messages don't fit, the rest are // dropped rather than producing a truncated (invalid) array. - if (pipeline_warnings && num_warnings) { + if (pipeline_warnings && (num_warnings || any_pipeline_warning)) { StringStreamPrinter<16384> wjson(user_context); wjson << "["; bool first = true; - for (int w = 0; w < num_warnings; w++) { - sstr.clear(); - int cid = warnings[w].canonical_id; - rule(&canon_fs[cid], &canon_cs[cid], (WarningKind)warnings[w].rule_id, /*emit=*/true); + // Append sstr's current contents as a JSON string element, escaping + // as needed. Returns false (appending nothing) if it wouldn't fit, + // so the array is bounded and never left truncated mid-string. + auto append_json = [&]() -> bool { const char *msg = sstr.str(); - // Bound the array so it never overflows wjson mid-string: - // reserve room for the escaped message (worst case 2x) plus - // the quotes, separator, and closing bracket. if (wjson.size() + 2 * strlen(msg) + 8 >= 16384) { - break; + return false; } wjson << (first ? "\"" : ", \""); first = false; @@ -1721,6 +1732,27 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st wjson << one; } wjson << "\""; + return true; + }; + // Pipeline-level warnings first (same order as the report above), + // then the per-Func ones. + for (int k = 0; k < 3; k++) { + if (!pipeline_warning_fired(k)) { + continue; + } + sstr.clear(); + append_pipeline_warning(k); + if (!append_json()) { + break; + } + } + for (int w = 0; w < num_warnings; w++) { + sstr.clear(); + int cid = warnings[w].canonical_id; + rule(&canon_fs[cid], &canon_cs[cid], (WarningKind)warnings[w].rule_id, /*emit=*/true); + if (!append_json()) { + break; + } } wjson << "]"; pipeline_warnings[pipeline_pos] = (char *)malloc(wjson.size() + 1); From 26623a6f9efc6eb2a7ab9b47fad7ad8e2680640c Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Tue, 25 Aug 2026 12:09:00 -0700 Subject: [PATCH 4/6] Profiler: nest per-Func warnings in each Func's JSON entry Per-Func (numbered) warnings were appended to the pipeline's top-level warnings array. Move them into a "warnings" array on each Func's JSON entry instead, grouped by canonical id (they fire per canonical Func, so every instance shows the same set). The top-level array now holds only the pipeline-level warnings. The JSON escaping is factored into a shared template helper used by both. Co-Authored-By: Claude Opus 4.8 --- src/runtime/profiler_common.cpp | 151 +++++++++++++++++++++++--------- 1 file changed, 112 insertions(+), 39 deletions(-) diff --git a/src/runtime/profiler_common.cpp b/src/runtime/profiler_common.cpp index a8ca870df87b..804599fdf146 100644 --- a/src/runtime/profiler_common.cpp +++ b/src/runtime/profiler_common.cpp @@ -510,6 +510,33 @@ ALWAYS_INLINE bool counter_is_approximate(const halide_profiler_func_stats *fs, return (fs->counters_approximated & (1u << counter)) != 0; } +// Append msg to a JSON string-array builder as an escaped "..." element, +// preceded by a comma unless it's the first. Returns false without appending +// if it wouldn't fit, so the array stays bounded and is never left truncated +// mid-string. (extern "C++" because this file is inside an extern "C" block +// and templates need C++ linkage.) +extern "C++" { +template +ALWAYS_INLINE bool json_append_escaped(StringStreamPrinter &out, bool &first, + const char *msg) { + if (out.size() + 2 * strlen(msg) + 8 >= out.capacity()) { + return false; + } + out << (first ? "\"" : ", \""); + first = false; + char one[2] = {0, 0}; + for (const char *c = msg; *c; c++) { + if (*c == '"' || *c == '\\') { + out << "\\"; + } + one[0] = *c; + out << one; + } + out << "\""; + return true; +} +} // extern "C++" + WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_state *s) { StringStreamPrinter<1024> sstr(user_context); @@ -522,13 +549,20 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st p = (halide_profiler_pipeline_stats *)(p->next)) { num_pipelines++; } + // Pipeline-level warnings, one JSON array string per pipeline. char **pipeline_warnings = nullptr; + // Per-Func warnings, grouped by canonical id: for each pipeline, an array + // (indexed by canonical id) of JSON array strings, nested into each Func's + // JSON entry below. + char ***pipeline_func_warnings = nullptr; // Per-pipeline subtree-cumulative func stats, computed once while printing // the report (which needs them anyway) and consumed by the JSON pass. halide_profiler_func_stats **pipeline_cumulative = nullptr; if (json_path && num_pipelines) { pipeline_warnings = (char **)malloc(num_pipelines * sizeof(char *)); __builtin_memset(pipeline_warnings, 0, num_pipelines * sizeof(char *)); + pipeline_func_warnings = (char ***)malloc(num_pipelines * sizeof(char **)); + __builtin_memset(pipeline_func_warnings, 0, num_pipelines * sizeof(char **)); pipeline_cumulative = (halide_profiler_func_stats **)malloc( num_pipelines * sizeof(halide_profiler_func_stats *)); __builtin_memset(pipeline_cumulative, 0, @@ -1705,52 +1739,22 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st support_colors = old; } - // Render this pipeline's warnings as a JSON array of message strings - // for the JSON pass below. Kept separate from the text formatting - // above, and bounded: if the messages don't fit, the rest are - // dropped rather than producing a truncated (invalid) array. - if (pipeline_warnings && (num_warnings || any_pipeline_warning)) { + // Render this pipeline's warnings as JSON array strings for the JSON + // pass below (kept separate from the text formatting above, and bounded + // so a too-long array is dropped rather than left truncated/invalid). + // Pipeline-level warnings go in a top-level array; per-Func warnings are + // grouped by canonical id and nested into each Func's entry. + if (pipeline_warnings && any_pipeline_warning) { StringStreamPrinter<16384> wjson(user_context); wjson << "["; bool first = true; - // Append sstr's current contents as a JSON string element, escaping - // as needed. Returns false (appending nothing) if it wouldn't fit, - // so the array is bounded and never left truncated mid-string. - auto append_json = [&]() -> bool { - const char *msg = sstr.str(); - if (wjson.size() + 2 * strlen(msg) + 8 >= 16384) { - return false; - } - wjson << (first ? "\"" : ", \""); - first = false; - char one[2] = {0, 0}; - for (const char *c = msg; *c; c++) { - if (*c == '"' || *c == '\\') { - wjson << "\\"; - } - one[0] = *c; - wjson << one; - } - wjson << "\""; - return true; - }; - // Pipeline-level warnings first (same order as the report above), - // then the per-Func ones. for (int k = 0; k < 3; k++) { if (!pipeline_warning_fired(k)) { continue; } sstr.clear(); append_pipeline_warning(k); - if (!append_json()) { - break; - } - } - for (int w = 0; w < num_warnings; w++) { - sstr.clear(); - int cid = warnings[w].canonical_id; - rule(&canon_fs[cid], &canon_cs[cid], (WarningKind)warnings[w].rule_id, /*emit=*/true); - if (!append_json()) { + if (!json_append_escaped(wjson, first, sstr.str())) { break; } } @@ -1758,6 +1762,46 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st pipeline_warnings[pipeline_pos] = (char *)malloc(wjson.size() + 1); memcpy(pipeline_warnings[pipeline_pos], wjson.str(), wjson.size() + 1); } + // Per-Func warnings fire on the canonical Func (every instance sharing a + // name shows the same set), so group them by canonical id. A Func's JSON + // entry looks up its own canonical id. + if (pipeline_func_warnings && num_warnings) { + char **fw = (char **)malloc(p->num_funcs * sizeof(char *)); + if (fw) { + __builtin_memset(fw, 0, p->num_funcs * sizeof(char *)); + for (int c = 0; c < p->num_funcs; c++) { + bool has = false; + for (int w = 0; w < num_warnings; w++) { + if (warnings[w].canonical_id == c) { + has = true; + break; + } + } + if (!has) { + continue; + } + StringStreamPrinter<16384> cjson(user_context); + cjson << "["; + bool cfirst = true; + for (int w = 0; w < num_warnings; w++) { + if (warnings[w].canonical_id != c) { + continue; + } + sstr.clear(); + rule(&canon_fs[c], &canon_cs[c], (WarningKind)warnings[w].rule_id, /*emit=*/true); + if (!json_append_escaped(cjson, cfirst, sstr.str())) { + break; + } + } + cjson << "]"; + fw[c] = (char *)malloc(cjson.size() + 1); + if (fw[c]) { + memcpy(fw[c], cjson.str(), cjson.size() + 1); + } + } + } + pipeline_func_warnings[pipeline_pos] = fw; + } sstr.clear(); emit_dim(horiz_rule); @@ -1892,7 +1936,7 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st uint64_t at_root = pp->funcs[fs->canonical_id].points_required_at_root; float recompute = at_root ? (float)fs->points_computed / at_root : 0.0f; field_float(" ", "recompute", recompute, - /*last=*/(cumulative == nullptr)); + /*last=*/false); } if (cumulative) { // The cumulative block is many fields; flush first so @@ -1925,8 +1969,23 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st field_u64(" ", "points_required_at_production", cum->points_required_at_production); field_u64(" ", "points_required_inwards", cum->points_required_inwards); field_u64(" ", "productions_if_inwards", cum->productions_if_inwards, /*last=*/true); - json << " }\n"; + json << " },\n"; + } + // Nested per-Func warnings: this Func's canonical set (fires + // per canonical Func; every instance shows the same set), or + // [] if none. Written straight to the file (it can exceed + // json's buffer). + json << " \"warnings\": "; + flush(); + { + char **fw = pipeline_func_warnings + ? pipeline_func_warnings[json_pipeline_pos] + : nullptr; + const char *fws = + (fw && fw[fs->canonical_id]) ? fw[fs->canonical_id] : "[]"; + fwrite(fws, strlen(fws), 1, f); } + json << "\n"; json << " }"; // Flush periodically so we don't overflow the buffer for @@ -1963,6 +2022,20 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st } free(pipeline_warnings); } + if (pipeline_func_warnings) { + int pi = 0; + for (halide_profiler_pipeline_stats *p = s->pipelines; p; + p = (halide_profiler_pipeline_stats *)(p->next)) { + char **fw = pipeline_func_warnings[pi++]; + if (fw) { + for (int j = 0; j < p->num_funcs; j++) { + free(fw[j]); + } + free(fw); + } + } + free(pipeline_func_warnings); + } if (pipeline_cumulative) { for (int i = 0; i < num_pipelines; i++) { free(pipeline_cumulative[i]); From 58297a4497df394437af1e4233944bd4e2db2fc9 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Tue, 25 Aug 2026 12:27:05 -0700 Subject: [PATCH 5/6] Profiler: latch parallel_tasks in the cumulative stats too The cumulative subtree rollup summed every counter but kept a separate downward-latched parallel_tasks just for the report's warning, leaving the JSON cumulative parallel_tasks a subtree sum. Latch it directly in the cumulative stats instead, so the report and the JSON see the same value, and document that parallel_tasks is the odd one out (a latched task count, not a subtree sum). Drops the separate latched_tasks array. Co-Authored-By: Claude Opus 4.8 --- src/runtime/profiler_common.cpp | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/runtime/profiler_common.cpp b/src/runtime/profiler_common.cpp index 804599fdf146..4b05551f244d 100644 --- a/src/runtime/profiler_common.cpp +++ b/src/runtime/profiler_common.cpp @@ -937,22 +937,23 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st } } } - // parallel_tasks doesn't sum meaningfully across a subtree. The report - // latches it downward instead — a Func realized inside a parent's - // parallel loop inherits the parent's task count. Compute that latched - // value separately; cum_stats keeps the plain subtree sum so the JSON - // output sums every counter uniformly. - uint64_t *latched_tasks = - (uint64_t *)__builtin_alloca(p->num_funcs * sizeof(uint64_t)); - __builtin_memset(latched_tasks, 0, p->num_funcs * sizeof(uint64_t)); + // parallel_tasks is the one field that doesn't sum across a subtree. + // The report latches it downward instead — a Func realized inside a + // parent's parallel loop inherits the parent's task count if it has + // none of its own — so do the same here, overwriting the summed value. + // Both the report and the JSON cumulative stats therefore see the + // latched value rather than a subtree sum (the JSON emitter notes this). + for (int i = 0; i < p->num_funcs; i++) { + cum_stats[i].parallel_tasks = 0; + } for (int i = 0; i < p->num_funcs; i++) { int j = tree_order[i]; int parent = p->funcs[j].parent; if (parent >= 0) { if (p->funcs[j].parallel_tasks == 0) { - latched_tasks[j] = latched_tasks[parent]; + cum_stats[j].parallel_tasks = cum_stats[parent].parallel_tasks; } else { - latched_tasks[j] = p->funcs[j].parallel_tasks; + cum_stats[j].parallel_tasks = p->funcs[j].parallel_tasks; } } } @@ -1058,7 +1059,7 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st dst_cs.time += cum_stats[i].time; dst_cs.active_threads_numerator += cum_stats[i].active_threads_numerator; dst_cs.active_threads_denominator += cum_stats[i].active_threads_denominator; - dst_cs.parallel_tasks += latched_tasks[i]; + dst_cs.parallel_tasks += cum_stats[i].parallel_tasks; } // ---- Heuristic warnings ----------------------------------------- @@ -1939,8 +1940,14 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st /*last=*/false); } if (cumulative) { - // The cumulative block is many fields; flush first so - // it can't overflow json's fixed-size buffer. + // Subtree rollup: this Func plus all its descendants. + // Every field is a sum EXCEPT parallel_tasks, which + // (like the report) is the latched task count — the + // parallel-loop task count this Func runs under, + // inherited from its parent if it has none of its own, + // not a sum over the subtree. + // The block is many fields; flush first so it can't + // overflow json's fixed-size buffer. flush(); const halide_profiler_func_stats *cum = &cumulative[i]; json << " \"cumulative\": {\n"; From 84eb20378703a04705a800f2815574774067f9cb Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Tue, 25 Aug 2026 12:54:48 -0700 Subject: [PATCH 6/6] Profiler: harden JSON output per code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Abort (halide_abort_if_false) if any of the JSON-side allocations return null — the three side arrays, the pipeline-level warnings string, the per-Func warnings array and its per-canonical strings, and the persisted cumulative copy. A failed small allocation means the system is in a bad state, so bailing out immediately beats limping along. Legitimate "not built" nulls (a skipped pipeline's cumulative, a pipeline/Func with no warnings) are still handled and emit the empty case. - Bound the cumulative parent-propagation and parallel_tasks latch with parent < num_funcs, not just parent >= 0. The tree builder deliberately tolerates orphans whose parent points outside the array; without the upper bound those would index cum_stats out of range (a pre-existing gap that this change had widened to the full counter region). Co-Authored-By: Claude Opus 4.8 --- src/runtime/profiler_common.cpp | 79 +++++++++++++++++---------------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/src/runtime/profiler_common.cpp b/src/runtime/profiler_common.cpp index 4b05551f244d..6c351d01510f 100644 --- a/src/runtime/profiler_common.cpp +++ b/src/runtime/profiler_common.cpp @@ -559,12 +559,17 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st // the report (which needs them anyway) and consumed by the JSON pass. halide_profiler_func_stats **pipeline_cumulative = nullptr; if (json_path && num_pipelines) { + // These allocations are small; if they fail the system is in a bad + // enough state that bailing out immediately beats trying to limp along. pipeline_warnings = (char **)malloc(num_pipelines * sizeof(char *)); + halide_abort_if_false(user_context, pipeline_warnings != nullptr); __builtin_memset(pipeline_warnings, 0, num_pipelines * sizeof(char *)); pipeline_func_warnings = (char ***)malloc(num_pipelines * sizeof(char **)); + halide_abort_if_false(user_context, pipeline_func_warnings != nullptr); __builtin_memset(pipeline_func_warnings, 0, num_pipelines * sizeof(char **)); pipeline_cumulative = (halide_profiler_func_stats **)malloc( num_pipelines * sizeof(halide_profiler_func_stats *)); + halide_abort_if_false(user_context, pipeline_cumulative != nullptr); __builtin_memset(pipeline_cumulative, 0, num_pipelines * sizeof(halide_profiler_func_stats *)); } @@ -926,7 +931,7 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st jc[w] += sc[w]; } int parent = p->funcs[j].parent; - if (parent >= 0) { + if (parent >= 0 && parent < p->num_funcs) { cum_stats[parent].time += cum_stats[j].time; cum_stats[parent].memory_peak += cum_stats[j].memory_peak; cum_stats[parent].stack_peak += cum_stats[j].stack_peak; @@ -949,7 +954,7 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st for (int i = 0; i < p->num_funcs; i++) { int j = tree_order[i]; int parent = p->funcs[j].parent; - if (parent >= 0) { + if (parent >= 0 && parent < p->num_funcs) { if (p->funcs[j].parallel_tasks == 0) { cum_stats[j].parallel_tasks = cum_stats[parent].parallel_tasks; } else { @@ -962,10 +967,9 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st halide_profiler_func_stats *copy = (halide_profiler_func_stats *)malloc( p->num_funcs * sizeof(halide_profiler_func_stats)); - if (copy) { - memcpy(copy, cum_stats, - p->num_funcs * sizeof(halide_profiler_func_stats)); - } + halide_abort_if_false(user_context, copy != nullptr); + memcpy(copy, cum_stats, + p->num_funcs * sizeof(halide_profiler_func_stats)); pipeline_cumulative[pipeline_pos] = copy; } @@ -1761,6 +1765,7 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st } wjson << "]"; pipeline_warnings[pipeline_pos] = (char *)malloc(wjson.size() + 1); + halide_abort_if_false(user_context, pipeline_warnings[pipeline_pos] != nullptr); memcpy(pipeline_warnings[pipeline_pos], wjson.str(), wjson.size() + 1); } // Per-Func warnings fire on the canonical Func (every instance sharing a @@ -1768,38 +1773,36 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st // entry looks up its own canonical id. if (pipeline_func_warnings && num_warnings) { char **fw = (char **)malloc(p->num_funcs * sizeof(char *)); - if (fw) { - __builtin_memset(fw, 0, p->num_funcs * sizeof(char *)); - for (int c = 0; c < p->num_funcs; c++) { - bool has = false; - for (int w = 0; w < num_warnings; w++) { - if (warnings[w].canonical_id == c) { - has = true; - break; - } + halide_abort_if_false(user_context, fw != nullptr); + __builtin_memset(fw, 0, p->num_funcs * sizeof(char *)); + for (int c = 0; c < p->num_funcs; c++) { + bool has = false; + for (int w = 0; w < num_warnings; w++) { + if (warnings[w].canonical_id == c) { + has = true; + break; } - if (!has) { + } + if (!has) { + continue; + } + StringStreamPrinter<16384> cjson(user_context); + cjson << "["; + bool cfirst = true; + for (int w = 0; w < num_warnings; w++) { + if (warnings[w].canonical_id != c) { continue; } - StringStreamPrinter<16384> cjson(user_context); - cjson << "["; - bool cfirst = true; - for (int w = 0; w < num_warnings; w++) { - if (warnings[w].canonical_id != c) { - continue; - } - sstr.clear(); - rule(&canon_fs[c], &canon_cs[c], (WarningKind)warnings[w].rule_id, /*emit=*/true); - if (!json_append_escaped(cjson, cfirst, sstr.str())) { - break; - } - } - cjson << "]"; - fw[c] = (char *)malloc(cjson.size() + 1); - if (fw[c]) { - memcpy(fw[c], cjson.str(), cjson.size() + 1); + sstr.clear(); + rule(&canon_fs[c], &canon_cs[c], (WarningKind)warnings[w].rule_id, /*emit=*/true); + if (!json_append_escaped(cjson, cfirst, sstr.str())) { + break; } } + cjson << "]"; + fw[c] = (char *)malloc(cjson.size() + 1); + halide_abort_if_false(user_context, fw[c] != nullptr); + memcpy(fw[c], cjson.str(), cjson.size() + 1); } pipeline_func_warnings[pipeline_pos] = fw; } @@ -1888,9 +1891,9 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st json << " \"funcs\": ["; // The subtree-cumulative stats, computed once while printing the - // text report above (see pipeline_cumulative). Null only if the - // allocation failed, in which case the cumulative block is - // omitted. + // text report above (see pipeline_cumulative). Null if an + // allocation failed or the pipeline was skipped in the text + // pass, in which case the cumulative block is omitted. const halide_profiler_func_stats *cumulative = pipeline_cumulative[json_pipeline_pos]; @@ -1985,9 +1988,7 @@ WEAK void halide_profiler_report_unlocked(void *user_context, halide_profiler_st json << " \"warnings\": "; flush(); { - char **fw = pipeline_func_warnings - ? pipeline_func_warnings[json_pipeline_pos] - : nullptr; + char **fw = pipeline_func_warnings[json_pipeline_pos]; const char *fws = (fw && fw[fs->canonical_id]) ? fw[fs->canonical_id] : "[]"; fwrite(fws, strlen(fws), 1, f);