From 8754a09591a3bf4ab51baf15e8511cf331f20c15 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Sun, 19 Jul 2026 23:59:52 -0400 Subject: [PATCH 01/18] Add change_type() directive with reduction-aware overflow checks Add Func::change_type(Type, unsafe), which changes the type at which a Func computes and stores its values. It works eagerly at schedule time by splitting the Func in two: a returned intermediate that copies the Func's definitions but accumulates at the new type (inserting casts, preferring integer forms like widening_mul over float round-trips), and the original Func, rewritten in place into an inline wrapper that casts the intermediate's result back to the original type so every existing consumer is unaffected. Safety is validated with the bounds machinery: for an integer target, change_type() bounds the accumulator by combining the per-term value range (constant_integer_bounds augmented by FuncValueBounds) with the reduction extent. Statically-safe cases pass silently; a case provable only under a runtime precondition (symbolic RDom extent) records that condition, which a new lowering pass (add_type_change_checks, modeled on add_split_factor_checks) injects into the pipeline's assertion block and no_asserts strips. Otherwise change_type() errors unless unsafe=true. Supporting changes: Function::clear_definition() to redefine a Func in place as the wrapper; FuncSchedule carries the injected type_change_checks; get_associative_identity() for retyped reduction identities; StrictifyFloat treats int<->float casts as strict so change_type() won't strip a user's strict_cast. Adds as_binary_operands()/make_binary_op() and select_binary_operand() as reusable binary-operator helpers, placed early in Func.cpp (alongside project_rdom()) so hoist_invariants() can reuse all three without redefining them. Adds a Python binding. --- Makefile | 2 + python_bindings/src/halide/halide_/PyFunc.cpp | 1 + src/AddTypeChangeChecks.cpp | 35 ++ src/AddTypeChangeChecks.h | 29 ++ src/AssociativeOpsTable.cpp | 26 +- src/AssociativeOpsTable.h | 5 + src/CMakeLists.txt | 2 + src/ConstantInterval.cpp | 26 ++ src/ConstantInterval.h | 13 + src/Func.cpp | 380 ++++++++++++++++++ src/Func.h | 27 ++ src/Function.cpp | 15 + src/Function.h | 7 + src/IROperator.cpp | 70 ++++ src/IROperator.h | 9 + src/Lower.cpp | 5 + src/Schedule.cpp | 19 + src/Schedule.h | 10 + src/StrictifyFloat.cpp | 3 +- 19 files changed, 680 insertions(+), 4 deletions(-) create mode 100644 src/AddTypeChangeChecks.cpp create mode 100644 src/AddTypeChangeChecks.h diff --git a/Makefile b/Makefile index 05df3ef8446b..44b98aa9b998 100644 --- a/Makefile +++ b/Makefile @@ -447,6 +447,7 @@ SOURCE_FILES = \ AddImageChecks.cpp \ AddParameterChecks.cpp \ AddSplitFactorChecks.cpp \ + AddTypeChangeChecks.cpp \ AlignLoads.cpp \ AllocationBoundsInference.cpp \ ApplySplit.cpp \ @@ -649,6 +650,7 @@ HEADER_FILES = \ AddImageChecks.h \ AddParameterChecks.h \ AddSplitFactorChecks.h \ + AddTypeChangeChecks.h \ AlignLoads.h \ AllocationBoundsInference.h \ ApplySplit.h \ diff --git a/python_bindings/src/halide/halide_/PyFunc.cpp b/python_bindings/src/halide/halide_/PyFunc.cpp index cccd87bc1789..bf91bdb738b8 100644 --- a/python_bindings/src/halide/halide_/PyFunc.cpp +++ b/python_bindings/src/halide/halide_/PyFunc.cpp @@ -223,6 +223,7 @@ void define_func(py::module &m) { .def("eager_inline", [](Func &func, const py::args &args) -> Func & { return func.eager_inline(args_to_vector(args)); }) + .def("change_type", &Func::change_type, py::arg("type"), py::arg("unsafe") = false) .def("compute_root", &Func::compute_root) .def("store_root", &Func::store_root) diff --git a/src/AddTypeChangeChecks.cpp b/src/AddTypeChangeChecks.cpp new file mode 100644 index 000000000000..a9c3d72609fc --- /dev/null +++ b/src/AddTypeChangeChecks.cpp @@ -0,0 +1,35 @@ +#include "AddTypeChangeChecks.h" +#include "Function.h" +#include "IR.h" +#include "IROperator.h" +#include "Schedule.h" +#include "Simplify.h" + +namespace Halide { +namespace Internal { + +Stmt add_type_change_checks(const Stmt &s, const std::map &env) { + std::vector stmts; + + for (const auto &p : env) { + const Function &f = p.second; + for (const auto &[condition, message] : f.schedule().type_change_checks()) { + if (!condition.defined()) { + continue; + } + Expr proven = simplify(condition); + if (is_const_one(proven)) { + // Statically proven; no runtime check needed. + continue; + } + Expr error = requirement_failed_error(condition, {Expr(message)}); + stmts.push_back(AssertStmt::make(condition, error)); + } + } + + stmts.push_back(s); + return Block::make(stmts); +} + +} // namespace Internal +} // namespace Halide diff --git a/src/AddTypeChangeChecks.h b/src/AddTypeChangeChecks.h new file mode 100644 index 000000000000..194830a7f6b7 --- /dev/null +++ b/src/AddTypeChangeChecks.h @@ -0,0 +1,29 @@ +#ifndef HALIDE_ADD_TYPE_CHANGE_CHECKS_H +#define HALIDE_ADD_TYPE_CHANGE_CHECKS_H + +/** \file + * Defines the lowering pass that injects the overflow-safety preconditions + * recorded by Func::change_type() into the pipeline's assertion block. + */ + +#include +#include + +#include "Expr.h" + +namespace Halide { +namespace Internal { + +class Function; + +/** Prepend assertions for any static preconditions that Func::change_type() + * recorded on the funcs in `env` (that it could not discharge at schedule time, + * e.g. because a reduction extent was symbolic). Statically-true conditions are + * dropped. Like the other check passes, the resulting asserts are removed later + * when the no_asserts target feature is set. */ +Stmt add_type_change_checks(const Stmt &s, const std::map &env); + +} // namespace Internal +} // namespace Halide + +#endif diff --git a/src/AssociativeOpsTable.cpp b/src/AssociativeOpsTable.cpp index 34fb40d93ab5..2f94f018c862 100644 --- a/src/AssociativeOpsTable.cpp +++ b/src/AssociativeOpsTable.cpp @@ -101,6 +101,11 @@ struct TableKey { map> pattern_tables; +std::mutex &ops_table_lock() { + static std::mutex lock; + return lock; +} + #define declare_vars(t, index) \ Expr x##index = Variable::make((t), "x" + std::to_string(index)); \ Expr y##index = Variable::make((t), "y" + std::to_string(index)); \ @@ -354,8 +359,7 @@ const vector &get_ops_table(const vector &exprs) { const vector &table = [&]() -> decltype(auto) { // get_ops_table_helper() lazily initializes the table, so ensure // that multiple threads can't try to do so at the same time. - static std::mutex ops_table_lock; - std::scoped_lock lock_guard(ops_table_lock); + std::scoped_lock lock_guard(ops_table_lock()); return get_ops_table_helper(types, exprs[0].node_type(), exprs.size()); }(); @@ -368,5 +372,23 @@ const vector &get_ops_table(const vector &exprs) { return table; } +std::optional get_associative_identity(Type type, IRNodeType root) { + std::scoped_lock lock_guard(ops_table_lock()); + + const vector &table = get_ops_table_helper({type}, root, 1); + if (table.empty()) { + return std::nullopt; + } + + const Expr &identity = table.front().identities.front(); + for (const AssociativePattern &pattern : table) { + internal_assert(pattern.size() == 1); + if (!equal(pattern.identities.front(), identity)) { + return std::nullopt; + } + } + return identity; +} + } // namespace Internal } // namespace Halide diff --git a/src/AssociativeOpsTable.h b/src/AssociativeOpsTable.h index 9bbb12db7536..ab3568c951d3 100644 --- a/src/AssociativeOpsTable.h +++ b/src/AssociativeOpsTable.h @@ -8,6 +8,7 @@ #include "IREquality.h" #include "IROperator.h" +#include #include #include @@ -71,6 +72,10 @@ struct AssociativePattern { const std::vector &get_ops_table(const std::vector &exprs); +/** Return the identity for a single-output associative op, if the table has one + * and all matching patterns agree on it. */ +std::optional get_associative_identity(Type type, IRNodeType root); + } // namespace Internal } // namespace Halide diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 82cd6bdba5bd..7727588fe764 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -57,6 +57,7 @@ target_sources( AddImageChecks.h AddParameterChecks.h AddSplitFactorChecks.h + AddTypeChangeChecks.h AlignLoads.h AllocationBoundsInference.h ApplySplit.h @@ -239,6 +240,7 @@ target_sources( AddImageChecks.cpp AddParameterChecks.cpp AddSplitFactorChecks.cpp + AddTypeChangeChecks.cpp AlignLoads.cpp AllocationBoundsInference.cpp ApplySplit.cpp diff --git a/src/ConstantInterval.cpp b/src/ConstantInterval.cpp index ab40ab0c5604..ec7e587efbc4 100644 --- a/src/ConstantInterval.cpp +++ b/src/ConstantInterval.cpp @@ -3,6 +3,7 @@ #include "Error.h" #include "IROperator.h" #include "IRPrinter.h" +#include "Interval.h" namespace Halide { namespace Internal { @@ -101,6 +102,14 @@ bool ConstantInterval::contains(uint64_t x) const { } } +bool ConstantInterval::contains(const ConstantInterval &other) const { + // Every value in `other` must lie within this interval. Where `other` is + // unbounded, this must be unbounded on the same side to contain it. + const bool too_small = min_defined && (!other.min_defined || other.min < min); + const bool too_large = max_defined && (!other.max_defined || other.max > max); + return !(too_small || too_large); +} + ConstantInterval ConstantInterval::make_union(const ConstantInterval &a, const ConstantInterval &b) { ConstantInterval result = a; result.include(b); @@ -140,6 +149,23 @@ ConstantInterval ConstantInterval::make_intersection(const ConstantInterval &a, return result; } +ConstantInterval covering_constant_interval(const Interval &in) { + ConstantInterval ci = ConstantInterval::everything(); + if (in.has_lower_bound()) { + if (auto lo = as_const_int(in.min)) { + ci.min_defined = true; + ci.min = *lo; + } + } + if (in.has_upper_bound()) { + if (auto hi = as_const_int(in.max)) { + ci.max_defined = true; + ci.max = *hi; + } + } + return ci; +} + void ConstantInterval::operator+=(const ConstantInterval &other) { (*this) = (*this) + other; } diff --git a/src/ConstantInterval.h b/src/ConstantInterval.h index cc3e6893b3b1..4be6d38bae1e 100644 --- a/src/ConstantInterval.h +++ b/src/ConstantInterval.h @@ -13,6 +13,8 @@ struct Type; namespace Internal { +struct Interval; + /** A class to represent ranges of integers. Can be unbounded above or below, * but they cannot be empty. */ struct ConstantInterval { @@ -64,6 +66,11 @@ struct ConstantInterval { /** Test if the interval contains a particular unsigned value */ bool contains(uint64_t x) const; + /** Test if this interval contains every value of another interval. An + * unbounded side of the other interval is contained only if this interval is + * also unbounded on that side. */ + bool contains(const ConstantInterval &other) const; + /** Construct the smallest interval containing two intervals. */ static ConstantInterval make_union(const ConstantInterval &a, const ConstantInterval &b); @@ -101,6 +108,12 @@ struct ConstantInterval { static ConstantInterval bounds_of_type(Type); }; +/** Convert a symbolic Interval to a ConstantInterval, keeping only endpoints that + * are already constant integers. A symbolic or infinite bound becomes unbounded. + * This does no bounds analysis of its own; any tightening must have already been + * done to the Interval upstream. */ +ConstantInterval covering_constant_interval(const Interval &in); + /** Arithmetic operators on ConstantIntervals. The resulting interval contains * all possible values of the operator applied to any two elements of the * argument intervals. Note that these operator on unbounded integers. If you diff --git a/src/Func.cpp b/src/Func.cpp index 90beb3326a56..800a5e91744c 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -12,8 +13,10 @@ #include "ApplySplit.h" #include "Argument.h" #include "Associativity.h" +#include "Bounds.h" #include "Callable.h" #include "CodeGen_LLVM.h" +#include "ConstantBounds.h" #include "Debug.h" #include "ExprUsesVar.h" #include "FindCalls.h" @@ -21,9 +24,11 @@ #include "Function.h" #include "IR.h" #include "IREquality.h" +#include "IRMatch.h" #include "IRMutator.h" #include "IROperator.h" #include "IRPrinter.h" +#include "IRVisitor.h" #include "ImageParam.h" #include "Inline.h" #include "LLVM_Output.h" @@ -763,6 +768,24 @@ pair project_rdom(const vector &dims, con return {new_rdom, dim_projection}; } +// If `e` is a binary op of node type `op` and exactly one of its two operands +// satisfies `is_selected`, returns {selected operand, other operand}. Returns +// nullopt if `e` isn't a binary `op`, or if neither/both operands match. +template +optional> select_binary_operand(const Expr &e, IRNodeType op, Predicate &&is_selected) { + if (e.node_type() != op) { + return std::nullopt; + } + // `op` is always a binary op, so this is guaranteed to have a value. + auto [a, b] = *as_binary_operands(e); + const bool a_sel = is_selected(a); + const bool b_sel = is_selected(b); + if (a_sel == b_sel) { + return std::nullopt; + } + return a_sel ? std::make_pair(a, b) : std::make_pair(b, a); +} + } // namespace pair, vector> Stage::rfactor_validate_args(const std::vector> &preserved, const AssociativeOp &prover_result) { @@ -3274,6 +3297,363 @@ Func &Func::eager_inline(const std::vector &fs) { return *this; } +// Helpers for change_type implementation +namespace { + +// Does `e` contain a direct Halide call to `fname` (a self-reference)? +bool contains_self_reference(const Expr &e, const string &fname) { + class Finder : public IRGraphVisitor { + using IRGraphVisitor::visit; + const string &fname; + void visit(const Call *c) override { + if (c->call_type == Call::Halide && c->name == fname) { + found = true; + } + IRGraphVisitor::visit(c); + } + + public: + bool found = false; + explicit Finder(const string &f) + : fname(f) { + } + } finder(fname); + e.accept(&finder); + return finder.found; +} + +/** + * Initialize a constant-bounds cache with the FuncValueBounds-derived range + * of every Func call in \p e. The bounds machinery keys its cache by pointer + * identity (\ref Halide::ExprCompare), so using the exact Call nodes that + * appear in \p e lets \ref constant_integer_bounds and \ref lossless_cast see + * a tighter range than just the type's bounds. + * + * @return An initialized constant-bounds cache for \p e + */ +auto cache_call_bounds(const Expr &e, const FuncValueBounds &fvb) { + std::map cache; + if (!fvb.empty()) { + visit_with(e, [&](auto *self, const Call *op) { + self->visit_base(op); // Recurse into the call's arguments. + if (op->call_type != Call::Halide || !op->type.is_int_or_uint()) { + return; + } + auto it = fvb.find({op->name, op->value_index}); + if (it == fvb.end()) { + return; + } + auto type_range = ConstantInterval::bounds_of_type(op->type); + auto value_range = covering_constant_interval(it->second); + cache.emplace(Expr(op), ConstantInterval::make_intersection(type_range, value_range)); + }); + } + return cache; +} + +// Rewrite a factor-free leaf expression `e` to type `t` +Expr retype_leaf(const Expr &e, Type t, const FuncValueBounds &fvb) { + if (e.type() == t) { + return e; + } + + // Expose a single promotion cast for a sum, difference, or product of two + // integer operands that are exactly representable in the float result type, + // e.g. cast(a) * cast(b) == cast(widening_mul(a, b)). When a and + // b round-trip through the float losslessly, the float op rounds the true + // result identically to casting the exact widening op, so this is exact for + // any width and signedness the float can hold (int8/int16 under f32, up to + // int32 under f64, ...). Exposing the widening form lets lossless_cast() below + // carry it to the target integer type as an integer dot-product term. + Expr folded = e; + if (folded.type().is_float() && + (folded.node_type() == IRNodeType::Add || + folded.node_type() == IRNodeType::Sub || + folded.node_type() == IRNodeType::Mul)) { + auto operands = as_binary_operands(folded); + const Cast *ca = operands->first.as(); + const Cast *cb = operands->second.as(); + if (ca && cb && ca->value.type() == cb->value.type() && + ca->value.type().is_int_or_uint() && + folded.type().can_represent(ca->value.type())) { + const Expr &x = ca->value, &y = cb->value; + folded = cast(folded.type(), + folded.node_type() == IRNodeType::Add ? widening_add(x, y) : + folded.node_type() == IRNodeType::Sub ? widening_sub(x, y) : + widening_mul(x, y)); + } + } + + // Peel an int->float promotion: we're accumulating at an integer type, so + // the float round-trip is dead weight. This also exposes an integer form + // (e.g. cast(widening_mul(a, b)) -> widening_mul(a, b)) that + // lossless_cast() can retype without a detour through float, which f32 + // can't always undo. A strict_cast is a Call, not a Cast, and is left alone. + if (t.is_int_or_uint()) { + if (const Cast *c = folded.as()) { + if (folded.type().is_float() && c->value.type().is_int_or_uint()) { + folded = c->value; + } + } + } + + // Retype via lossless_cast() when it can prove the cast exact, pushing it down + // through widening intrinsics so integer forms survive to instruction + // selection. Seeding the cache with producer value ranges lets it succeed for + // casts that are only exact under those ranges (e.g. narrowing a clamped + // producer). This is a no-op-or-improvement for any target type: a float + // target just takes lossless_cast()'s representable-widening path, and + // anything it can't prove falls through to the plain cast below. + auto cache = cache_call_bounds(folded, fvb); + if (Expr r = lossless_cast(t, folded, Scope::empty_scope(), &cache); + r.defined()) { + return r; + } + + return cast(t, folded); +} + +// Retype a whole definition value to type `t`, retargeting self-references from +// `fname` to `dst` and pushing casts down to the increment leaves. Only reduction +// updates shaped as a tree of binary combiners over a self-reference and an +// increment are supported. +Expr retype_value(const Expr &e, const string &fname, const Function &dst, Type t, + const FuncValueBounds &fvb) { + if (const Call *c = e.as()) { + if (c->call_type == Call::Halide && c->name == fname) { + return Call::make(dst, c->args, c->value_index); + } + } + if (contains_self_reference(e, fname)) { + optional> operands = as_binary_operands(e); + user_assert(operands) + << "change_type() only supports update definitions built from binary " + << "operators over the accumulator; " << fname << " has an unsupported shape.\n"; + return make_binary_op(e.node_type(), + retype_value(operands->first, fname, dst, t, fvb), + retype_value(operands->second, fname, dst, t, fvb)); + } + return retype_leaf(e, t, fvb); +} + +// The top-level associative combiner of a (let-stripped) reduction update value, +// i.e. the node type of the binary op whose operands are the self-reference and +// the increment. Returns nullopt if `val` isn't such a shape. +optional reduction_op(const Expr &val, const string &fname) { + optional> split = select_binary_operand(val, val.node_type(), [&](const Expr &e) { + return contains_self_reference(e, fname); + }); + return split ? std::make_optional(val.node_type()) : std::nullopt; +} + +// Prove that computing `typed`'s reduction at type `t` cannot overflow. Returns +// true if it is safe; if safety can only be guaranteed under a runtime +// precondition, that condition is returned in *condition. Returns an error +// message if it cannot be proven. +std::optional change_type_prove_safe( + const Func &typed, Type t, const FuncValueBounds &fvb, Expr *condition // +) { + *condition = Expr(); + const Function fn = typed.function(); + const ConstantInterval limit = ConstantInterval::bounds_of_type(t); + + // Bound `e` using constant integer bounds, refined by the proven value ranges + // of any producer Funcs it references (e.g. a clamp upstream). retype_leaf() + // wraps a leaf in a cast to t when it can't prove the cast lossless; we bound + // the pre-cast value so a truncating narrowing shows its true range instead of + // being hidden by the cast clamping to t. (lossless_cast() never introduces a + // narrowing outer cast, so stripping one here only ever exposes that fallback.) + auto bounds_of = [&](const Expr &e) { + Expr v = e; + if (const Cast *c = v.as()) { + v = c->value; + } + auto cache = cache_call_bounds(v, fvb); + return constant_integer_bounds(v, Scope::empty_scope(), &cache); + }; + + // Pure / identity values must be representable at the new type. + for (const Expr &v : fn.values()) { + if (!limit.contains(bounds_of(v))) { + return "the initial value may not be representable in the target type"; + } + } + + for (const Definition &def : fn.updates()) { + Expr val = substitute_in_all_lets(def.values()[0]); + optional op = reduction_op(val, fn.name()); + + // The increment is the non-self-reference operand of the combiner. + Expr increment = val; + if (op) { + optional> split = select_binary_operand(val, *op, [&](const Expr &e) { + return contains_self_reference(e, fn.name()); + }); + if (split) { + increment = split->second; + } + } + const ConstantInterval term = bounds_of(increment); + + // Each term must itself be representable at the new type. Otherwise the + // cast retype_leaf() wrapped this leaf in truncates it, silently changing + // the result. This also rejects a term with unbounded magnitude, which no + // reduction extent could make safe. + if (!limit.contains(term)) { + return "a term may not be representable in the target type"; + } + + // min / max / and / or leave the accumulator within a single term's range, + // which we just proved fits, so they need nothing more. + if (op && (*op == IRNodeType::Min || *op == IRNodeType::Max || + *op == IRNodeType::And || *op == IRNodeType::Or)) { + continue; + } + + // Sum and difference both grow the accumulator additively and are bounded + // below; every other combiner can grow it faster than a single term's + // range in a way we don't model -- a product reduction most importantly, + // or an unrecognized shape -- so reject it rather than silently overflow. + if (!op || (*op != IRNodeType::Add && *op != IRNodeType::Sub)) { + return "change_type() only supports sum, difference, min, max, and, " + "and or reductions; this reduction's accumulator could overflow " + "the target type"; + } + + // Each reduction step adds (Add) or subtracts (Sub) a term, so bound the + // accumulator by (number of terms) x (per-step contribution). + const ConstantInterval step = (*op == IRNodeType::Sub) ? -term : term; + int64_t n_max = 1; + bool symbolic = false; + Expr n_terms = make_const(Int(64), 1); + for (const auto &rv : def.schedule().rvars()) { + n_terms = simplify(n_terms * cast(Int(64), rv.extent)); + // Only a literal extent is known at compile time; a symbolic extent + // (e.g. an ImageParam dimension) gets only type-based bounds, which we + // must not treat as a static bound. + if (optional ext = as_const_int(simplify(rv.extent)); ext && *ext >= 0) { + n_max *= *ext; + } else { + symbolic = true; + } + } + if (!symbolic) { + if (!limit.contains(step * ConstantInterval(0, n_max))) { + return "the accumulated sum may exceed the target type's range"; + } + continue; + } + // Symbolic term count: emit a runtime precondition instead. step's + // endpoints are defined because term's are (checked above). + Expr cond = (make_const(Int(64), step.max) * n_terms <= make_const(Int(64), limit.max)) && + (make_const(Int(64), step.min) * n_terms >= make_const(Int(64), limit.min)); + *condition = condition->defined() ? (*condition && cond) : cond; + } + return std::nullopt; +} + +} // namespace + +Func Func::change_type(Type t, bool unsafe) { + user_assert(defined()) << "change_type() called on undefined Func.\n"; + user_assert(!func.has_extern_definition()) + << "change_type() cannot be applied to the extern Func " << name() << ".\n"; + user_assert(outputs() == 1) + << "change_type() currently supports only single-output Funcs, but " + << name() << " has " << outputs() << " outputs.\n"; + + invalidate_cache(); + + const Type old_t = func.output_types()[0]; + if (old_t == t) { + return *this; + } + + const string fname = func.name(); + const vector pure_vars = args(); + const vector pure_arg_exprs(pure_vars.begin(), pure_vars.end()); + + // Proven value ranges for this Func's producers. Retyping references the same + // producers (only self-references and casts are rewritten), so bounds keyed + // by the original Funcs line up with the calls in the retyped clone. This + // lets a clamped producer tighten both the cast rewrites and the overflow proof. + FuncValueBounds func_bounds = [&] { + const map env = find_transitive_calls(func); + return compute_function_value_bounds(topological_order({func}, env), env); + }(); + + // Determine the reduction op (if any), so min/max accumulations get the + // right identity at the new type rather than a lossy cast of e.g. +inf. + optional op; + if (func.has_update_definition()) { + op = reduction_op(substitute_in_all_lets(func.update(0).values()[0]), fname); + } + const bool is_min_max = op && (*op == IRNodeType::Min || *op == IRNodeType::Max); + + // Build the retyped clone. + Func typed(fname + "_typed"); + + // Pure definition. + { + vector retyped; + for (const Expr &v : func.values()) { + if (is_min_max) { + optional id = get_associative_identity(t, *op); + user_assert(id) << "change_type() could not find an identity for " + << IRNodeType_string(*op) << " at type " << t << ".\n"; + retyped.push_back(*id); + } else { + retyped.push_back(retype_leaf(v, t, func_bounds)); + } + } + // Single-output only (asserted above), so there is exactly one value. + typed(pure_vars) = retyped[0]; + } + + // Update definitions. The retyped values still reference the original + // reduction domain, so pass a default domain and let define_update discover + // it from the values (passing a freshly-built one would trip its identity + // check). + for (size_t u = 0; u < func.updates().size(); u++) { + const Definition &def = func.update(u); + vector vals; + vals.reserve(def.values().size()); + for (const Expr &v : def.values()) { + vals.push_back(retype_value(substitute_in_all_lets(v), fname, typed.function(), t, func_bounds)); + } + typed.function().define_update(def.args(), vals, ReductionDomain{}); + typed.function().update(u).schedule() = def.schedule().get_copy(); + } + + // Safety check. + if (!unsafe && t.is_int_or_uint()) { + Expr condition; + const auto err = change_type_prove_safe(typed, t, func_bounds, &condition); + user_assert(!err) + << "change_type(" << t << ") on " << fname << " may overflow: " << *err << ".\n" + << "Pass unsafe=true to change_type() to bypass this check.\n"; + if (condition.defined()) { + std::ostringstream msg; + msg << "change_type(" << t << ") on " << fname + << " requires the reduction extent to be small enough not to overflow"; + typed.function().schedule().type_change_checks().emplace_back(condition, msg.str()); + } + } + + // Rewrite this Func into an inline cast-back wrapper of the retyped clone, so + // that every existing consumer keeps seeing the original type. + const Expr wrapped = cast(old_t, Call::make(typed.function(), pure_arg_exprs, 0)); + vector arg_names; + arg_names.reserve(pure_vars.size()); + for (const Var &v : pure_vars) { + arg_names.push_back(v.name()); + } + func.clear_definition(); + func.define(arg_names, {wrapped}); + + return typed; +} + Func &Func::trace_loads() { invalidate_cache(); func.trace_loads(); diff --git a/src/Func.h b/src/Func.h index 347bca1d77a3..aceed46fe59f 100644 --- a/src/Func.h +++ b/src/Func.h @@ -2653,6 +2653,33 @@ class Func { */ Func &compute_inline(); + /** Change the type at which this Func computes and stores its values, + * subject to a reduction-aware safety check. + * + * This splits the Func in two: a new intermediate Func (returned) that + * copies this Func's definitions but accumulates at the requested type `t` + * (with the appropriate casts inserted, preferring integer forms such as + * widening_mul over float round-trips), and this Func, which is rewritten in + * place into an inline wrapper that casts the intermediate's result back to + * the original type. Every existing consumer therefore keeps seeing the + * original type, while the returned intermediate can be scheduled to exploit + * the new type (e.g. an Int(32) accumulator eligible for dot-product + * instructions). Schedule the returned Func to control the retyped + * computation. + * + * The change is validated with the bounds machinery: for an integer target, + * change_type() proves the accumulation cannot overflow by combining the + * per-term value range with the reduction extent. If it can only be + * guaranteed under a runtime precondition (e.g. the RDom extent isn't too + * wide), that precondition is injected into the pipeline's assertion block + * (and removed by the no_asserts target feature). If safety cannot be + * established, change_type() errors unless `unsafe` is true, which bypasses + * the check entirely. + * + * Currently supports single-output Funcs whose update definitions are built + * from binary operators over the accumulator. */ + Func change_type(Type t, bool unsafe = false); + /** Immediately inline direct calls to each of the given Funcs into this * Func's initial (pure) definition. The Funcs are inlined in dependency * order regardless of the order they are passed, so if one inlined Func's diff --git a/src/Function.cpp b/src/Function.cpp index 15674b18f353..b5f38c0a233e 100644 --- a/src/Function.cpp +++ b/src/Function.cpp @@ -707,6 +707,21 @@ void Function::define(const vector &args, vector values) { } } +void Function::clear_definition() { + contents->output_types.clear(); + contents->args.clear(); + contents->func_schedule = FuncSchedule(); + contents->init_def = Definition(); + contents->updates.clear(); + contents->output_buffers.clear(); + contents->extern_arguments.clear(); + contents->extern_function_name.clear(); + contents->extern_mangling = NameMangling::Default; + contents->extern_function_device_api = DeviceAPI::Host; + contents->extern_proxy_expr = Expr(); + contents->frozen = false; +} + void Function::create_output_buffers(const std::vector &types, int dims) const { internal_assert(contents->output_buffers.empty()); internal_assert(!types.empty() && dims != AnyDims); diff --git a/src/Function.h b/src/Function.h index a7344d6a61c1..7f9df67c2bf8 100644 --- a/src/Function.h +++ b/src/Function.h @@ -120,6 +120,13 @@ class Function { * reduction domain */ void define(const std::vector &args, std::vector values); + /** Reset this Function to an undefined state in place (clearing all pure, + * update, and extern definitions, output types/buffers, and schedule) while + * preserving the Function's object identity, so existing references to it + * remain valid and it can be given a fresh definition with define(). Used by + * Func::change_type() to turn the original Func into an inline wrapper. */ + void clear_definition(); + /** Add an update definition to this function. It must already have a pure * definition but not an update definition, and the length of args must * match the length of args used in the pure definition. 'value' may depend diff --git a/src/IROperator.cpp b/src/IROperator.cpp index a676b7f9f933..2cdf8a6f0ff7 100644 --- a/src/IROperator.cpp +++ b/src/IROperator.cpp @@ -247,6 +247,76 @@ std::optional is_const_power_of_two_integer(int64_t val) { return val < 0 ? std::nullopt : is_const_power_of_two_integer((uint64_t)val); } +std::optional> as_binary_operands(const Expr &e) { + // We switch on the actual node type, so we can downcast e.get() directly + // rather than going through Expr::as<>(), which would redundantly re-check + // the node type the switch case has already established. + switch (e.node_type()) { +#define HANDLE_BINARY_OP(NodeType) \ + case IRNodeType::NodeType: { \ + const NodeType *op = static_cast(e.get()); \ + return std::pair{op->a, op->b}; \ + } + HANDLE_BINARY_OP(Add) + HANDLE_BINARY_OP(Sub) + HANDLE_BINARY_OP(Mul) + HANDLE_BINARY_OP(Div) + HANDLE_BINARY_OP(Mod) + HANDLE_BINARY_OP(Min) + HANDLE_BINARY_OP(Max) + HANDLE_BINARY_OP(EQ) + HANDLE_BINARY_OP(NE) + HANDLE_BINARY_OP(LT) + HANDLE_BINARY_OP(LE) + HANDLE_BINARY_OP(GT) + HANDLE_BINARY_OP(GE) + HANDLE_BINARY_OP(And) + HANDLE_BINARY_OP(Or) +#undef HANDLE_BINARY_OP + default: + return std::nullopt; + } +} + +Expr make_binary_op(IRNodeType t, const Expr &a, const Expr &b) { + switch (t) { + case IRNodeType::Add: + return a + b; + case IRNodeType::Sub: + return a - b; + case IRNodeType::Mul: + return a * b; + case IRNodeType::Div: + return a / b; + case IRNodeType::Mod: + return a % b; + case IRNodeType::Min: + return min(a, b); + case IRNodeType::Max: + return max(a, b); + case IRNodeType::EQ: + return a == b; + case IRNodeType::NE: + return a != b; + case IRNodeType::LT: + return a < b; + case IRNodeType::LE: + return a <= b; + case IRNodeType::GT: + return a > b; + case IRNodeType::GE: + return a >= b; + case IRNodeType::And: + return a && b; + case IRNodeType::Or: + return a || b; + default: + internal_error << "make_binary_op: " << IRNodeType_string(t) + << " is not a binary operator\n"; + return Expr(); + } +} + bool is_positive_const(const Expr &e) { if (const IntImm *i = e.as()) { return i->value > 0; diff --git a/src/IROperator.h b/src/IROperator.h index f377522d4b07..b9499873c018 100644 --- a/src/IROperator.h +++ b/src/IROperator.h @@ -10,6 +10,7 @@ #include #include #include +#include #include "ConstantInterval.h" #include "Expr.h" @@ -50,6 +51,14 @@ std::optional is_const_power_of_two_integer(uint64_t); std::optional is_const_power_of_two_integer(int64_t); // @} +/** If `e` is a binary operator, return its two operands; otherwise return std::nullopt. */ +std::optional> as_binary_operands(const Expr &e); + +/** Build a binary expression of node type `t` from operands `a` and `b`, using + * the corresponding operator overload (so the usual type matching and constant + * folding apply). `t` must be a binary operator; it is an internal error otherwise. */ +Expr make_binary_op(IRNodeType t, const Expr &a, const Expr &b); + /** Is the expression a const (as defined by is_const), and also * strictly greater than zero (in all lanes, if a vector expression) */ bool is_positive_const(const Expr &e); diff --git a/src/Lower.cpp b/src/Lower.cpp index a370336e3961..e179376e05c9 100644 --- a/src/Lower.cpp +++ b/src/Lower.cpp @@ -11,6 +11,7 @@ #include "AddImageChecks.h" #include "AddParameterChecks.h" #include "AddSplitFactorChecks.h" +#include "AddTypeChangeChecks.h" #include "AllocationBoundsInference.h" #include "AsyncProducers.h" #include "BoundConstantExtentLoops.h" @@ -209,6 +210,10 @@ void lower_impl(const vector &output_funcs, s = add_split_factor_checks(s, env); log("Lowering after asserting that all split factors are positive:", s); + debug(1) << "Asserting change_type() accumulations cannot overflow...\n"; + s = add_type_change_checks(s, env); + log("Lowering after asserting change_type() accumulations cannot overflow:", s); + debug(1) << "Removing extern loops...\n"; s = remove_extern_loops(s); log("Lowering after removing extern loops:", s); diff --git a/src/Schedule.cpp b/src/Schedule.cpp index ca7bcb9e98e8..948233112b7c 100644 --- a/src/Schedule.cpp +++ b/src/Schedule.cpp @@ -244,6 +244,11 @@ struct FuncScheduleContents { // This is an extent of the ring buffer and expected to be a positive integer. Expr ring_buffer; Expr memoize_eviction_key; + // Static preconditions injected by change_type() that must hold for the + // retyped accumulation not to overflow. Each is a (condition, message) pair; + // a lowering pass turns them into assertions in the pipeline's initial + // assertion block (removed by the no_asserts target feature). + std::vector> type_change_checks; FuncScheduleContents() : store_level(LoopLevel::inlined()), compute_level(LoopLevel::inlined()), hoist_storage_level(LoopLevel::inlined()) { @@ -279,6 +284,11 @@ struct FuncScheduleContents { b.remainder = mutator(b.remainder); } } + for (auto &check : type_change_checks) { + if (check.first.defined()) { + check.first = mutator(check.first); + } + } } }; @@ -369,6 +379,7 @@ FuncSchedule FuncSchedule::deep_copy( copy.contents->memoize_eviction_key = contents->memoize_eviction_key; copy.contents->async = contents->async; copy.contents->ring_buffer = contents->ring_buffer; + copy.contents->type_change_checks = contents->type_change_checks; // Deep-copy wrapper functions. In a partial deep-copy (e.g. cloning a // single Func via clone_in), the wrapper Funcs may not be among the Funcs @@ -423,6 +434,14 @@ Expr &FuncSchedule::ring_buffer() const { return contents->ring_buffer; } +const std::vector> &FuncSchedule::type_change_checks() const { + return contents->type_change_checks; +} + +std::vector> &FuncSchedule::type_change_checks() { + return contents->type_change_checks; +} + std::vector &FuncSchedule::storage_dims() { return contents->storage_dims; } diff --git a/src/Schedule.h b/src/Schedule.h index a951fe9b718f..ba3d1eea5ca3 100644 --- a/src/Schedule.h +++ b/src/Schedule.h @@ -608,6 +608,16 @@ class FuncSchedule { Expr &ring_buffer(); Expr &ring_buffer() const; + /** Static preconditions injected by Func::change_type() that guarantee the + * retyped accumulation cannot overflow. Each entry is a (condition, message) + * pair; a lowering pass (add_type_change_checks) asserts them in the + * pipeline's initial assertion block, and they are removed by the no_asserts + * target feature. */ + // @{ + const std::vector> &type_change_checks() const; + std::vector> &type_change_checks(); + // @} + /** The list and order of dimensions used to store this * function. The first dimension in the vector corresponds to the * innermost dimension for storage (i.e. which dimension is diff --git a/src/StrictifyFloat.cpp b/src/StrictifyFloat.cpp index 9deb86679808..40dc753655e7 100644 --- a/src/StrictifyFloat.cpp +++ b/src/StrictifyFloat.cpp @@ -85,8 +85,7 @@ class Strictify : public IRMutator { } Expr visit(const Cast *op) override { - if (op->value.type().is_float() && - op->type.is_float()) { + if (op->value.type().is_float() || op->type.is_float()) { return Call::make(op->type, Call::strict_cast, {mutate(op->value)}, Call::PureIntrinsic); } else { From db74af9bcc23393fd1ac18706fa0dc0013c83975 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Sun, 19 Jul 2026 23:59:52 -0400 Subject: [PATCH 02/18] Add tests for change_type() directive Co-Authored-By: Claude Opus 4.8 (1M context) --- test/correctness/CMakeLists.txt | 1 + test/correctness/change_type.cpp | 592 +++++++++++++++++++++++++++++++ 2 files changed, 593 insertions(+) create mode 100644 test/correctness/change_type.cpp diff --git a/test/correctness/CMakeLists.txt b/test/correctness/CMakeLists.txt index 48a5c336be9d..21dadb8d45b0 100644 --- a/test/correctness/CMakeLists.txt +++ b/test/correctness/CMakeLists.txt @@ -45,6 +45,7 @@ tests( cascaded_filters.cpp cast.cpp cast_handle.cpp + change_type.cpp chunk.cpp chunk_sharing.cpp circular_reference_leak.cpp diff --git a/test/correctness/change_type.cpp b/test/correctness/change_type.cpp new file mode 100644 index 000000000000..25486ceedee7 --- /dev/null +++ b/test/correctness/change_type.cpp @@ -0,0 +1,592 @@ +#include "Halide.h" +#include +#include +#include +#include +#include + +using namespace Halide; + +namespace { + +// TODO: add tests that compose change_type() with hoist_invariants() -- e.g. +// retyping the float dot-product intermediate hoist_invariants() returns to an +// Int(32) accumulator, and confirming a min-reduction retype uses the +// reduction identity at the new type rather than a lossy cast of the original +// float identity. + +// A symbolic reduction extent can't be bounded at schedule time, so change_type +// injects a runtime precondition. With a valid (small) extent it passes and the +// result is correct. +int change_type_symbolic_extent_test() { + ImageParam A{Int(8), 1, "A"}, B{Int(8), 1, "B"}; + + Var i{"i"}; + // The extent is a runtime value (an ImageParam dimension), so it can't be + // bounded at schedule time. + RDom r(0, A.dim(0).extent(), "r"); + + Func Acc{"Acc"}; + Acc(i) = 0.0f; + Acc(i) += cast(widening_mul(A(r), B(r))); + + // int8*int8 accumulated over a symbolic number of terms: change_type injects + // a runtime precondition guaranteeing the sum fits in Int(32). + Func Acc_i32 = Acc.change_type(Int(32)); + internal_assert(Acc_i32.types()[0] == Int(32)) + << "change_type symbolic: expected Int(32), got " << Acc_i32.types()[0] << "\n"; + Acc_i32.compute_root(); + + const int K = 100; + Buffer a(K), b(K); + for (int k = 0; k < K; k++) { + a(k) = (int8_t)((k % 9) - 4); + b(k) = (int8_t)((k % 7) - 3); + } + A.set(a); + B.set(b); + + Buffer result = Acc.realize({4}); + int32_t dot = 0; + for (int k = 0; k < K; k++) { + dot += (int32_t)a(k) * (int32_t)b(k); + } + for (int m = 0; m < 4; m++) { + float expected = (float)dot; + internal_assert(result(m) == expected) + << "change_type symbolic-extent mismatch at " << m << ": " << result(m) + << " vs " << expected << "\n"; + } + return 0; +} + +// change_type() can be applied more than once, retyping the intermediate +// returned by a previous change_type(). Each step must remain safe and correct. +int change_type_twice_test() { + const int K = 32; + ImageParam A{Int(8), 1, "A"}; + + Var i{"i"}; + RDom r(0, K, "r"); + + Func Acc{"Acc"}; + Acc(i) = 0.0f; + // Sum of K int8 values: |sum| <= 32 * 127 = 4064, which fits Int(16), so both + // retypes (Float(32) -> Int(32) -> Int(16)) are statically safe. + Acc(i) += cast(A(r)); + + Func Acc_i32 = Acc.change_type(Int(32)); + Func Acc_i16 = Acc_i32.change_type(Int(16)); + internal_assert(Acc.types()[0] == Float(32) && + Acc_i32.types()[0] == Int(32) && + Acc_i16.types()[0] == Int(16)) + << "change_type twice: unexpected types " + << Acc.types()[0] << " / " << Acc_i32.types()[0] << " / " << Acc_i16.types()[0] << "\n"; + Acc_i16.compute_root(); + Acc_i32.compute_root(); + + Buffer a(K); + for (int k = 0; k < K; k++) { + a(k) = (int8_t)((k % 15) - 7); + } + A.set(a); + + Buffer result = Acc.realize({2}); + int32_t sum = 0; + for (int k = 0; k < K; k++) { + sum += (int32_t)a(k); + } + for (int m = 0; m < 2; m++) { + internal_assert(result(m) == (float)sum) + << "change_type twice mismatch at " << m << ": " << result(m) << " vs " << sum << "\n"; + } + return 0; +} + +// A statically-sized accumulation whose per-term range is only small enough to +// fit the target type because an upstream producer clamps its value. Without the +// producer's proven bounds, change_type() sees the term's full type range and +// (correctly) rejects the retype as an overflow risk. This test therefore relies +// on change_type() consulting FuncValueBounds, and checks the result is correct. +int change_type_producer_bounds_test() { + const int K = 1000; + ImageParam A{Int(16), 1, "A"}; + + Var i{"i"}, x{"x"}; + RDom r(0, K, "r"); + + // clamp(A, 0, 10) has a proven value range of [0, 10], so a sum of K of them + // is at most 10 * 1000 = 10000, which fits Int(16). A raw Int(16) term would + // span [-32768, 32767], and K of those would blow past Int(16). + Func p{"p"}; + p(x) = clamp(A(x), 0, 10); + + Func Acc{"Acc"}; + Acc(i) = 0.0f; + Acc(i) += cast(p(r)); + + Func Acc_i16 = Acc.change_type(Int(16)); + internal_assert(Acc_i16.types()[0] == Int(16)) + << "change_type producer-bounds: expected Int(16), got " << Acc_i16.types()[0] << "\n"; + p.compute_root(); + Acc_i16.compute_root(); + + Buffer a(K); + for (int k = 0; k < K; k++) { + // Spread values well outside [0, 10] so the clamp actually bites and a + // missing clamp would give a different (and overflowing) answer. + a(k) = (int16_t)(((k * 37) % 400) - 150); + } + A.set(a); + + Buffer result = Acc.realize({4}); + int32_t sum = 0; + for (int k = 0; k < K; k++) { + sum += std::min(std::max((int32_t)a(k), 0), 10); + } + for (int m = 0; m < 4; m++) { + internal_assert(result(m) == (float)sum) + << "change_type producer-bounds mismatch at " << m << ": " << result(m) + << " vs " << sum << "\n"; + } + +#if HALIDE_WITH_EXCEPTIONS + // The same reduction without the clamp is a genuine overflow risk: a sum of K + // raw Int(16) terms does not fit Int(16). change_type() must reject it, which + // confirms the test above passed because of the producer's bounds and not for + // some unrelated reason. + if (Halide::exceptions_enabled()) { + ImageParam B{Int(16), 1, "B"}; + Func q{"q"}; + q(x) = B(x); // no clamp -> full Int(16) value range + + Func Acc2{"Acc2"}; + Acc2(i) = 0.0f; + Acc2(i) += cast(q(r)); + + bool threw = false; + try { + Acc2.change_type(Int(16)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type without producer bounds should have been rejected as an overflow risk\n"; + } +#endif + + return 0; +} + +// A dot product of two clamped producers. The widening_mul term is only provably +// within Int(32) because both producers are clamped, and this also exercises the +// retype_leaf/lossless_cast interaction: the float-wrapped widening_mul must come +// back as an integer widening_mul (no float round-trip) at the new type. +int change_type_bounded_dot_product_test() { + const int K = 8; + ImageParam A{Int(16), 1, "A"}, B{Int(16), 1, "B"}; + + Var i{"i"}, x{"x"}; + RDom r(0, K, "r"); + + Func pa{"pa"}, pb{"pb"}; + pa(x) = clamp(A(x), 0, 100); + pb(x) = clamp(B(x), 0, 100); + + Func Acc{"Acc"}; + Acc(i) = 0.0f; + Acc(i) += cast(pa(r)) * cast(pb(r)); + + // Per-term product is at most 100 * 100 = 10000; over K = 8 terms that is at + // most 80000, comfortably inside Int(32). A raw Int(16) x Int(16) product + // spans up to ~2^30, and K of those would overflow Int(32). + Func Acc_i32 = Acc.change_type(Int(32)); + internal_assert(Acc_i32.types()[0] == Int(32)) + << "change_type bounded dot-product: expected Int(32), got " << Acc_i32.types()[0] << "\n"; + pa.compute_root(); + pb.compute_root(); + Acc_i32.compute_root(); + + Buffer a(K), b(K); + for (int k = 0; k < K; k++) { + a(k) = (int16_t)((k * 53) % 500 - 200); + b(k) = (int16_t)((k * 71) % 500 - 200); + } + A.set(a); + B.set(b); + + Buffer result = Acc.realize({4}); + int32_t dot = 0; + for (int k = 0; k < K; k++) { + int32_t ca = std::min(std::max((int32_t)a(k), 0), 100); + int32_t cb = std::min(std::max((int32_t)b(k), 0), 100); + dot += ca * cb; + } + for (int m = 0; m < 4; m++) { + internal_assert(result(m) == (float)dot) + << "change_type bounded dot-product mismatch at " << m << ": " << result(m) + << " vs " << dot << "\n"; + } + return 0; +} + +// change_type() on a pure Func that narrows to a type its values fit only after +// a clamp. lossless_cast() can't prove the narrowing (it can't push the cast into +// the min(A, 100) subterm, whose range spills below Int(8)), so retype_leaf() +// falls back to a plain cast -- which is nonetheless exact here because the clamp +// keeps every value in [0, 100]. This exercises the retype_leaf() fallback path. +int change_type_pure_narrowing_test() { + ImageParam A{Int(16), 1, "A"}; + + Var x{"x"}; + Func f{"f"}; + f(x) = clamp(A(x), 0, 100); + + Func f_i8 = f.change_type(Int(8)); + internal_assert(f_i8.types()[0] == Int(8)) + << "change_type pure-narrowing: expected Int(8), got " << f_i8.types()[0] << "\n"; + f_i8.compute_root(); + + const int W = 64; + Buffer a(W); + for (int j = 0; j < W; j++) { + a(j) = (int16_t)(((j * 41) % 600) - 250); // spans well past [0, 100] + } + A.set(a); + + Buffer result = f.realize({W}); + for (int j = 0; j < W; j++) { + int16_t expected = (int16_t)std::min(std::max((int)a(j), 0), 100); + internal_assert(result(j) == expected) + << "change_type pure-narrowing mismatch at " << j << ": " << result(j) + << " vs " << expected << "\n"; + } + return 0; +} + +// A dot product of two clamped producers retyped to Int(16) -- narrower than the +// natural Int(32) product. The producer bounds do two things here. For +// acceptance: the overflow proof only fits the K-term accumulator in Int(16) +// because each product is in [0, 10000] (a raw Int(16)^2 term would blow past +// Int(16)); without the clamps change_type() rejects the retype. For code +// quality: seeding those bounds into lossless_cast() lets it push the cast into +// the widening_mul and recover a narrow Int(16) multiply rather than falling back +// to a cast of the Int(32) product. That second effect is not observable here -- +// the fallback is value-equal whenever the product fits Int(16), so only the +// generated IR differs -- but this exercises that lossless_cast() path. +int change_type_narrowing_dot_product_test() { + const int K = 3; + ImageParam A{Int(16), 1, "A"}, B{Int(16), 1, "B"}; + + Var i{"i"}, x{"x"}; + RDom r(0, K, "r"); + + Func pa{"pa"}, pb{"pb"}; + pa(x) = clamp(A(x), 0, 100); + pb(x) = clamp(B(x), 0, 100); + + Func Acc{"Acc"}; + Acc(i) = 0.0f; + Acc(i) += cast(pa(r)) * cast(pb(r)); + + // Per-term product <= 10000; over K = 3 terms the sum <= 30000, which fits + // Int(16). The per-term bound is what lets the widening_mul narrow to Int(16). + Func Acc_i16 = Acc.change_type(Int(16)); + internal_assert(Acc_i16.types()[0] == Int(16)) + << "change_type narrowing dot-product: expected Int(16), got " << Acc_i16.types()[0] << "\n"; + pa.compute_root(); + pb.compute_root(); + Acc_i16.compute_root(); + + Buffer a(K), b(K); + for (int k = 0; k < K; k++) { + a(k) = (int16_t)((k * 53) % 500 - 200); + b(k) = (int16_t)((k * 71) % 500 - 200); + } + A.set(a); + B.set(b); + + Buffer result = Acc.realize({4}); + int32_t dot = 0; + for (int k = 0; k < K; k++) { + int32_t ca = std::min(std::max((int32_t)a(k), 0), 100); + int32_t cb = std::min(std::max((int32_t)b(k), 0), 100); + dot += ca * cb; + } + for (int m = 0; m < 4; m++) { + internal_assert(result(m) == (float)dot) + << "change_type narrowing dot-product mismatch at " << m << ": " << result(m) + << " vs " << dot << "\n"; + } + return 0; +} + +// A narrowing change_type() that can't be proven exact must be rejected rather +// than silently truncating -- unless the caller opts in with unsafe = true. This +// covers both the pure path and the min/max reduction path, whose per-term casts +// would otherwise clamp their own bounds and hide the truncation. +int change_type_truncating_rejected_test() { +#if HALIDE_WITH_EXCEPTIONS + if (!Halide::exceptions_enabled()) { + return 0; + } + Var i{"i"}, x{"x"}; + + // Pure narrowing of an unbounded Int(16) value to Int(8): not representable. + { + ImageParam A{Int(16), 1, "A"}; + Func f{"f_trunc"}; + f(x) = A(x); + + bool threw = false; + try { + f.change_type(Int(8)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type(Int(8)) on an unbounded Int(16) value should be rejected as truncating\n"; + + // With unsafe = true the caller takes responsibility and it is allowed. + Func g{"g_trunc"}; + g(x) = A(x); + Func g_i8 = g.change_type(Int(8), /*unsafe*/ true); + internal_assert(g_i8.types()[0] == Int(8)) + << "unsafe change_type should proceed despite possible truncation\n"; + } + + // A max-reduction over an unbounded Int(16) term narrowed to Int(8): the term + // itself may not be representable, so it must be rejected too. + { + ImageParam A{Int(16), 1, "A"}; + RDom r(0, 8, "r"); + Func m{"m_trunc"}; + m(i) = cast(A(0)); + m(i) = max(m(i), cast(A(r))); + + bool threw = false; + try { + m.change_type(Int(8)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type(Int(8)) on an unbounded max-reduction term should be rejected as truncating\n"; + } +#endif + return 0; +} + +// Code-quality regression: when producer bounds let lossless_cast() narrow the +// dot-product term, it should push the cast into the multiply and leave a native +// Int(16) multiply, not a cast of an Int(32) widening_mul. This is what the cache +// seeded into lossless_cast() buys; without it the retype still computes the right +// answer but emits the wider multiply, so a correctness test can't catch its loss. +int change_type_keeps_narrow_multiply_test() { + const int K = 3; + ImageParam A{Int(16), 1, "A"}, B{Int(16), 1, "B"}; + + Var i{"i"}, x{"x"}; + RDom r(0, K, "r"); + + Func pa{"pa"}, pb{"pb"}; + pa(x) = clamp(A(x), 0, 100); + pb(x) = clamp(B(x), 0, 100); + + Func Acc{"Acc"}; + Acc(i) = 0.0f; + Acc(i) += cast(pa(r)) * cast(pb(r)); + + Func Acc_i16 = Acc.change_type(Int(16)); + + // Inspect the retyped update expression directly (before lowering, so no later + // pass can reintroduce or erase the intrinsic). + std::ostringstream os; + os << Acc_i16.update_value(0); + const std::string retyped = os.str(); + internal_assert(retyped.find("widening_mul") == std::string::npos) + << "change_type() should push the narrowing cast into the multiply, leaving a " + << "native Int(16) multiply, but the retyped term kept a widening_mul:\n" + << retyped << "\n"; + return 0; +} + +// A product reduction grows the accumulator multiplicatively, which the overflow +// proof does not model, so change_type() rejects it (a product of K terms that +// each fit the target type can still be term^K, far past its range). Support may +// be added later; until then it must fail loudly rather than silently overflow. +int change_type_product_reduction_unsupported_test() { +#if HALIDE_WITH_EXCEPTIONS + if (!Halide::exceptions_enabled()) { + return 0; + } + ImageParam A{Int(8), 1, "A"}; + Var i{"i"}; + RDom r(0, 8, "r"); + + Func Acc{"Acc"}; + Acc(i) = 1.0f; + Acc(i) *= cast(A(r)); // product reduction + + bool threw = false; + try { + Acc.change_type(Int(32)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type() should reject a product reduction as unsupported\n"; +#endif + return 0; +} + +// A difference reduction subtracts each term, which grows the accumulator +// additively (by -term), so change_type() supports it with the same bound as a +// sum. Here -sum of K int8 values fits Int(16). +int change_type_difference_reduction_test() { + const int K = 100; + ImageParam A{Int(8), 1, "A"}; + + Var i{"i"}; + RDom r(0, K, "r"); + + Func Acc{"Acc"}; + Acc(i) = 0.0f; + Acc(i) -= cast(A(r)); // difference reduction: -sum of int8 + + Func Acc_i16 = Acc.change_type(Int(16)); + internal_assert(Acc_i16.types()[0] == Int(16)) + << "change_type difference: expected Int(16), got " << Acc_i16.types()[0] << "\n"; + Acc_i16.compute_root(); + + Buffer a(K); + for (int k = 0; k < K; k++) { + a(k) = (int8_t)((k * 31) % 255 - 127); // spans the full int8 range + } + A.set(a); + + Buffer result = Acc.realize({4}); + int32_t neg_sum = 0; + for (int k = 0; k < K; k++) { + neg_sum -= (int32_t)a(k); + } + for (int m = 0; m < 4; m++) { + internal_assert(result(m) == (float)neg_sum) + << "change_type difference mismatch at " << m << ": " << result(m) + << " vs " << neg_sum << "\n"; + } + return 0; +} + +// The widening fold is keyed on float-exact-representability, not a fixed table +// of 8/16-bit Float(32) patterns, so it also fires for int32 operands under a +// Float(64) accumulator. A dot product of two clamped int32 producers retyped to +// Int(64) should therefore come back as an integer widening_mul(i32, i32). +int change_type_widening_fold_generalizes_test() { + const int K = 4; + ImageParam A{Int(32), 1, "A"}, B{Int(32), 1, "B"}; + + Var i{"i"}, x{"x"}; + RDom r(0, K, "r"); + + Func pa{"pa"}, pb{"pb"}; + pa(x) = clamp(A(x), 0, 1000); + pb(x) = clamp(B(x), 0, 1000); + + Func Acc{"Acc"}; + Acc(i) = cast(0); + Acc(i) += cast(pa(r)) * cast(pb(r)); + + // Per-term product <= 1e6; over K = 4 terms the sum <= 4e6, well inside + // Int(64). The int32 operands round-trip through Float(64) exactly, so the + // fold applies even though the old table only covered 8/16-bit under f32. + Func Acc_i64 = Acc.change_type(Int(64)); + internal_assert(Acc_i64.types()[0] == Int(64)) + << "change_type widening-fold: expected Int(64), got " << Acc_i64.types()[0] << "\n"; + + std::ostringstream os; + os << Acc_i64.update_value(0); + const std::string retyped = os.str(); + internal_assert(retyped.find("widening_mul") != std::string::npos) + << "change_type() should expose an integer widening_mul for int32-under-f64, " + << "but the retyped term was:\n" + << retyped << "\n"; + + pa.compute_root(); + pb.compute_root(); + Acc_i64.compute_root(); + + Buffer a(K), b(K); + for (int k = 0; k < K; k++) { + a(k) = (k * 811) % 3000 - 1000; + b(k) = (k * 977) % 3000 - 1000; + } + A.set(a); + B.set(b); + + Buffer result = Acc.realize({4}); + int64_t dot = 0; + for (int k = 0; k < K; k++) { + int64_t ca = std::min(std::max(a(k), 0), 1000); + int64_t cb = std::min(std::max(b(k), 0), 1000); + dot += ca * cb; + } + for (int m = 0; m < 4; m++) { + internal_assert(result(m) == (double)dot) + << "change_type widening-fold mismatch at " << m << ": " << result(m) + << " vs " << dot << "\n"; + } + return 0; +} + +} // namespace + +int main(int argc, char **argv) { + printf("Running change_type_symbolic_extent_test\n"); + if (change_type_symbolic_extent_test()) { + return 1; + } + printf("Running change_type_twice_test\n"); + if (change_type_twice_test()) { + return 1; + } + printf("Running change_type_producer_bounds_test\n"); + if (change_type_producer_bounds_test()) { + return 1; + } + printf("Running change_type_bounded_dot_product_test\n"); + if (change_type_bounded_dot_product_test()) { + return 1; + } + printf("Running change_type_pure_narrowing_test\n"); + if (change_type_pure_narrowing_test()) { + return 1; + } + printf("Running change_type_narrowing_dot_product_test\n"); + if (change_type_narrowing_dot_product_test()) { + return 1; + } + printf("Running change_type_truncating_rejected_test\n"); + if (change_type_truncating_rejected_test()) { + return 1; + } + printf("Running change_type_keeps_narrow_multiply_test\n"); + if (change_type_keeps_narrow_multiply_test()) { + return 1; + } + printf("Running change_type_product_reduction_unsupported_test\n"); + if (change_type_product_reduction_unsupported_test()) { + return 1; + } + printf("Running change_type_difference_reduction_test\n"); + if (change_type_difference_reduction_test()) { + return 1; + } + printf("Running change_type_widening_fold_generalizes_test\n"); + if (change_type_widening_fold_generalizes_test()) { + return 1; + } + + printf("Success!\n"); + return 0; +} From 953d72e12c863854d16d90a77e2916a68dd4c0f0 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 30 Jul 2026 12:48:42 -0400 Subject: [PATCH 03/18] Fix change_type soundness checks --- src/Func.cpp | 280 +++++++++++++++++---- src/Func.h | 11 +- test/correctness/change_type.cpp | 411 +++++++++++++++++++++++++++++++ 3 files changed, 655 insertions(+), 47 deletions(-) diff --git a/src/Func.cpp b/src/Func.cpp index 800a5e91744c..378b07ea74cc 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -3322,6 +3323,14 @@ bool contains_self_reference(const Expr &e, const string &fname) { return finder.found; } +// Is `e` itself a direct call to `fname`? +bool is_self_reference(const Expr &e, const string &fname) { + if (const Call *c = e.as()) { + return c->call_type == Call::Halide && c->name == fname; + } + return false; +} + /** * Initialize a constant-bounds cache with the FuncValueBounds-derived range * of every Func call in \p e. The bounds machinery keys its cache by pointer @@ -3413,10 +3422,10 @@ Expr retype_leaf(const Expr &e, Type t, const FuncValueBounds &fvb) { return cast(t, folded); } -// Retype a whole definition value to type `t`, retargeting self-references from -// `fname` to `dst` and pushing casts down to the increment leaves. Only reduction -// updates shaped as a tree of binary combiners over a self-reference and an -// increment are supported. +// Retype a whole definition value to type `t`, retargeting a direct +// self-reference from `fname` to `dst` and retyping the other operand as the +// increment. The direct-call restriction keeps the recurrence visible to the +// overflow proof. Expr retype_value(const Expr &e, const string &fname, const Function &dst, Type t, const FuncValueBounds &fvb) { if (const Call *c = e.as()) { @@ -3426,9 +3435,19 @@ Expr retype_value(const Expr &e, const string &fname, const Function &dst, Type } if (contains_self_reference(e, fname)) { optional> operands = as_binary_operands(e); - user_assert(operands) - << "change_type() only supports update definitions built from binary " - << "operators over the accumulator; " << fname << " has an unsupported shape.\n"; + user_assert(operands) << "change_type() only supports update definitions " + "built from a binary operator with a direct call to " + << fname << " as one operand.\n"; + const bool a_is_self = is_self_reference(operands->first, fname); + const bool b_is_self = is_self_reference(operands->second, fname); + user_assert(a_is_self != b_is_self && + !contains_self_reference(a_is_self ? operands->second : operands->first, fname)) + << "change_type() requires the accumulator operand of every update to " + "be a single direct call to " + << fname << ".\n"; + user_assert(e.node_type() != IRNodeType::Sub || a_is_self) + << "change_type() only supports difference reductions of the form " + << fname << "(...) - term.\n"; return make_binary_op(e.node_type(), retype_value(operands->first, fname, dst, t, fvb), retype_value(operands->second, fname, dst, t, fvb)); @@ -3437,13 +3456,123 @@ Expr retype_value(const Expr &e, const string &fname, const Function &dst, Type } // The top-level associative combiner of a (let-stripped) reduction update value, -// i.e. the node type of the binary op whose operands are the self-reference and -// the increment. Returns nullopt if `val` isn't such a shape. +// i.e. the node type of the binary op whose operands are a direct self-reference +// and a self-reference-free increment. Returns nullopt if `val` isn't such a +// shape. optional reduction_op(const Expr &val, const string &fname) { - optional> split = select_binary_operand(val, val.node_type(), [&](const Expr &e) { - return contains_self_reference(e, fname); - }); - return split ? std::make_optional(val.node_type()) : std::nullopt; + optional> operands = as_binary_operands(val); + if (!operands) { + return std::nullopt; + } + const bool a_is_self = is_self_reference(operands->first, fname); + const bool b_is_self = is_self_reference(operands->second, fname); + if (a_is_self == b_is_self) { + return std::nullopt; + } + const Expr &increment = a_is_self ? operands->second : operands->first; + if (contains_self_reference(increment, fname) || + (val.node_type() == IRNodeType::Sub && !a_is_self)) { + return std::nullopt; + } + return val.node_type(); +} + +// Given a current accumulator interval, a per-step contribution interval, and +// the target type's limits, return the largest non-negative term count that is +// guaranteed not to overflow. Safety is monotonic in the term count, so use +// ConstantInterval's overflow-aware arithmetic in a binary search rather than +// duplicating its endpoint math here. +int64_t maximum_safe_term_count(const ConstantInterval &accumulator, + const ConstantInterval &step, + const ConstantInterval &limit) { + internal_assert(accumulator.is_bounded() && limit.is_bounded()); + internal_assert(limit.contains(accumulator)); + + uint64_t min_safe = 0; + uint64_t max_possible = std::numeric_limits::max(); + while (min_safe < max_possible) { + const uint64_t midpoint = + min_safe + (max_possible - min_safe + 1) / 2; + const ConstantInterval contribution = + step * ConstantInterval(0, (int64_t)midpoint); + if (limit.contains(accumulator + contribution)) { + min_safe = midpoint; + } else { + max_possible = midpoint - 1; + } + } + return (int64_t)min_safe; +} + +// Build an overflow-free runtime predicate that the product of all reduction +// extents is non-negative and at most `limit`. `product` is kept valid by a +// select whenever a factor would exceed the remaining budget, so no wrapping +// multiplication feeds a later comparison. +Expr reduction_cardinality_fits(const Definition &def, int64_t limit) { + Expr product = make_const(Int(64), 1); + Expr all_non_negative = const_true(); + Expr any_zero = const_false(); + Expr product_fits = const_true(); + const Expr max_terms = make_const(Int(64), limit); + for (const auto &rv : def.schedule().rvars()) { + Expr extent = cast(Int(64), rv.extent); + Expr positive_extent = max(extent, 1); + Expr factor_ok = product <= max_terms / positive_extent; + all_non_negative = all_non_negative && (extent >= 0); + any_zero = any_zero || (extent == 0); + product_fits = product_fits && factor_ok; + product = select(factor_ok, product * extent, max_terms); + } + return simplify(all_non_negative && (any_zero || product_fits)); +} + +// Prove that the first update executes at least once for every pure coordinate, +// as required when translating an identity that does not round-trip through the +// target type. Symbolic extents produce a runtime precondition. +std::optional nonempty_dense_update_precondition(const Function &fn, + Expr *condition) { + *condition = Expr(); + internal_assert(fn.has_update_definition()); + const Definition &def = fn.update(0); + + if (def.args().size() != fn.args().size()) { + return "the first update does not cover every pure coordinate"; + } + for (size_t i = 0; i < def.args().size(); i++) { + const Variable *arg = def.args()[i].as(); + if (!arg || arg->name != fn.args()[i] || + arg->param.defined() || arg->image.defined() || + arg->reduction_domain.defined()) { + return "the first update does not cover every pure coordinate"; + } + } + + if (!is_const_one(simplify(def.predicate()))) { + return "the first update is predicated"; + } + + Expr positive_extents = const_true(); + for (const auto &rv : def.schedule().rvars()) { + Expr extent = simplify(rv.extent); + if (optional ext = as_const_int(extent)) { + if (*ext <= 0) { + return "the first update has an empty reduction domain"; + } + } else if (optional ext = as_const_uint(extent)) { + if (*ext == 0) { + return "the first update has an empty reduction domain"; + } + } else { + positive_extents = + positive_extents && (cast(Int(64), extent) > 0); + } + } + + positive_extents = simplify(positive_extents); + if (!is_const_one(positive_extents)) { + *condition = positive_extents; + } + return std::nullopt; } // Prove that computing `typed`'s reduction at type `t` cannot overflow. Returns @@ -3455,7 +3584,15 @@ std::optional change_type_prove_safe( ) { *condition = Expr(); const Function fn = typed.function(); - const ConstantInterval limit = ConstantInterval::bounds_of_type(t); + ConstantInterval limit = ConstantInterval::bounds_of_type(t); + if (!limit.max_defined) { + // ConstantInterval cannot represent UInt(64)'s true upper bound. Use the + // largest representable conservative subset rather than treating an + // unbounded upper range as safe. + internal_assert(t.is_uint() && t.bits() == 64); + limit.max_defined = true; + limit.max = std::numeric_limits::max(); + } // Bound `e` using constant integer bounds, refined by the proven value ranges // of any producer Funcs it references (e.g. a clamp upstream). retype_leaf() @@ -3472,11 +3609,13 @@ std::optional change_type_prove_safe( return constant_integer_bounds(v, Scope::empty_scope(), &cache); }; - // Pure / identity values must be representable at the new type. - for (const Expr &v : fn.values()) { - if (!limit.contains(bounds_of(v))) { - return "the initial value may not be representable in the target type"; - } + // The initial accumulator value must be representable at the new type. Carry + // its interval through every update so each stage is checked against all + // preceding work rather than against an implicit zero. + internal_assert(fn.values().size() == 1); + ConstantInterval accumulator = bounds_of(fn.values()[0]); + if (!limit.contains(accumulator)) { + return "the initial value may not be representable in the target type"; } for (const Definition &def : fn.updates()) { @@ -3503,10 +3642,15 @@ std::optional change_type_prove_safe( return "a term may not be representable in the target type"; } - // min / max / and / or leave the accumulator within a single term's range, - // which we just proved fits, so they need nothing more. - if (op && (*op == IRNodeType::Min || *op == IRNodeType::Max || - *op == IRNodeType::And || *op == IRNodeType::Or)) { + // min and max leave the result within the union of the previous + // accumulator and a term. and/or are closed over the target type, but + // use its full interval for any later update. + if (op && (*op == IRNodeType::Min || *op == IRNodeType::Max)) { + accumulator = ConstantInterval::make_union(accumulator, term); + continue; + } + if (op && (*op == IRNodeType::And || *op == IRNodeType::Or)) { + accumulator = limit; continue; } @@ -3523,31 +3667,42 @@ std::optional change_type_prove_safe( // Each reduction step adds (Add) or subtracts (Sub) a term, so bound the // accumulator by (number of terms) x (per-step contribution). const ConstantInterval step = (*op == IRNodeType::Sub) ? -term : term; - int64_t n_max = 1; + ConstantInterval cardinality = ConstantInterval::single_point(1); bool symbolic = false; - Expr n_terms = make_const(Int(64), 1); for (const auto &rv : def.schedule().rvars()) { - n_terms = simplify(n_terms * cast(Int(64), rv.extent)); // Only a literal extent is known at compile time; a symbolic extent // (e.g. an ImageParam dimension) gets only type-based bounds, which we // must not treat as a static bound. if (optional ext = as_const_int(simplify(rv.extent)); ext && *ext >= 0) { - n_max *= *ext; + cardinality *= *ext; + if (!cardinality.is_single_point()) { + return "the reduction extent exceeds the range of Int(64)"; + } } else { symbolic = true; } } + if (!symbolic) { - if (!limit.contains(step * ConstantInterval(0, n_max))) { + ConstantInterval next = + accumulator + step * ConstantInterval(0, cardinality.max); + if (!limit.contains(next)) { return "the accumulated sum may exceed the target type's range"; } + accumulator = next; continue; } - // Symbolic term count: emit a runtime precondition instead. step's - // endpoints are defined because term's are (checked above). - Expr cond = (make_const(Int(64), step.max) * n_terms <= make_const(Int(64), limit.max)) && - (make_const(Int(64), step.min) * n_terms >= make_const(Int(64), limit.min)); + + // Symbolic term count: constrain the cardinality directly rather than + // multiplying it by a range endpoint, since either product could itself + // overflow while evaluating the guard. + const int64_t max_terms = + maximum_safe_term_count(accumulator, step, limit); + Expr cond = reduction_cardinality_fits(def, max_terms); *condition = condition->defined() ? (*condition && cond) : cond; + // Under the runtime condition the result fits, but without retaining a + // symbolic interval its tightest conservative range is the whole target. + accumulator = limit; } return std::nullopt; } @@ -3590,24 +3745,45 @@ Func Func::change_type(Type t, bool unsafe) { } const bool is_min_max = op && (*op == IRNodeType::Min || *op == IRNodeType::Max); + optional translated_identity; + Expr identity_precondition; + if (is_min_max) { + const Expr &initial = func.values()[0]; + const optional old_id = get_associative_identity(old_t, *op); + if (old_id && can_prove(initial == *old_id)) { + translated_identity = get_associative_identity(t, *op); + user_assert(translated_identity) + << "change_type() could not find an identity for " + << IRNodeType_string(*op) << " at type " << t << ".\n"; + + const Expr round_tripped = + simplify(cast(old_t, *translated_identity)); + if (!unsafe && !can_prove(round_tripped == *old_id)) { + const auto err = + nonempty_dense_update_precondition(func, &identity_precondition); + user_assert(!err) + << "change_type(" << t << ") on " << fname + << " cannot safely translate its " << IRNodeType_string(*op) + << " identity because " << *err << ".\n" + << "Pass unsafe=true to bypass this check.\n"; + } + } + } + // Build the retyped clone. Func typed(fname + "_typed"); // Pure definition. { - vector retyped; - for (const Expr &v : func.values()) { - if (is_min_max) { - optional id = get_associative_identity(t, *op); - user_assert(id) << "change_type() could not find an identity for " - << IRNodeType_string(*op) << " at type " << t << ".\n"; - retyped.push_back(*id); - } else { - retyped.push_back(retype_leaf(v, t, func_bounds)); - } + internal_assert(func.values().size() == 1); + Expr pure_value = func.values()[0]; + + Expr retyped = translated_identity.value_or(Expr()); + if (!retyped.defined()) { + retyped = retype_leaf(pure_value, t, func_bounds); } - // Single-output only (asserted above), so there is exactly one value. - typed(pure_vars) = retyped[0]; + + typed(pure_vars) = retyped; } // Update definitions. The retyped values still reference the original @@ -3621,10 +3797,24 @@ Func Func::change_type(Type t, bool unsafe) { for (const Expr &v : def.values()) { vals.push_back(retype_value(substitute_in_all_lets(v), fname, typed.function(), t, func_bounds)); } - typed.function().define_update(def.args(), vals, ReductionDomain{}); + typed.function().define_update(def.args(), vals); typed.function().update(u).schedule() = def.schedule().get_copy(); } + // Retyping an already-retyped Func must not discard the preconditions that + // made the earlier cast-back wrapper safe. + typed.function().schedule().type_change_checks() = + func.schedule().type_change_checks(); + + if (identity_precondition.defined()) { + std::ostringstream msg; + msg << "change_type(" << t << ") on " << fname + << " requires a non-empty reduction domain to translate its " + << IRNodeType_string(*op) << " identity"; + typed.function().schedule().type_change_checks().emplace_back( + identity_precondition, msg.str()); + } + // Safety check. if (!unsafe && t.is_int_or_uint()) { Expr condition; diff --git a/src/Func.h b/src/Func.h index aceed46fe59f..c4bb8421b7ce 100644 --- a/src/Func.h +++ b/src/Func.h @@ -2676,8 +2676,15 @@ class Func { * established, change_type() errors unless `unsafe` is true, which bypasses * the check entirely. * - * Currently supports single-output Funcs whose update definitions are built - * from binary operators over the accumulator. */ + * Translating a min/max identity that does not round-trip through `t` + * additionally requires the first update to be dense and unpredicated. Its + * reduction extents must be statically positive or satisfy an injected + * runtime precondition that they are positive. + * + * Currently supports single-output Funcs whose update definitions use a + * binary operator with one operand that is a direct call to the accumulator + * and one self-reference-free term. Difference reductions must have the + * accumulator as the left operand. */ Func change_type(Type t, bool unsafe = false); /** Immediately inline direct calls to each of the given Funcs into this diff --git a/test/correctness/change_type.cpp b/test/correctness/change_type.cpp index 25486ceedee7..aeee5527e8f6 100644 --- a/test/correctness/change_type.cpp +++ b/test/correctness/change_type.cpp @@ -539,6 +539,381 @@ int change_type_widening_fold_generalizes_test() { return 0; } +// The overflow proof must include the initial value as well as the reduction +// terms. Although ten increments of one fit in Int(8), starting from 120 makes +// the final value 130, which does not. +int change_type_initial_value_contributes_to_overflow_test() { +#if HALIDE_WITH_EXCEPTIONS + if (!Halide::exceptions_enabled()) { + return 0; + } + + ImageParam A{UInt(1), 1, "A_seed_overflow"}; + Var i{"i"}; + RDom r(0, 10, "r"); + + Func Acc{"Acc_seed_overflow"}; + Acc(i) = 120.0f; + Acc(i) += cast(A(r)); + + bool threw = false; + try { + Acc.change_type(Int(8)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type(Int(8)) should reject 120 + ten increments of one: " + << "the initial value makes the accumulator overflow\n"; +#endif + return 0; +} + +// The accumulator range must flow from one update stage into the next. Each +// stage adds only 100, which fits Int(8) in isolation, but together they add 200. +int change_type_multiple_updates_accumulate_overflow_test() { +#if HALIDE_WITH_EXCEPTIONS + if (!Halide::exceptions_enabled()) { + return 0; + } + + ImageParam A{UInt(1), 1, "A_multi_update_1"}; + ImageParam B{UInt(1), 1, "B_multi_update_2"}; + Var i{"i"}; + RDom r1(0, 100, "r1"), r2(0, 100, "r2"); + + Func Acc{"Acc_multi_update_overflow"}; + Acc(i) = 0.0f; + Acc(i) += cast(A(r1)); + Acc(i) += cast(B(r2)); + + bool threw = false; + try { + Acc.change_type(Int(8)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type(Int(8)) should reject two update stages that cumulatively " + << "add 200, even though each stage adds only 100\n"; +#endif + return 0; +} + +// Looking only at the top-level '+' is not enough to classify an update as a +// sum reduction: the self-containing branch can apply another recurrence. This +// update grows as 2*x + 1 and reaches 255 after eight iterations. +int change_type_nested_accumulator_recurrence_rejected_test() { +#if HALIDE_WITH_EXCEPTIONS + if (!Halide::exceptions_enabled()) { + return 0; + } + + ImageParam A{UInt(1), 1, "A_nested_recurrence"}; + Var i{"i"}; + RDom r(0, 8, "r"); + + Func Acc{"Acc_nested_recurrence"}; + Acc(i) = 0.0f; + Acc(i) = Acc(i) * 2.0f + cast(A(r)); + + bool threw = false; + try { + Acc.change_type(Int(8)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type(Int(8)) should reject a nested 2*x + 1 recurrence instead " + << "of treating it as a sum of eight ones\n"; +#endif + return 0; +} + +// A min/max seed is not necessarily the operator identity. Retyping must +// preserve a finite seed rather than unconditionally replacing it with the +// target type's identity. +int change_type_min_preserves_non_identity_seed_test() { + const int K = 4; + ImageParam A{Int(8), 1, "A_min_seed"}; + + Var i{"i"}; + RDom r(0, K, "r"); + + Func Min{"Min_non_identity_seed"}; + Min(i) = 5.0f; + Min(i) = min(Min(i), cast(A(r))); + + Func Min_i8 = Min.change_type(Int(8)); + Min_i8.compute_root(); + + Buffer a(K); + for (int k = 0; k < K; k++) { + a(k) = (int8_t)(10 + k); + } + A.set(a); + + Buffer result = Min.realize({1}); + internal_assert(result(0) == 5.0f) + << "change_type() changed a min reduction's seed from 5 to the Int(8) " + << "identity; result was " << result(0) << " instead of 5\n"; + return 0; +} + +// The non-identity case above must not regress the intended special handling +// for a true floating-point min identity, which cannot be cast directly to an +// integer target without losing its identity semantics. +int change_type_min_translates_identity_seed_test() { + const int K = 4; + ImageParam A{Int(8), 1, "A_min_identity"}; + + Var i{"i"}; + RDom r(0, K, "r"); + + Func Min{"Min_identity_seed"}; + Min(i) = Float(32).max(); + Min(i) = min(Min(i), cast(A(r))); + + Func Min_i8 = Min.change_type(Int(8)); + Min_i8.compute_root(); + + Buffer a(K); + for (int k = 0; k < K; k++) { + a(k) = (int8_t)(10 + k); + } + A.set(a); + + Buffer result = Min.realize({1}); + internal_assert(result(0) == 10.0f) + << "change_type() did not translate the Float(32) min identity to the " + << "Int(8) identity; result was " << result(0) << " instead of 10\n"; + return 0; +} + +// Translating an identity that does not round-trip through the target type is +// only sound when the first update is guaranteed to replace it at every pure +// coordinate. Constant empty domains are rejected, symbolic domains get a +// runtime non-empty check, and scatter or predicated updates are rejected. +int change_type_identity_translation_requires_dense_nonempty_update_test() { +#if HALIDE_WITH_EXCEPTIONS + if (!Halide::exceptions_enabled()) { + return 0; + } + + // A statically empty dense reduction would expose the translated Int(8) + // identity (127) instead of the original Float(32) identity (+infinity). + { + Var i{"i"}; + RDom r(0, 0, "r_static_empty"); + Func Min{"Min_static_empty"}; + Min(i) = Float(32).max(); + Min(i) = min(Min(i), cast(r % 2)); + + bool threw = false; + try { + Min.change_type(Int(8)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type() should reject identity translation for a statically " + << "empty reduction domain\n"; + } + + // A symbolic dense reduction is allowed, but zero must fail its generated + // runtime precondition while a positive extent still computes normally. + { + Param extent{"identity_extent"}; + Var i{"i"}; + RDom r(0, extent, "r_symbolic_empty"); + Func Min{"Min_symbolic_empty"}; + Min(i) = Float(32).max(); + Min(i) = min(Min(i), cast(r % 2)); + + Func Min_i8 = Min.change_type(Int(8)); + Min_i8.compute_root(); + + extent.set(0); + bool threw = false; + try { + (void)Min.realize({1}); + } catch (const Halide::RuntimeError &) { + threw = true; + } + internal_assert(threw) + << "change_type() should require a symbolic reduction domain to be non-empty " + << "when translating an identity\n"; + + extent.set(1); + Buffer result = Min.realize({1}); + internal_assert(result(0) == 0.0f); + } + + // A non-empty scatter domain does not update every pure coordinate. + { + Var x{"x"}; + RDom r(0, 4, "r_scatter"); + Func Min{"Min_scatter_identity"}; + Min(x) = Float(32).max(); + Min(r) = min(Min(r), cast(r % 2)); + + bool threw = false; + try { + Min.change_type(Int(8)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type() should reject identity translation for a scatter update\n"; + } + + // A predicate can filter out every reduction point for an output. + { + Var i{"i"}; + RDom r(0, 4, "r_predicated"); + r.where(r < 2); + Func Min{"Min_predicated_identity"}; + Min(i) = Float(32).max(); + Min(i) = min(Min(i), cast(r % 2)); + + bool threw = false; + try { + Min.change_type(Int(8)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type() should reject identity translation for a predicated update\n"; + } +#endif + return 0; +} + +// A runtime guard installed by an earlier change_type() must survive a later +// retype. The first step requires the symbolic reduction to fit Int(16); the +// second step widens the actual accumulator to Int(32), but its cast-back wrapper +// still narrows the result through Int(16). +int change_type_chaining_preserves_runtime_checks_test() { +#if HALIDE_WITH_EXCEPTIONS + if (!Halide::exceptions_enabled()) { + return 0; + } + + Param extent{"chain_extent"}; + ImageParam A{Int(8), 1, "A_chain_checks"}; + + Var i{"i"}; + RDom r(0, extent, "r"); + + Func Acc{"Acc_chain_checks"}; + Acc(i) = 0.0f; + Acc(i) += cast(A(r)); + + Func Acc_i16 = Acc.change_type(Int(16)); + Func Acc_i32 = Acc_i16.change_type(Int(32)); + Acc_i32.compute_root(); + + const int K = 300; + Buffer a(K); + a.fill(127); + A.set(a); + extent.set(K); + + bool threw = false; + try { + (void)Acc.realize({1}); + } catch (const Halide::RuntimeError &) { + threw = true; + } + internal_assert(threw) + << "the Int(16) overflow guard was lost after chaining change_type(Int(32)); " + << "a sum of 300 * 127 should have failed at runtime\n"; +#endif + return 0; +} + +// The runtime guard itself must not use wrapping arithmetic. For a full-range +// Int(64) term, multiplying either endpoint by a symbolic extent of two wraps, +// making both comparisons spuriously true even though the accumulation can +// overflow. +int change_type_runtime_check_arithmetic_does_not_overflow_test() { +#if HALIDE_WITH_EXCEPTIONS + if (!Halide::exceptions_enabled()) { + return 0; + } + + Param extent{"guard_extent"}; + ImageParam A{Int(64), 1, "A_guard_overflow"}; + + Var i{"i"}; + RDom r(0, extent, "r"); + + Func Acc{"Acc_guard_overflow"}; + Acc(i) = cast(0); + Acc(i) += cast(A(r)); + + Func Acc_i64 = Acc.change_type(Int(64)); + Acc_i64.compute_root(); + + Buffer a(2); + a.fill(int64_t{1} << 62); + A.set(a); + + // One full-range term is permitted and must not be rejected by an overly + // conservative or malformed guard. + extent.set(1); + Buffer safe_result = Acc.realize({1}); + internal_assert(safe_result(0) == (double)(int64_t{1} << 62)); + + // Two such terms may overflow Int(64), so the guard must reject the extent + // before the reduction executes. + extent.set(2); + + bool threw = false; + try { + (void)Acc.realize({1}); + } catch (const Halide::RuntimeError &) { + threw = true; + } + internal_assert(threw) + << "the change_type(Int(64)) guard overflowed while checking a symbolic " + << "extent of two and failed to reject an overflowing accumulation\n"; +#endif + return 0; +} + +// The compile-time term count must also use checked arithmetic. Three extents +// of 2^22 have a product of 2^66; the unchecked signed multiplication is +// undefined and, in this case, makes an enormous reduction look harmless. +int change_type_static_extent_count_does_not_overflow_test() { +#if HALIDE_WITH_EXCEPTIONS + if (!Halide::exceptions_enabled()) { + return 0; + } + + constexpr int extent = 1 << 22; + RDom r({{0, extent}, {0, extent}, {0, extent}}, "r"); + + Func Acc{"Acc_static_extent_overflow"}; + Acc() = 0.0f; + // Keep all three RVars live without requiring an impossibly large buffer. + // Each loop extent is legal on its own, and a scalar reduction has no + // allocation proportional to the product of its reduction extents. + Acc() += cast((r.x == 0) && (r.y == 0) && (r.z == 0)); + + bool threw = false; + try { + Acc.change_type(Int(8)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type(Int(8)) should reject a 2^66-term reduction; its static " + << "term-count calculation overflowed and made the reduction appear safe\n"; +#endif + return 0; +} + } // namespace int main(int argc, char **argv) { @@ -586,6 +961,42 @@ int main(int argc, char **argv) { if (change_type_widening_fold_generalizes_test()) { return 1; } + printf("Running change_type_initial_value_contributes_to_overflow_test\n"); + if (change_type_initial_value_contributes_to_overflow_test()) { + return 1; + } + printf("Running change_type_multiple_updates_accumulate_overflow_test\n"); + if (change_type_multiple_updates_accumulate_overflow_test()) { + return 1; + } + printf("Running change_type_nested_accumulator_recurrence_rejected_test\n"); + if (change_type_nested_accumulator_recurrence_rejected_test()) { + return 1; + } + printf("Running change_type_min_preserves_non_identity_seed_test\n"); + if (change_type_min_preserves_non_identity_seed_test()) { + return 1; + } + printf("Running change_type_min_translates_identity_seed_test\n"); + if (change_type_min_translates_identity_seed_test()) { + return 1; + } + printf("Running change_type_identity_translation_requires_dense_nonempty_update_test\n"); + if (change_type_identity_translation_requires_dense_nonempty_update_test()) { + return 1; + } + printf("Running change_type_chaining_preserves_runtime_checks_test\n"); + if (change_type_chaining_preserves_runtime_checks_test()) { + return 1; + } + printf("Running change_type_runtime_check_arithmetic_does_not_overflow_test\n"); + if (change_type_runtime_check_arithmetic_does_not_overflow_test()) { + return 1; + } + printf("Running change_type_static_extent_count_does_not_overflow_test\n"); + if (change_type_static_extent_count_does_not_overflow_test()) { + return 1; + } printf("Success!\n"); return 0; From f9a24ff0e690f8835fffe606b5d1aeac63d9d1ef Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 30 Jul 2026 12:57:16 -0400 Subject: [PATCH 04/18] Refactor get_associative_identity --- src/AssociativeOpsTable.cpp | 6 +++--- src/AssociativeOpsTable.h | 2 +- src/Func.cpp | 27 +++++++++------------------ 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/src/AssociativeOpsTable.cpp b/src/AssociativeOpsTable.cpp index 2f94f018c862..0ec6a7399001 100644 --- a/src/AssociativeOpsTable.cpp +++ b/src/AssociativeOpsTable.cpp @@ -372,19 +372,19 @@ const vector &get_ops_table(const vector &exprs) { return table; } -std::optional get_associative_identity(Type type, IRNodeType root) { +Expr get_associative_identity(Type type, IRNodeType root) { std::scoped_lock lock_guard(ops_table_lock()); const vector &table = get_ops_table_helper({type}, root, 1); if (table.empty()) { - return std::nullopt; + return Expr(); } const Expr &identity = table.front().identities.front(); for (const AssociativePattern &pattern : table) { internal_assert(pattern.size() == 1); if (!equal(pattern.identities.front(), identity)) { - return std::nullopt; + return Expr(); } } return identity; diff --git a/src/AssociativeOpsTable.h b/src/AssociativeOpsTable.h index ab3568c951d3..4e1557662fbd 100644 --- a/src/AssociativeOpsTable.h +++ b/src/AssociativeOpsTable.h @@ -74,7 +74,7 @@ const std::vector &get_ops_table(const std::vector &ex /** Return the identity for a single-output associative op, if the table has one * and all matching patterns agree on it. */ -std::optional get_associative_identity(Type type, IRNodeType root); +Expr get_associative_identity(Type type, IRNodeType root); } // namespace Internal } // namespace Halide diff --git a/src/Func.cpp b/src/Func.cpp index 378b07ea74cc..bd0c9f649e7d 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -3745,20 +3745,19 @@ Func Func::change_type(Type t, bool unsafe) { } const bool is_min_max = op && (*op == IRNodeType::Min || *op == IRNodeType::Max); - optional translated_identity; + Expr translated_identity; Expr identity_precondition; if (is_min_max) { const Expr &initial = func.values()[0]; - const optional old_id = get_associative_identity(old_t, *op); - if (old_id && can_prove(initial == *old_id)) { + const Expr old_id = get_associative_identity(old_t, *op); + if (old_id.defined() && can_prove(initial == old_id)) { translated_identity = get_associative_identity(t, *op); - user_assert(translated_identity) + user_assert(translated_identity.defined()) << "change_type() could not find an identity for " << IRNodeType_string(*op) << " at type " << t << ".\n"; - const Expr round_tripped = - simplify(cast(old_t, *translated_identity)); - if (!unsafe && !can_prove(round_tripped == *old_id)) { + const Expr round_tripped = cast(old_t, translated_identity); + if (!unsafe && !can_prove(round_tripped == old_id)) { const auto err = nonempty_dense_update_precondition(func, &identity_precondition); user_assert(!err) @@ -3774,17 +3773,9 @@ Func Func::change_type(Type t, bool unsafe) { Func typed(fname + "_typed"); // Pure definition. - { - internal_assert(func.values().size() == 1); - Expr pure_value = func.values()[0]; - - Expr retyped = translated_identity.value_or(Expr()); - if (!retyped.defined()) { - retyped = retype_leaf(pure_value, t, func_bounds); - } - - typed(pure_vars) = retyped; - } + typed(pure_vars) = translated_identity.defined() ? + translated_identity : + retype_leaf(value(), t, func_bounds); // Update definitions. The retyped values still reference the original // reduction domain, so pass a default domain and let define_update discover From 02ff163a2a50e33c79ed02874c7dc57177ffa20f Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 30 Jul 2026 14:03:21 -0400 Subject: [PATCH 05/18] Appease clang-tidy --- src/IROperator.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/IROperator.cpp b/src/IROperator.cpp index 2cdf8a6f0ff7..3a6e18b28a4c 100644 --- a/src/IROperator.cpp +++ b/src/IROperator.cpp @@ -252,10 +252,10 @@ std::optional> as_binary_operands(const Expr &e) { // rather than going through Expr::as<>(), which would redundantly re-check // the node type the switch case has already established. switch (e.node_type()) { -#define HANDLE_BINARY_OP(NodeType) \ - case IRNodeType::NodeType: { \ - const NodeType *op = static_cast(e.get()); \ - return std::pair{op->a, op->b}; \ +#define HANDLE_BINARY_OP(NodeType) \ + case IRNodeType::NodeType: { \ + const NodeType *op = (const NodeType *)(e.get()); \ + return std::pair{op->a, op->b}; \ } HANDLE_BINARY_OP(Add) HANDLE_BINARY_OP(Sub) From 054bddab6e067bf580375006a050f11b89a16113 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 30 Jul 2026 15:39:40 -0400 Subject: [PATCH 06/18] Add narrow-blocks test --- test/correctness/change_type.cpp | 44 ++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/correctness/change_type.cpp b/test/correctness/change_type.cpp index aeee5527e8f6..ebe4dc0aefd0 100644 --- a/test/correctness/change_type.cpp +++ b/test/correctness/change_type.cpp @@ -60,6 +60,46 @@ int change_type_symbolic_extent_test() { return 0; } +// A reduction with a runtime extent can be split into fixed-size blocks and +// factored so that only the bounded partial reductions use a narrow accumulator. +// The final reduction retains the original wider type and combines an unknown +// number of partial results. +int change_type_rfactor_symbolic_extent_blocks_test() { + constexpr int block_size = 128; + ImageParam A{Int(8), 1, "A_rfactor_blocks"}; + + RDom r(0, A.dim(0).extent(), "r"); + Func Acc{"Acc_rfactor_blocks"}; + Acc() = 0.0f; + Acc() += cast(A(r)); + + RVar ro{"ro"}, ri{"ri"}; + Var block{"block"}; + Func partial = Acc.update(0) + .split(r, ro, ri, block_size, TailStrategy::GuardWithIf) + .rfactor(ro, block); + + // Every partial reduction contains at most 128 Int(8) terms, which + // fits Int(16). The final reduction over `block` remains Float(32), + // since its runtime extent is unbounded. + Func partial_i16 = partial.change_type(Int(16)); + internal_assert(partial_i16.types()[0] == Int(16)); + partial_i16.compute_root(); + + // Make the total exceed Int(16) while each partial remains safe. + constexpr int K = 1000; + Buffer a(K); + a.fill(127); + A.set(a); + + Buffer result = Acc.realize(); + const int32_t expected = K * 127; + internal_assert(result() == (float)expected) + << "change_type after rfactor produced " << result() + << " instead of " << expected << "\n"; + return 0; +} + // change_type() can be applied more than once, retyping the intermediate // returned by a previous change_type(). Each step must remain safe and correct. int change_type_twice_test() { @@ -921,6 +961,10 @@ int main(int argc, char **argv) { if (change_type_symbolic_extent_test()) { return 1; } + printf("Running change_type_rfactor_symbolic_extent_blocks_test\n"); + if (change_type_rfactor_symbolic_extent_blocks_test()) { + return 1; + } printf("Running change_type_twice_test\n"); if (change_type_twice_test()) { return 1; From 437e957b137b869ef3c31f03deade475ce22a91d Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 30 Jul 2026 15:42:06 -0400 Subject: [PATCH 07/18] Drop stale TODO --- test/correctness/change_type.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/correctness/change_type.cpp b/test/correctness/change_type.cpp index ebe4dc0aefd0..5f5ab10bd56c 100644 --- a/test/correctness/change_type.cpp +++ b/test/correctness/change_type.cpp @@ -9,12 +9,6 @@ using namespace Halide; namespace { -// TODO: add tests that compose change_type() with hoist_invariants() -- e.g. -// retyping the float dot-product intermediate hoist_invariants() returns to an -// Int(32) accumulator, and confirming a min-reduction retype uses the -// reduction identity at the new type rather than a lossy cast of the original -// float identity. - // A symbolic reduction extent can't be bounded at schedule time, so change_type // injects a runtime precondition. With a valid (small) extent it passes and the // result is correct. From fc5af09928271ae98dfcf280d437e07d3b770a1c Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Fri, 31 Jul 2026 12:07:54 -0400 Subject: [PATCH 08/18] Serialize type-change checks --- src/Deserialization.cpp | 10 ++++++++++ src/Serialization.cpp | 14 +++++++++++++- src/halide_ir.fbs | 6 ++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/Deserialization.cpp b/src/Deserialization.cpp index 0257a59b6480..f7f8566326db 100644 --- a/src/Deserialization.cpp +++ b/src/Deserialization.cpp @@ -1035,6 +1035,15 @@ FuncSchedule Deserializer::deserialize_func_schedule(const Serialize::FuncSchedu const auto async = func_schedule->async(); const auto ring_buffer = deserialize_expr(func_schedule->ring_buffer_type(), func_schedule->ring_buffer()); const auto memoize_eviction_key = deserialize_expr(func_schedule->memoize_eviction_key_type(), func_schedule->memoize_eviction_key()); + std::vector> type_change_checks; + if (func_schedule->type_change_checks() != nullptr) { + type_change_checks.reserve(func_schedule->type_change_checks()->size()); + for (const auto *check : *func_schedule->type_change_checks()) { + type_change_checks.emplace_back( + deserialize_expr(check->condition_type(), check->condition()), + deserialize_string(check->message())); + } + } auto hl_func_schedule = FuncSchedule(); hl_func_schedule.store_level() = store_level; hl_func_schedule.compute_level() = compute_level; @@ -1048,6 +1057,7 @@ FuncSchedule Deserializer::deserialize_func_schedule(const Serialize::FuncSchedu hl_func_schedule.async() = async; hl_func_schedule.ring_buffer() = ring_buffer; hl_func_schedule.memoize_eviction_key() = memoize_eviction_key; + hl_func_schedule.type_change_checks() = std::move(type_change_checks); return hl_func_schedule; } diff --git a/src/Serialization.cpp b/src/Serialization.cpp index 91841437a1ac..2dd7bf4f33aa 100644 --- a/src/Serialization.cpp +++ b/src/Serialization.cpp @@ -1139,6 +1139,17 @@ Offset Serializer::serialize_func_schedule(FlatBufferBu const auto async = func_schedule.async(); const auto ring_buffer = serialize_expr(builder, func_schedule.ring_buffer()); const auto memoize_eviction_key_serialized = serialize_expr(builder, func_schedule.memoize_eviction_key()); + std::vector> type_change_checks_serialized; + type_change_checks_serialized.reserve(func_schedule.type_change_checks().size()); + for (const auto &[condition, message] : func_schedule.type_change_checks()) { + const auto condition_serialized = serialize_expr(builder, condition); + const auto message_serialized = serialize_string(builder, message); + type_change_checks_serialized.push_back( + Serialize::CreateTypeChangeCheck(builder, + condition_serialized.first, + condition_serialized.second, + message_serialized)); + } return Serialize::CreateFuncSchedule(builder, store_level_serialized, compute_level_serialized, hoist_storage_level_serialized, builder.CreateVector(storage_dims_serialized), @@ -1147,7 +1158,8 @@ Offset Serializer::serialize_func_schedule(FlatBufferBu builder.CreateVector(wrappers_serialized), memory_type, memoized, async, ring_buffer.first, ring_buffer.second, - memoize_eviction_key_serialized.first, memoize_eviction_key_serialized.second); + memoize_eviction_key_serialized.first, memoize_eviction_key_serialized.second, + builder.CreateVector(type_change_checks_serialized)); } Offset Serializer::serialize_specialization(FlatBufferBuilder &builder, const Specialization &specialization) { diff --git a/src/halide_ir.fbs b/src/halide_ir.fbs index 4d26c45f904c..4bba4bb79a8f 100644 --- a/src/halide_ir.fbs +++ b/src/halide_ir.fbs @@ -515,6 +515,11 @@ table WrapperRef { func_index: int32; } +table TypeChangeCheck { + condition: Expr; + message: string; +} + table FuncSchedule { store_level: LoopLevel; compute_level: LoopLevel; @@ -528,6 +533,7 @@ table FuncSchedule { async: bool; ring_buffer: Expr; memoize_eviction_key: Expr; + type_change_checks: [TypeChangeCheck]; } table Specialization { From 10a47a00782fdc6375ca3d18c64209ce8e7b5b6c Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Tue, 4 Aug 2026 17:27:41 -0400 Subject: [PATCH 09/18] Address review: check float change_type() targets for exactness change_type_prove_safe() was only invoked for integer targets, so retyping a reduction to a float type got no safety check at all. Run it for float targets too, bounding the accumulation against the largest integer the target can represent exactly (e.g. 2048 for float16) rather than its full dynamic range. Also fixes bounds_of() to recover the exact integer value of a leaf that retype_leaf() constant-folded directly into a float literal (e.g. a seed of 0), which it previously treated as unbounded. Adds test coverage for float targets, and for sum-then-clamp, sum-scan, and histogram reductions confirming the existing extent-based bound is conservative for those shapes too. --- src/Func.cpp | 53 +++++- src/Func.h | 10 +- test/correctness/change_type.cpp | 285 +++++++++++++++++++++++++++++++ 3 files changed, 343 insertions(+), 5 deletions(-) diff --git a/src/Func.cpp b/src/Func.cpp index bd0c9f649e7d..403ace9faf6b 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -3575,6 +3576,36 @@ std::optional nonempty_dense_update_precondition(const Function &fn return std::nullopt; } +// The largest (and, by symmetry, most negative) integer exactly representable +// in floating-point type `t`: 2^(significant bits), where significant bits +// counts the mantissa plus its implicit leading one. Bounding a reduction by +// this range instead of `t`'s full dynamic range ensures an integer +// accumulation retyped to `t` is exact, not merely finite. +ConstantInterval float_exact_integer_bounds(Type t) { + int significant_bits; + if (t.is_bfloat()) { + internal_assert(t.bits() == 16) << "Unhandled bfloat width in change_type()\n"; + significant_bits = 8; // 7 explicit mantissa bits + 1 implicit + } else { + switch (t.bits()) { + case 16: + significant_bits = 11; + break; + case 32: + significant_bits = 24; + break; + case 64: + significant_bits = 53; + break; + default: + internal_error << "Unhandled float width in change_type()\n"; + significant_bits = 0; + } + } + const int64_t bound = (int64_t)1 << significant_bits; + return ConstantInterval(-bound, bound); +} + // Prove that computing `typed`'s reduction at type `t` cannot overflow. Returns // true if it is safe; if safety can only be guaranteed under a runtime // precondition, that condition is returned in *condition. Returns an error @@ -3584,7 +3615,13 @@ std::optional change_type_prove_safe( ) { *condition = Expr(); const Function fn = typed.function(); - ConstantInterval limit = ConstantInterval::bounds_of_type(t); + // A float target's bounds machinery limit is the largest exactly + // representable integer, not its full dynamic range: an accumulation that + // overflows that range would silently round instead of erroring, so it + // must be caught here just like integer overflow is. + ConstantInterval limit = t.is_float() ? + float_exact_integer_bounds(t) : + ConstantInterval::bounds_of_type(t); if (!limit.max_defined) { // ConstantInterval cannot represent UInt(64)'s true upper bound. Use the // largest representable conservative subset rather than treating an @@ -3605,6 +3642,18 @@ std::optional change_type_prove_safe( if (const Cast *c = v.as()) { v = c->value; } + // retype_leaf() may have folded a leaf directly into a float constant + // (e.g. a literal seed retyped to a float target) rather than leaving + // a Cast to strip. constant_integer_bounds() only reasons about + // integer-typed expressions, so recover such a leaf's exact integer + // value here; a non-integer float constant falls through to the + // type-based (unbounded) fallback below, same as before this check + // was added. + if (v.type().is_float()) { + if (optional fv = as_const_float(v); fv && std::floor(*fv) == *fv) { + return ConstantInterval::single_point((int64_t)*fv); + } + } auto cache = cache_call_bounds(v, fvb); return constant_integer_bounds(v, Scope::empty_scope(), &cache); }; @@ -3807,7 +3856,7 @@ Func Func::change_type(Type t, bool unsafe) { } // Safety check. - if (!unsafe && t.is_int_or_uint()) { + if (!unsafe && (t.is_int_or_uint() || t.is_float())) { Expr condition; const auto err = change_type_prove_safe(typed, t, func_bounds, &condition); user_assert(!err) diff --git a/src/Func.h b/src/Func.h index c4bb8421b7ce..4df562e272ca 100644 --- a/src/Func.h +++ b/src/Func.h @@ -2667,9 +2667,13 @@ class Func { * instructions). Schedule the returned Func to control the retyped * computation. * - * The change is validated with the bounds machinery: for an integer target, - * change_type() proves the accumulation cannot overflow by combining the - * per-term value range with the reduction extent. If it can only be + * The change is validated with the bounds machinery: for an integer or + * floating-point target, change_type() proves the accumulation cannot + * overflow (or, for a float target, lose precision) by combining the + * per-term value range with the reduction extent. A float target is + * checked against the largest integer it can represent exactly (e.g. 2048 + * for float16), not its full dynamic range, so an integer accumulation + * retyped to it stays exact rather than merely finite. If it can only be * guaranteed under a runtime precondition (e.g. the RDom extent isn't too * wide), that precondition is injected into the pipeline's assertion block * (and removed by the no_asserts target feature). If safety cannot be diff --git a/test/correctness/change_type.cpp b/test/correctness/change_type.cpp index 5f5ab10bd56c..8f74039952cd 100644 --- a/test/correctness/change_type.cpp +++ b/test/correctness/change_type.cpp @@ -948,6 +948,271 @@ int change_type_static_extent_count_does_not_overflow_test() { return 0; } +// A float target is checked against the largest integer it can represent +// exactly (2048 for float16), not its much larger dynamic range, so an +// integer-valued accumulation retyped to it stays exact. +int change_type_float_target_precision_test() { + const int K = 10; + ImageParam A{Int(8), 1, "A_f16_safe"}; + + Var i{"i"}; + RDom r(0, K, "r"); + + Func Acc{"Acc_f16_safe"}; + Acc(i) = cast(0); + Acc(i) += cast(A(r)); + + // K terms of magnitude at most 4 sum to at most 40, well within float16's + // exactly-representable range of [-2048, 2048]. + Func Acc_f16 = Acc.change_type(Float(16)); + internal_assert(Acc_f16.types()[0] == Float(16)) + << "change_type float target: expected Float(16), got " << Acc_f16.types()[0] << "\n"; + Acc_f16.compute_root(); + + Buffer a(K); + for (int k = 0; k < K; k++) { + a(k) = (int8_t)((k % 9) - 4); + } + A.set(a); + + Buffer result = Acc.realize({4}); + double sum = 0; + for (int k = 0; k < K; k++) { + sum += a(k); + } + for (int m = 0; m < 4; m++) { + internal_assert(result(m) == sum) + << "change_type float target mismatch at " << m << ": " << result(m) + << " vs " << sum << "\n"; + } + return 0; +} + +// A sum whose magnitude can exceed float16's exactly-representable range must +// be rejected just like an integer overflow would be: past that range, +// retyping the accumulator to Float(16) would silently round instead of +// producing the exact integer result. +int change_type_float_target_precision_rejected_test() { +#if HALIDE_WITH_EXCEPTIONS + if (!Halide::exceptions_enabled()) { + return 0; + } + + const int K = 100; + + { + ImageParam A{Int(8), 1, "A_f16_unsafe"}; + Var i{"i"}; + RDom r(0, K, "r"); + + Func Acc{"Acc_f16_unsafe"}; + Acc(i) = cast(0); + Acc(i) += cast(A(r)); // magnitude up to 100 * 127 = 12700 + + bool threw = false; + try { + Acc.change_type(Float(16)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type(Float(16)) should reject a sum whose magnitude can reach " + << "12700, well past float16's exactly-representable range of 2048\n"; + } + + // With unsafe = true the caller takes responsibility and it is allowed. + { + ImageParam A{Int(8), 1, "A_f16_bypass"}; + Var i{"i"}; + RDom r(0, K, "r"); + + Func Acc{"Acc_f16_bypass"}; + Acc(i) = cast(0); + Acc(i) += cast(A(r)); + + Func Acc_f16 = Acc.change_type(Float(16), /*unsafe*/ true); + internal_assert(Acc_f16.types()[0] == Float(16)) + << "unsafe change_type should proceed despite possible precision loss\n"; + } +#endif + return 0; +} + +// A second update stage that clamps the accumulator with a big minimum does +// not relax the safety check on the summation before it: the accumulator is +// physically stored as `t` between stages, so a sum that can overflow `t` is +// unsafe even though the later min brings the final result back into range. +int change_type_sum_then_clamp_test() { +#if HALIDE_WITH_EXCEPTIONS + if (!Halide::exceptions_enabled()) { + return 0; + } + + const int K = 100; + + { + ImageParam A{Int(8), 1, "A_sum_then_clamp_narrow"}; + Var i{"i"}; + RDom r(0, K, "r"); + + Func Acc{"Acc_sum_then_clamp_narrow"}; + Acc(i) = 0.0f; + Acc(i) += cast(A(r)); // magnitude up to 100 * 127 = 12700 + Acc(i) = min(Acc(i), 10000.0f); // clamps the final result, not the running sum + + bool threw = false; + try { + Acc.change_type(Int(8)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type(Int(8)) should reject a sum that overflows Int(8) before " + << "the clamp ever runs\n"; + } + + // Int(16) comfortably holds the intermediate sum, so the clamp is just an + // ordinary min reduction stacked on top of it and this must succeed. + { + ImageParam A{Int(8), 1, "A_sum_then_clamp_wide"}; + Var i{"i"}; + RDom r(0, K, "r"); + + Func Acc{"Acc_sum_then_clamp_wide"}; + Acc(i) = 0.0f; + Acc(i) += cast(A(r)); + Acc(i) = min(Acc(i), 10000.0f); + + Func Acc_i16 = Acc.change_type(Int(16)); + Acc_i16.compute_root(); + + Buffer a(K); + for (int k = 0; k < K; k++) { + a(k) = (int8_t)((k * 31) % 255 - 127); + } + A.set(a); + + Buffer result = Acc.realize({4}); + int32_t sum = 0; + for (int k = 0; k < K; k++) { + sum += (int32_t)a(k); + } + const float expected = std::min((float)sum, 10000.0f); + for (int m = 0; m < 4; m++) { + internal_assert(result(m) == expected) + << "change_type sum-then-clamp mismatch at " << m << ": " << result(m) + << " vs " << expected << "\n"; + } + } +#endif + return 0; +} + +// A scan's self-reference is offset from the update's own coordinate (e.g. +// Acc(r-1) inside the update that defines Acc(r)), but change_type() treats +// any direct call to the accumulator as its self-reference regardless of +// offset, so the same reduction-extent bound applies as for a plain +// reduction: the worst case is every increment landing on a single output +// location. +int change_type_sum_scan_test() { + const int K = 100; + ImageParam A{Int(8), 1, "A_scan"}; + + Var x{"x"}; + RDom r(1, K - 1, "r"); + + Func Acc{"Acc_scan"}; + Acc(x) = cast(A(0)); + Acc(r) = Acc(r - 1) + cast(A(r)); + + // The worst-case bound treats all K-1 increments as landing on a single + // output element: (K - 1) * 127 = 12573 in magnitude, which fits Int(16). + Func Acc_i16 = Acc.change_type(Int(16)); + internal_assert(Acc_i16.types()[0] == Int(16)) + << "change_type scan: expected Int(16), got " << Acc_i16.types()[0] << "\n"; + Acc_i16.compute_root(); + + Buffer a(K); + for (int k = 0; k < K; k++) { + a(k) = (int8_t)((k * 31) % 255 - 127); + } + A.set(a); + + Buffer result = Acc.realize({K}); + int32_t running = a(0); + internal_assert(result(0) == (float)running) + << "change_type scan mismatch at 0: " << result(0) << " vs " << running << "\n"; + for (int k = 1; k < K; k++) { + running += (int32_t)a(k); + internal_assert(result(k) == (float)running) + << "change_type scan mismatch at " << k << ": " << result(k) + << " vs " << running << "\n"; + } + return 0; +} + +// A histogram's update writes to whichever bin A(r) selects, so any of the K +// increments could scatter into the same bin. change_type() bounds it exactly +// like a sum into a single accumulator, by (extent) * (per-term magnitude), +// rather than by the (much smaller) count any single bin can actually reach. +int change_type_histogram_test() { + const int K = 200; + const int NBINS = 8; + +#if HALIDE_WITH_EXCEPTIONS + if (Halide::exceptions_enabled()) { + ImageParam A{UInt(8), 1, "A_hist_narrow"}; + Var i{"i"}; + RDom r(0, K, "r"); + + Func Hist{"Hist_narrow"}; + Hist(i) = 0.0f; + Hist(cast(A(r)) % NBINS) += 1.0f; + + bool threw = false; + try { + Hist.change_type(Int(8)); + } catch (const Halide::CompileError &) { + threw = true; + } + internal_assert(threw) + << "change_type(Int(8)) should reject a histogram whose 200 increments " + << "could all land in the same bin\n"; + } +#endif + + ImageParam A{UInt(8), 1, "A_hist_wide"}; + Var i{"i"}; + RDom r(0, K, "r"); + + Func Hist{"Hist_wide"}; + Hist(i) = 0.0f; + Hist(cast(A(r)) % NBINS) += 1.0f; + + Func Hist_i16 = Hist.change_type(Int(16)); + internal_assert(Hist_i16.types()[0] == Int(16)) + << "change_type histogram: expected Int(16), got " << Hist_i16.types()[0] << "\n"; + Hist_i16.compute_root(); + + Buffer a(K); + for (int k = 0; k < K; k++) { + a(k) = (uint8_t)(k * 37); + } + A.set(a); + + Buffer result = Hist.realize({NBINS}); + int32_t expected[NBINS] = {0}; + for (int k = 0; k < K; k++) { + expected[(int)a(k) % NBINS]++; + } + for (int b = 0; b < NBINS; b++) { + internal_assert(result(b) == (float)expected[b]) + << "change_type histogram mismatch at bin " << b << ": " << result(b) + << " vs " << expected[b] << "\n"; + } + return 0; +} + } // namespace int main(int argc, char **argv) { @@ -1035,6 +1300,26 @@ int main(int argc, char **argv) { if (change_type_static_extent_count_does_not_overflow_test()) { return 1; } + printf("Running change_type_float_target_precision_test\n"); + if (change_type_float_target_precision_test()) { + return 1; + } + printf("Running change_type_float_target_precision_rejected_test\n"); + if (change_type_float_target_precision_rejected_test()) { + return 1; + } + printf("Running change_type_sum_then_clamp_test\n"); + if (change_type_sum_then_clamp_test()) { + return 1; + } + printf("Running change_type_sum_scan_test\n"); + if (change_type_sum_scan_test()) { + return 1; + } + printf("Running change_type_histogram_test\n"); + if (change_type_histogram_test()) { + return 1; + } printf("Success!\n"); return 0; From f72bf4326cb9a06814dfff1320dfd75cccca913f Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Sat, 8 Aug 2026 15:53:40 -0400 Subject: [PATCH 10/18] Add follow_global_wrappers arguments --- src/Func.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Func.cpp b/src/Func.cpp index 403ace9faf6b..821a989aad4e 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -3431,7 +3431,7 @@ Expr retype_value(const Expr &e, const string &fname, const Function &dst, Type const FuncValueBounds &fvb) { if (const Call *c = e.as()) { if (c->call_type == Call::Halide && c->name == fname) { - return Call::make(dst, c->args, c->value_index); + return Call::make(dst, c->args, c->value_index, /*follow_global_wrappers=*/false); } } if (contains_self_reference(e, fname)) { @@ -3872,7 +3872,7 @@ Func Func::change_type(Type t, bool unsafe) { // Rewrite this Func into an inline cast-back wrapper of the retyped clone, so // that every existing consumer keeps seeing the original type. - const Expr wrapped = cast(old_t, Call::make(typed.function(), pure_arg_exprs, 0)); + const Expr wrapped = cast(old_t, Call::make(typed.function(), pure_arg_exprs, 0, /*follow_global_wrappers=*/true)); vector arg_names; arg_names.reserve(pure_vars.size()); for (const Var &v : pure_vars) { From 8a3b4f7cfe469a4a411e77983ddc9aeec505481d Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Sat, 8 Aug 2026 16:19:44 -0400 Subject: [PATCH 11/18] Address review: thread FuncValueBounds through lossless_cast/constant_integer_bounds Replace the eager cache_call_bounds() prewarming pass with an optional FuncValueBounds parameter on constant_integer_bounds() and lossless_cast(), consulted lazily when either function hits a Call::Halide node. Also add a codegen test verifying that a change_type() retype from float to Int(32) reaches CodeGen_ARM's dot-product instruction selection. Co-Authored-By: Claude Sonnet 5 --- src/ConstantBounds.cpp | 17 ++++++--- src/ConstantBounds.h | 17 +++++---- src/Func.cpp | 46 +++++------------------- src/IROperator.cpp | 55 ++++++++++++++-------------- src/IROperator.h | 14 +++++--- test/correctness/change_type.cpp | 61 ++++++++++++++++++++++++++++++++ 6 files changed, 130 insertions(+), 80 deletions(-) diff --git a/src/ConstantBounds.cpp b/src/ConstantBounds.cpp index bf228f2c86a4..b8b158ebe9c0 100644 --- a/src/ConstantBounds.cpp +++ b/src/ConstantBounds.cpp @@ -9,11 +9,12 @@ namespace Internal { namespace { ConstantInterval bounds_helper(const Expr &e, Scope &scope, - std::map *cache) { + std::map *cache, + const FuncValueBounds *func_bounds) { internal_assert(e.defined()); auto recurse = [&](const Expr &e) { - return bounds_helper(e, scope, cache); + return bounds_helper(e, scope, cache, func_bounds); }; auto get_infinite_bounds = [&]() { @@ -129,6 +130,13 @@ ConstantInterval bounds_helper(const Expr &e, // We can't do much with the other bitwise ops, but we can treat // bitwise_not as an all-ones bit pattern minus the argument. return recurse(make_const(e.type(), -1) - op->args[0]); + } else if (func_bounds && op->call_type == Call::Halide && op->type.is_int_or_uint()) { + auto it = func_bounds->find({op->name, op->value_index}); + if (it != func_bounds->end()) { + return ConstantInterval::make_intersection( + ConstantInterval::bounds_of_type(op->type), + covering_constant_interval(it->second)); + } } // If you add a new intrinsic here, also add it to the expression // generator in test/correctness/lossless_cast.cpp @@ -163,10 +171,11 @@ ConstantInterval bounds_helper(const Expr &e, ConstantInterval constant_integer_bounds(const Expr &e, const Scope &scope, - std::map *cache) { + std::map *cache, + const FuncValueBounds *func_bounds) { Scope sub_scope; sub_scope.set_containing_scope(&scope); - return bounds_helper(e, sub_scope, cache); + return bounds_helper(e, sub_scope, cache, func_bounds); } } // namespace Internal diff --git a/src/ConstantBounds.h b/src/ConstantBounds.h index 26ab114455bb..4406d6e7a601 100644 --- a/src/ConstantBounds.h +++ b/src/ConstantBounds.h @@ -1,6 +1,7 @@ #ifndef HALIDE_CONSTANT_BOUNDS_H #define HALIDE_CONSTANT_BOUNDS_H +#include "Bounds.h" #include "ConstantInterval.h" #include "Expr.h" #include "Scope.h" @@ -19,15 +20,19 @@ namespace Internal { * negated, be incremented, etc without risking overflow. * * Also optionally accepts a scope containing the integer bounds of any - * variables that may be referenced, and a cache of constant integer bounds on - * known Exprs, which this function will update. The cache is helpful to - * short-circuit large numbers of redundant queries, but it should not be used - * in contexts where the same Expr object may take on different values within a - * single Expr (i.e. before uniquify_variable_names). + * variables that may be referenced, a cache of constant integer bounds on + * known Exprs, which this function will update, and previously-computed + * FuncValueBounds for any Halide Call nodes encountered, which lets a call to + * a producer Func (e.g. one known to be the result of a clamp) get a tighter + * bound than its type's full range. The cache is helpful to short-circuit + * large numbers of redundant queries, but it should not be used in contexts + * where the same Expr object may take on different values within a single + * Expr (i.e. before uniquify_variable_names). */ ConstantInterval constant_integer_bounds(const Expr &e, const Scope &scope = Scope::empty_scope(), - std::map *cache = nullptr); + std::map *cache = nullptr, + const FuncValueBounds *func_bounds = nullptr); } // namespace Internal } // namespace Halide diff --git a/src/Func.cpp b/src/Func.cpp index 821a989aad4e..e5d1161a1aa5 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -3332,35 +3332,6 @@ bool is_self_reference(const Expr &e, const string &fname) { return false; } -/** - * Initialize a constant-bounds cache with the FuncValueBounds-derived range - * of every Func call in \p e. The bounds machinery keys its cache by pointer - * identity (\ref Halide::ExprCompare), so using the exact Call nodes that - * appear in \p e lets \ref constant_integer_bounds and \ref lossless_cast see - * a tighter range than just the type's bounds. - * - * @return An initialized constant-bounds cache for \p e - */ -auto cache_call_bounds(const Expr &e, const FuncValueBounds &fvb) { - std::map cache; - if (!fvb.empty()) { - visit_with(e, [&](auto *self, const Call *op) { - self->visit_base(op); // Recurse into the call's arguments. - if (op->call_type != Call::Halide || !op->type.is_int_or_uint()) { - return; - } - auto it = fvb.find({op->name, op->value_index}); - if (it == fvb.end()) { - return; - } - auto type_range = ConstantInterval::bounds_of_type(op->type); - auto value_range = covering_constant_interval(it->second); - cache.emplace(Expr(op), ConstantInterval::make_intersection(type_range, value_range)); - }); - } - return cache; -} - // Rewrite a factor-free leaf expression `e` to type `t` Expr retype_leaf(const Expr &e, Type t, const FuncValueBounds &fvb) { if (e.type() == t) { @@ -3409,13 +3380,13 @@ Expr retype_leaf(const Expr &e, Type t, const FuncValueBounds &fvb) { // Retype via lossless_cast() when it can prove the cast exact, pushing it down // through widening intrinsics so integer forms survive to instruction - // selection. Seeding the cache with producer value ranges lets it succeed for - // casts that are only exact under those ranges (e.g. narrowing a clamped - // producer). This is a no-op-or-improvement for any target type: a float - // target just takes lossless_cast()'s representable-widening path, and - // anything it can't prove falls through to the plain cast below. - auto cache = cache_call_bounds(folded, fvb); - if (Expr r = lossless_cast(t, folded, Scope::empty_scope(), &cache); + // selection. Passing fvb lets it succeed for casts that are only exact under + // a producer's proven value range (e.g. narrowing a clamped producer). This + // is a no-op-or-improvement for any target type: a float target just takes + // lossless_cast()'s representable-widening path, and anything it can't + // prove falls through to the plain cast below. + std::map cache; + if (Expr r = lossless_cast(t, folded, Scope::empty_scope(), &cache, &fvb); r.defined()) { return r; } @@ -3654,8 +3625,7 @@ std::optional change_type_prove_safe( return ConstantInterval::single_point((int64_t)*fv); } } - auto cache = cache_call_bounds(v, fvb); - return constant_integer_bounds(v, Scope::empty_scope(), &cache); + return constant_integer_bounds(v, Scope::empty_scope(), nullptr, &fvb); }; // The initial accumulator value must be representable at the new type. Carry diff --git a/src/IROperator.cpp b/src/IROperator.cpp index 3a6e18b28a4c..b0e5dca973b1 100644 --- a/src/IROperator.cpp +++ b/src/IROperator.cpp @@ -502,17 +502,18 @@ Expr const_false(int w) { Expr lossless_cast(Type t, Expr e, const Scope &scope, - std::map *cache) { + std::map *cache, + const FuncValueBounds *func_bounds) { if (!e.defined() || t == e.type()) { return e; } else if (t.can_represent(e.type())) { return cast(t, std::move(e)); } else if (const Cast *c = e.as()) { if (c->type.can_represent(c->value.type())) { - return lossless_cast(t, c->value, scope, cache); + return lossless_cast(t, c->value, scope, cache, func_bounds); } } else if (const Broadcast *b = e.as()) { - Expr v = lossless_cast(t.with_lanes(b->value.type().lanes()), b->value, scope, cache); + Expr v = lossless_cast(t.with_lanes(b->value.type().lanes()), b->value, scope, cache, func_bounds); if (v.defined()) { return Broadcast::make(v, b->lanes); } @@ -531,7 +532,7 @@ Expr lossless_cast(Type t, } else if (const Shuffle *shuf = e.as()) { std::vector vecs; for (const auto &vec : shuf->vectors) { - vecs.emplace_back(lossless_cast(t.with_lanes(vec.type().lanes()), vec, scope, cache)); + vecs.emplace_back(lossless_cast(t.with_lanes(vec.type().lanes()), vec, scope, cache, func_bounds)); if (!vecs.back().defined()) { return Expr(); } @@ -540,72 +541,72 @@ Expr lossless_cast(Type t, } else if (t.is_int_or_uint()) { // Check the bounds. If they're small enough, we can throw narrowing // casts around e, or subterms. - ConstantInterval ci = constant_integer_bounds(e, scope, cache); + ConstantInterval ci = constant_integer_bounds(e, scope, cache, func_bounds); if (t.can_represent(ci)) { // There are certain IR nodes where if the result is expressible // using some type, and the args are expressible using that type, // then the operation can just be done in that type. if (const Add *op = e.as()) { - Expr a = lossless_cast(t, op->a, scope, cache); - Expr b = lossless_cast(t, op->b, scope, cache); + Expr a = lossless_cast(t, op->a, scope, cache, func_bounds); + Expr b = lossless_cast(t, op->b, scope, cache, func_bounds); if (a.defined() && b.defined()) { return Add::make(a, b); } } else if (const Sub *op = e.as()) { - Expr a = lossless_cast(t, op->a, scope, cache); - Expr b = lossless_cast(t, op->b, scope, cache); + Expr a = lossless_cast(t, op->a, scope, cache, func_bounds); + Expr b = lossless_cast(t, op->b, scope, cache, func_bounds); if (a.defined() && b.defined()) { return Sub::make(a, b); } } else if (const Mul *op = e.as()) { - Expr a = lossless_cast(t, op->a, scope, cache); - Expr b = lossless_cast(t, op->b, scope, cache); + Expr a = lossless_cast(t, op->a, scope, cache, func_bounds); + Expr b = lossless_cast(t, op->b, scope, cache, func_bounds); if (a.defined() && b.defined()) { return Mul::make(a, b); } } else if (const Min *op = e.as()) { - Expr a = lossless_cast(t, op->a, scope, cache); - Expr b = lossless_cast(t, op->b, scope, cache); + Expr a = lossless_cast(t, op->a, scope, cache, func_bounds); + Expr b = lossless_cast(t, op->b, scope, cache, func_bounds); if (a.defined() && b.defined()) { return Min::make(a, b); } } else if (const Max *op = e.as()) { - Expr a = lossless_cast(t, op->a, scope, cache); - Expr b = lossless_cast(t, op->b, scope, cache); + Expr a = lossless_cast(t, op->a, scope, cache, func_bounds); + Expr b = lossless_cast(t, op->b, scope, cache, func_bounds); if (a.defined() && b.defined()) { return Max::make(a, b); } } else if (const Mod *op = e.as()) { - Expr a = lossless_cast(t, op->a, scope, cache); - Expr b = lossless_cast(t, op->b, scope, cache); + Expr a = lossless_cast(t, op->a, scope, cache, func_bounds); + Expr b = lossless_cast(t, op->b, scope, cache, func_bounds); if (a.defined() && b.defined()) { return Mod::make(a, b); } } else if (const Call *op = Call::as_intrinsic(e, {Call::widening_add, Call::widen_right_add})) { - Expr a = lossless_cast(t, op->args[0], scope, cache); - Expr b = lossless_cast(t, op->args[1], scope, cache); + Expr a = lossless_cast(t, op->args[0], scope, cache, func_bounds); + Expr b = lossless_cast(t, op->args[1], scope, cache, func_bounds); if (a.defined() && b.defined()) { return Add::make(a, b); } } else if (const Call *op = Call::as_intrinsic(e, {Call::widening_sub, Call::widen_right_sub})) { - Expr a = lossless_cast(t, op->args[0], scope, cache); - Expr b = lossless_cast(t, op->args[1], scope, cache); + Expr a = lossless_cast(t, op->args[0], scope, cache, func_bounds); + Expr b = lossless_cast(t, op->args[1], scope, cache, func_bounds); if (a.defined() && b.defined()) { return Sub::make(a, b); } } else if (const Call *op = Call::as_intrinsic(e, {Call::widening_mul, Call::widen_right_mul})) { - Expr a = lossless_cast(t, op->args[0], scope, cache); - Expr b = lossless_cast(t, op->args[1], scope, cache); + Expr a = lossless_cast(t, op->args[0], scope, cache, func_bounds); + Expr b = lossless_cast(t, op->args[1], scope, cache, func_bounds); if (a.defined() && b.defined()) { return Mul::make(a, b); } } else if (const Call *op = Call::as_intrinsic(e, {Call::shift_left, Call::widening_shift_left, Call::shift_right, Call::widening_shift_right})) { - Expr a = lossless_cast(t, op->args[0], scope, cache); - Expr b = lossless_cast(t, op->args[1], scope, cache); + Expr a = lossless_cast(t, op->args[0], scope, cache, func_bounds); + Expr b = lossless_cast(t, op->args[1], scope, cache, func_bounds); if (a.defined() && b.defined()) { - ConstantInterval cb = constant_integer_bounds(b, scope, cache); + ConstantInterval cb = constant_integer_bounds(b, scope, cache, func_bounds); if (cb > -t.bits() && cb < t.bits()) { if (op->is_intrinsic({Call::shift_left, Call::widening_shift_left})) { return a << b; @@ -618,7 +619,7 @@ Expr lossless_cast(Type t, if ((t.bits() > 1 && op->op == VectorReduce::Add) || op->op == VectorReduce::Min || op->op == VectorReduce::Max) { - Expr v = lossless_cast(t.with_lanes(op->value.type().lanes()), op->value, scope, cache); + Expr v = lossless_cast(t.with_lanes(op->value.type().lanes()), op->value, scope, cache, func_bounds); if (v.defined()) { auto reduce_op = op->op; if (t.bits() == 1) { diff --git a/src/IROperator.h b/src/IROperator.h index b9499873c018..14d2b20d6699 100644 --- a/src/IROperator.h +++ b/src/IROperator.h @@ -12,6 +12,7 @@ #include #include +#include "Bounds.h" #include "ConstantInterval.h" #include "Expr.h" #include "Scope.h" @@ -161,16 +162,19 @@ Expr const_false(int lanes = 1); /** Attempt to cast an expression to a smaller type while provably not losing * information. If it can't be done, return an undefined Expr. * - * Optionally accepts a scope giving the constant bounds of any variables, and a + * Optionally accepts a scope giving the constant bounds of any variables, a * map that gives the constant bounds of exprs already analyzed to avoid redoing - * work across many calls to lossless_cast. It is not safe to use this optional - * map in contexts where the same Expr object may take on a different value. For - * example: (let x = 4 in some_expr_object) + (let x = 5 in + * work across many calls to lossless_cast, and previously-computed + * FuncValueBounds for any Halide Call nodes encountered (see + * constant_integer_bounds() in ConstantBounds.h). It is not safe to use the + * cache map in contexts where the same Expr object may take on a different + * value. For example: (let x = 4 in some_expr_object) + (let x = 5 in * the_same_expr_object)). It is safe to use it after uniquify_variable_names * has been run. */ Expr lossless_cast(Type t, Expr e, const Scope &scope = Scope::empty_scope(), - std::map *cache = nullptr); + std::map *cache = nullptr, + const FuncValueBounds *func_bounds = nullptr); /** Attempt to negate x without introducing new IR and without overflow. * If it can't be done, return an undefined Expr. */ diff --git a/test/correctness/change_type.cpp b/test/correctness/change_type.cpp index 8f74039952cd..5610a69c36c0 100644 --- a/test/correctness/change_type.cpp +++ b/test/correctness/change_type.cpp @@ -1,7 +1,9 @@ #include "Halide.h" +#include "halide_test_dirs.h" #include #include #include +#include #include #include @@ -355,6 +357,61 @@ int change_type_narrowing_dot_product_test() { return 0; } +// A retype from float to Int(32) should expose the same widening_mul reduction +// shape that a Func written directly in Int(32) would, so it's eligible for the +// same instruction selection. This cross-compiles for an arm64 target with the +// dot-product extension (no need to run the code) and checks that the +// generated assembly contains an `sdot` instruction, confirming that the +// retyped reduction reaches CodeGen_ARM's dot-product pattern rather than +// falling back to scalar-equivalent widening adds. +int change_type_float_to_dot_product_codegen_test() { + Target target = Target(Target::Linux, Target::ARM, 64) + .with_feature(Target::ARMDotProd) + .with_feature(Target::NoAsserts) + .with_feature(Target::NoBoundsQuery) + .with_feature(Target::NoRuntime); + + const int K = 4; + ImageParam A{Int(8), 1, "A"}, B{Int(8), 1, "B"}; + + Var x{"x"}, xo{"xo"}, xi{"xi"}; + RDom r(0, K, "r"); + + Func Acc{"Acc"}; + Acc(x) = 0.0f; + Acc(x) += cast(A(x * K + r)) * cast(B(x * K + r)); + + Func Acc_i32 = Acc.change_type(Int(32)); + internal_assert(Acc_i32.types()[0] == Int(32)) + << "change_type float-to-dot-product: expected Int(32), got " << Acc_i32.types()[0] << "\n"; + + Acc_i32.compute_root().bound(x, 0, 16).vectorize(x, 4); + Acc_i32.update(0) + .atomic(true) + .vectorize(r) + .split(x, xo, xi, 4) + .vectorize(xi); + + std::string file = Internal::get_test_tmp_dir() + "change_type_dot_product.s"; + Acc_i32.compile_to_assembly(file, {A, B}, target); + + std::ifstream asm_file(file); + internal_assert(asm_file.is_open()) << "Failed to open " << file << "\n"; + bool found_sdot = false; + std::string line; + while (getline(asm_file, line)) { + if (line.find("sdot") != std::string::npos) { + found_sdot = true; + break; + } + } + internal_assert(found_sdot) + << "change_type float-to-dot-product: expected an sdot instruction in the " + "generated assembly, but none was found in " + << file << "\n"; + return 0; +} + // A narrowing change_type() that can't be proven exact must be rejected rather // than silently truncating -- unless the caller opts in with unsafe = true. This // covers both the pure path and the min/max reduction path, whose per-term casts @@ -1244,6 +1301,10 @@ int main(int argc, char **argv) { if (change_type_narrowing_dot_product_test()) { return 1; } + printf("Running change_type_float_to_dot_product_codegen_test\n"); + if (change_type_float_to_dot_product_codegen_test()) { + return 1; + } printf("Running change_type_truncating_rejected_test\n"); if (change_type_truncating_rejected_test()) { return 1; From c2da8c6e9036d8893e9d0e41e732fbe3f3e05a49 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 13 Aug 2026 09:25:12 -0400 Subject: [PATCH 12/18] Implement covering_constant_interval with constant_integer_bounds --- src/ConstantInterval.cpp | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/ConstantInterval.cpp b/src/ConstantInterval.cpp index ec7e587efbc4..36c7e79294d2 100644 --- a/src/ConstantInterval.cpp +++ b/src/ConstantInterval.cpp @@ -1,5 +1,6 @@ #include "ConstantInterval.h" +#include "ConstantBounds.h" #include "Error.h" #include "IROperator.h" #include "IRPrinter.h" @@ -150,20 +151,14 @@ ConstantInterval ConstantInterval::make_intersection(const ConstantInterval &a, } ConstantInterval covering_constant_interval(const Interval &in) { - ConstantInterval ci = ConstantInterval::everything(); - if (in.has_lower_bound()) { - if (auto lo = as_const_int(in.min)) { - ci.min_defined = true; - ci.min = *lo; - } - } - if (in.has_upper_bound()) { - if (auto hi = as_const_int(in.max)) { - ci.max_defined = true; - ci.max = *hi; - } - } - return ci; + auto min_bounds = constant_integer_bounds(in.min); + auto max_bounds = constant_integer_bounds(in.max); + ConstantInterval result; + result.min = min_bounds.min; + result.min_defined = min_bounds.min_defined; + result.max = max_bounds.max; + result.max_defined = max_bounds.max_defined; + return result; } void ConstantInterval::operator+=(const ConstantInterval &other) { From 232c41092155815c56b9cd14920b9b764d79e669 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 13 Aug 2026 09:33:08 -0400 Subject: [PATCH 13/18] Improve comment on int-to-float cast-stripping --- src/Func.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Func.cpp b/src/Func.cpp index e5d1161a1aa5..40e7b86dc910 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -3365,11 +3365,11 @@ Expr retype_leaf(const Expr &e, Type t, const FuncValueBounds &fvb) { } } - // Peel an int->float promotion: we're accumulating at an integer type, so - // the float round-trip is dead weight. This also exposes an integer form - // (e.g. cast(widening_mul(a, b)) -> widening_mul(a, b)) that - // lossless_cast() can retype without a detour through float, which f32 - // can't always undo. A strict_cast is a Call, not a Cast, and is left alone. + // lossless_cast uses Type::can_represent to decide if it can strip an outer + // cast, which uses strict, not fast-math semantics. When accumulating at an + // integer type, we peel a redundant int-to-float promotion here according to + // fast-math semantics. This exposes an integer form that lossless_cast can + // retype. A strict_cast is a Call, not a Cast, and is left alone. if (t.is_int_or_uint()) { if (const Cast *c = folded.as()) { if (folded.type().is_float() && c->value.type().is_int_or_uint()) { From 46279db653d4a7e829be51192f2edfb0ce7ac557 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 13 Aug 2026 09:49:09 -0400 Subject: [PATCH 14/18] Make retype_value not-recursive, clarify comment. --- src/Func.cpp | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/Func.cpp b/src/Func.cpp index 40e7b86dc910..5714c9944e24 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -3394,17 +3394,29 @@ Expr retype_leaf(const Expr &e, Type t, const FuncValueBounds &fvb) { return cast(t, folded); } -// Retype a whole definition value to type `t`, retargeting a direct -// self-reference from `fname` to `dst` and retyping the other operand as the -// increment. The direct-call restriction keeps the recurrence visible to the -// overflow proof. +// Retype a whole definition value to type `t`, retargeting a direct self-reference +// from `fname` to `dst` and retyping the other operand as the increment. We only +// recognize values in a few simple forms: +// +// 1. A bare self-reference: f(..) = f(..) (e.g. transpose or copy) +// 2. A direct reduction: f(..) = f(..) OP increment (or increment OP f(..)) +// where `increment` contains no self-references. +// 3. An expression with no self-references. +// +// This constraint keeps the recurrence visible to the overflow proof. Expr retype_value(const Expr &e, const string &fname, const Function &dst, Type t, const FuncValueBounds &fvb) { - if (const Call *c = e.as()) { - if (c->call_type == Call::Halide && c->name == fname) { + auto retype_self_reference = [&](const Expr &expr) { + if (const Call *c = expr.as(); c && c->call_type == Call::Halide && c->name == fname) { return Call::make(dst, c->args, c->value_index, /*follow_global_wrappers=*/false); } + return Expr(); + }; + + if (Expr self = retype_self_reference(e); self.defined()) { + return self; } + if (contains_self_reference(e, fname)) { optional> operands = as_binary_operands(e); user_assert(operands) << "change_type() only supports update definitions " @@ -3420,10 +3432,11 @@ Expr retype_value(const Expr &e, const string &fname, const Function &dst, Type user_assert(e.node_type() != IRNodeType::Sub || a_is_self) << "change_type() only supports difference reductions of the form " << fname << "(...) - term.\n"; - return make_binary_op(e.node_type(), - retype_value(operands->first, fname, dst, t, fvb), - retype_value(operands->second, fname, dst, t, fvb)); + Expr a = a_is_self ? retype_self_reference(operands->first) : retype_leaf(operands->first, t, fvb); + Expr b = b_is_self ? retype_self_reference(operands->second) : retype_leaf(operands->second, t, fvb); + return make_binary_op(e.node_type(), a, b); } + return retype_leaf(e, t, fvb); } From e5e7aa7a621c9cc359a7f8aba1263aa7c9f93dd6 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 13 Aug 2026 09:50:52 -0400 Subject: [PATCH 15/18] Drop impossible uint case for RVar extent --- src/Func.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Func.cpp b/src/Func.cpp index 5714c9944e24..f9d9b8e783b5 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -3543,10 +3543,6 @@ std::optional nonempty_dense_update_precondition(const Function &fn if (*ext <= 0) { return "the first update has an empty reduction domain"; } - } else if (optional ext = as_const_uint(extent)) { - if (*ext == 0) { - return "the first update has an empty reduction domain"; - } } else { positive_extents = positive_extents && (cast(Int(64), extent) > 0); From eff2cef2f86adc95cc4aa7f0a6c59540a7be14c9 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 13 Aug 2026 10:07:09 -0400 Subject: [PATCH 16/18] Use Op::make rather than Op's operator overload --- src/IROperator.cpp | 49 ++++++++++++++++++---------------------------- src/IROperator.h | 3 +-- 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/src/IROperator.cpp b/src/IROperator.cpp index b0e5dca973b1..5e4e5146f05a 100644 --- a/src/IROperator.cpp +++ b/src/IROperator.cpp @@ -280,36 +280,25 @@ std::optional> as_binary_operands(const Expr &e) { Expr make_binary_op(IRNodeType t, const Expr &a, const Expr &b) { switch (t) { - case IRNodeType::Add: - return a + b; - case IRNodeType::Sub: - return a - b; - case IRNodeType::Mul: - return a * b; - case IRNodeType::Div: - return a / b; - case IRNodeType::Mod: - return a % b; - case IRNodeType::Min: - return min(a, b); - case IRNodeType::Max: - return max(a, b); - case IRNodeType::EQ: - return a == b; - case IRNodeType::NE: - return a != b; - case IRNodeType::LT: - return a < b; - case IRNodeType::LE: - return a <= b; - case IRNodeType::GT: - return a > b; - case IRNodeType::GE: - return a >= b; - case IRNodeType::And: - return a && b; - case IRNodeType::Or: - return a || b; +#define HANDLE_BINARY_OP(NodeType) \ + case IRNodeType::NodeType: \ + return NodeType::make(a, b); + HANDLE_BINARY_OP(Add) + HANDLE_BINARY_OP(Sub) + HANDLE_BINARY_OP(Mul) + HANDLE_BINARY_OP(Div) + HANDLE_BINARY_OP(Mod) + HANDLE_BINARY_OP(Min) + HANDLE_BINARY_OP(Max) + HANDLE_BINARY_OP(EQ) + HANDLE_BINARY_OP(NE) + HANDLE_BINARY_OP(LT) + HANDLE_BINARY_OP(LE) + HANDLE_BINARY_OP(GT) + HANDLE_BINARY_OP(GE) + HANDLE_BINARY_OP(And) + HANDLE_BINARY_OP(Or) +#undef HANDLE_BINARY_OP default: internal_error << "make_binary_op: " << IRNodeType_string(t) << " is not a binary operator\n"; diff --git a/src/IROperator.h b/src/IROperator.h index 14d2b20d6699..f9ae806d6356 100644 --- a/src/IROperator.h +++ b/src/IROperator.h @@ -56,8 +56,7 @@ std::optional is_const_power_of_two_integer(int64_t); std::optional> as_binary_operands(const Expr &e); /** Build a binary expression of node type `t` from operands `a` and `b`, using - * the corresponding operator overload (so the usual type matching and constant - * folding apply). `t` must be a binary operator; it is an internal error otherwise. */ + * the corresponding Op::make function. */ Expr make_binary_op(IRNodeType t, const Expr &a, const Expr &b); /** Is the expression a const (as defined by is_const), and also From 7c1854eac3adf6c08540fa59ac65652b84d4aa06 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 13 Aug 2026 10:10:56 -0400 Subject: [PATCH 17/18] Clarify comment for nonempty_dense_update_precondition --- src/Func.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Func.cpp b/src/Func.cpp index f9d9b8e783b5..4a468f29c9db 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -3511,9 +3511,11 @@ Expr reduction_cardinality_fits(const Definition &def, int64_t limit) { return simplify(all_non_negative && (any_zero || product_fits)); } -// Prove that the first update executes at least once for every pure coordinate, -// as required when translating an identity that does not round-trip through the -// target type. Symbolic extents produce a runtime precondition. +// Prove that the first update runs at least once for every pure coordinate. +// This is needed when an identity can't safely round-trip through the result type, +// so symbolic extents become a runtime precondition. Example: a min-histogram can +// use a sentinel like inf in float, but not in int, because int cannot distinguish +// "untouched" from the minimum possible value. std::optional nonempty_dense_update_precondition(const Function &fn, Expr *condition) { *condition = Expr(); From 9aff833326996ced1d70031889bdf59fecea5041 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Thu, 13 Aug 2026 10:12:20 -0400 Subject: [PATCH 18/18] Add TODO for saturating_add/sub --- src/Func.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Func.cpp b/src/Func.cpp index 4a468f29c9db..468188530c67 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -3688,6 +3688,7 @@ std::optional change_type_prove_safe( // below; every other combiner can grow it faster than a single term's // range in a way we don't model -- a product reduction most importantly, // or an unrecognized shape -- so reject it rather than silently overflow. + // TODO: add saturating_add/sub to match VectorReduce support? if (!op || (*op != IRNodeType::Add && *op != IRNodeType::Sub)) { return "change_type() only supports sum, difference, min, max, and, " "and or reductions; this reduction's accumulator could overflow "