From 4c2b4f0c38f8df946efde81407c0f0c18b146162 Mon Sep 17 00:00:00 2001 From: Dillon Date: Tue, 28 Jan 2025 14:38:28 -0800 Subject: [PATCH 1/4] assert loop steps are not 0 --- runtime/evaluate.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/runtime/evaluate.cc b/runtime/evaluate.cc index a3e3c445..fea5aff9 100644 --- a/runtime/evaluate.cc +++ b/runtime/evaluate.cc @@ -408,6 +408,7 @@ class evaluator { SLINKY_NO_INLINE index_t eval_loop_parallel(const loop* op) { interval bounds = eval(op->bounds); index_t step = eval(op->step, 1); + assert(step != 0); std::atomic result = 0; std::size_t n = ceil_div(bounds.max - bounds.min + 1, step); context.reserve(op->sym.id + 1); @@ -431,6 +432,7 @@ class evaluator { SLINKY_NO_INLINE index_t eval_loop_serial(const loop* op) { interval bounds = eval(op->bounds); index_t step = eval(op->step, 1); + assert(step != 0); // TODO(https://github.com/dsharlet/slinky/issues/3): We don't get a reference to context[op->sym] here // because the context could grow and invalidate the reference. This could be fixed by having evaluate // fully traverse the expression to find the max var, and pre-allocate the context up front. It's From d7ae7527a86b263e6c0557660227ef15e792aa02 Mon Sep 17 00:00:00 2001 From: Dillon Date: Tue, 28 Jan 2025 14:38:51 -0800 Subject: [PATCH 2/4] Handle constant_buffer in substitutor --- builder/substitute.cc | 11 +++++++++++ builder/substitute.h | 1 + 2 files changed, 12 insertions(+) diff --git a/builder/substitute.cc b/builder/substitute.cc index aad14c8d..471bac0c 100644 --- a/builder/substitute.cc +++ b/builder/substitute.cc @@ -455,6 +455,17 @@ void substitutor::visit(const make_buffer* op) { } exit_decls(); } +void substitutor::visit(const constant_buffer* op) { + var sym = enter_decl(op->sym); + stmt body = sym.defined() ? mutate(op->body) : op->body; + sym = sym.defined() ? sym : op->sym; + if (sym == op->sym && body.same_as(op->body)) { + set_result(op); + } else { + set_result(constant_buffer::make(sym, op->value, std::move(body))); + } + exit_decls(); +} void substitutor::visit(const slice_buffer* op) { var src = visit_symbol(op->src); diff --git a/builder/substitute.h b/builder/substitute.h index ab47f75e..b66a72c8 100644 --- a/builder/substitute.h +++ b/builder/substitute.h @@ -54,6 +54,7 @@ class substitutor : public node_mutator { void visit(const loop* op) override; void visit(const allocate* op) override; void visit(const make_buffer* op) override; + void visit(const constant_buffer* op) override; void visit(const slice_buffer* op) override; void visit(const slice_dim* op) override; void visit(const crop_buffer* op) override; From 87f77c5d03b3c9668e4d5c8ad732dfe961c20fcc Mon Sep 17 00:00:00 2001 From: Dillon Date: Tue, 28 Jan 2025 15:20:41 -0800 Subject: [PATCH 3/4] Don't construct symbols for sentinel loop --- builder/slide_and_fold_storage.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builder/slide_and_fold_storage.cc b/builder/slide_and_fold_storage.cc index 08ab9c9a..b49ea73c 100644 --- a/builder/slide_and_fold_storage.cc +++ b/builder/slide_and_fold_storage.cc @@ -225,6 +225,8 @@ class slide_and_fold : public stmt_mutator { return true; } + loop_info() = default; + loop_info(node_context& ctx, var sym, std::size_t loop_id, expr orig_min, interval_expr bounds, expr step, int max_workers) : sym(sym), orig_min(orig_min), bounds(bounds), step(step), max_workers(max_workers), @@ -243,7 +245,7 @@ class slide_and_fold : public stmt_mutator { symbol_map>& current_expr_alignment() { return *loops.back().expr_alignment; } slide_and_fold(node_context& ctx) : ctx(ctx), x(ctx.insert_unique("_x")) { - loops.emplace_back(ctx, var(), loop_counter++, expr(), interval_expr::none(), expr(), loop::serial); + loops.emplace_back(loop_info()); } stmt mutate(const stmt& s) override { From ac717d79c5822d66ee74e7c5c1e2305d2349be42 Mon Sep 17 00:00:00 2001 From: Dillon Date: Tue, 28 Jan 2025 21:42:38 -0800 Subject: [PATCH 4/4] Add comment --- builder/optimizations.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/builder/optimizations.cc b/builder/optimizations.cc index 3ca96d9b..38e2212a 100644 --- a/builder/optimizations.cc +++ b/builder/optimizations.cc @@ -1357,7 +1357,10 @@ class reuse_shadows : public stmt_mutator { void visit(const allocate* op) override { visit_buffer_decl(op); } void visit(const make_buffer* op) override { visit_buffer_decl(op); } - void visit(const constant_buffer* op) override { visit_buffer_decl(op, false); } + void visit(const constant_buffer* op) override { + // Constant buffers are not mutable, because the raw_buffer object we use is not allocated by a declaration. + visit_buffer_decl(op, false); + } void visit(const crop_buffer* op) override { visit_buffer_mutator(op); } void visit(const crop_dim* op) override { visit_buffer_mutator(op); } @@ -1405,9 +1408,7 @@ class node_canonicalizer : public node_mutator { } // namespace -expr canonicalize_nodes(const expr& e) { - return node_canonicalizer().mutate(e); -} +expr canonicalize_nodes(const expr& e) { return node_canonicalizer().mutate(e); } stmt canonicalize_nodes(const stmt& s) { scoped_trace trace("canonicalize_nodes"); return node_canonicalizer().mutate(s);