From e15df0716dc69987295f357cd382bd2c0bb99fa1 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Tue, 6 Nov 2018 11:29:36 -0800 Subject: [PATCH 1/8] Infer input/output buffer constraints from schedule directives This is still a work-in-progress that isn't ready for review, but I'm opening a PR to get wider feedback on it; it's not ready for a proper code review yet, but a conceptual review would be welcome. The idea here (from @dsharlet) is that we can use the existing scheduling language to infer the right constraints for input and output buffers, avoiding the need to use `dim().set_stride()` and such entirely; this is motivated by several things: - More compact code, less redundant specification (cf the changes to test/interleave) - Ability to specify constraints for Generators that use `Input` or `Output`; putting schedule-methods shims onto these allows you to conveniently use such a Generator both in 'standalone' mode, or via GeneratorStub (in which case the constraints would simply be ignored for inlined Funcs.) Theoretically, this would allow us to migrate all code to simply use scheduling directives and ~never need to use the set_stride(), etc methods; in practice, there are some holes that aren't addressable with the existing scheduling directives: - There's no way to set stride explicitly to a strange value (eg if you have a buffer that is referring to some array of structs or some such) - There's no way to set min without setting extent (eg if you want min=0 but extent aligned to a 16-byte boundary); align_bounds() aligns both min and extent. - There's no way to clamp an extent to an upper bound (eg if you want to ensure that a buffer is <= a hardware-specific limit) Also, the implementation has some ugly bits: - We emit user warnings if you attempt to constrain the bounds of an inline Func (since this is meaningless and probably an error); with these changes, it becomes reasonable to set constraints on the inputs or outputs of a Generator, which might be ignored in some situations. Rather than try to finesse this, I've just inserted a flag to allow suppressing these warnings for a given `Function`, and then ruthlessly set it for Generator inputs and outputs. This is probably ok, but it has a code smell I don't like. - There's some handwavy TODO stuff in the Generator code (re: limiting access to certain methods) that needs cleaning up. - There are a handful of existing tests and apps that have failures that need investigating. --- python_bindings/correctness/basics.py | 2 +- src/AddImageChecks.cpp | 13 +- src/Generator.cpp | 14 ++- src/Generator.h | 29 ++++- src/ImageParam.cpp | 42 +++++++ src/ImageParam.h | 15 +++ src/Inline.cpp | 28 +++-- src/Lower.cpp | 6 +- src/Parameter.cpp | 98 +++++++++++++++ src/Parameter.h | 7 +- src/Schedule.cpp | 13 +- src/Schedule.h | 7 ++ test/correctness/constraints.cpp | 127 +++++++++++++++----- test/correctness/interleave.cpp | 49 +------- test/correctness/interleave_rgb.cpp | 5 +- test/correctness/storage_folding.cpp | 7 +- test/generator/error_codes_aottest.cpp | 4 +- test/generator/nested_externs_generator.cpp | 2 +- test/generator/stubtest_aottest.cpp | 16 +-- test/generator/stubtest_generator.cpp | 23 ++++ test/generator/stubtest_jittest.cpp | 30 ++--- 21 files changed, 403 insertions(+), 134 deletions(-) diff --git a/python_bindings/correctness/basics.py b/python_bindings/correctness/basics.py index 6fd12de15c0f..80749e62aabe 100644 --- a/python_bindings/correctness/basics.py +++ b/python_bindings/correctness/basics.py @@ -28,7 +28,7 @@ def test_runtime_error(): try: f.realize(buf) except RuntimeError as e: - assert 'do not cover required region' in str(e) + assert 'Constraint violated' in str(e) else: assert False, 'Did not see expected exception!' diff --git a/src/AddImageChecks.cpp b/src/AddImageChecks.cpp index 15431838552a..fb301b97e8ad 100644 --- a/src/AddImageChecks.cpp +++ b/src/AddImageChecks.cpp @@ -1,4 +1,5 @@ #include "AddImageChecks.h" +#include "IREquality.h" #include "IRVisitor.h" #include "Simplify.h" #include "Substitute.h" @@ -189,6 +190,7 @@ Stmt add_image_checks(Stmt s, bool is_output_buffer = false; bool is_secondary_output_buffer = false; string buffer_name = name; + Parameter primary_output_buffer; for (Function f : outputs) { for (size_t i = 0; i < f.output_buffers().size(); i++) { if (param.defined() && @@ -199,6 +201,7 @@ Stmt add_image_checks(Stmt s, buffer_name = f.name(); if (i > 0) { is_secondary_output_buffer = true; + primary_output_buffer = f.output_buffers()[0]; } } } @@ -449,8 +452,14 @@ Stmt add_image_checks(Stmt s, // constrained to match the first output. if (param.defined()) { - user_assert(!param.extent_constraint(i).defined() && - !param.min_constraint(i).defined()) + // Parameter::set_constraints_from_schedule() can set the min/extent + // on secondary buffers, so don't complain if they are identical + // to the primary. + const auto constraint_ok = [](const Expr &secondary, const Expr &primary) -> bool { + return !secondary.defined() || equal(secondary, primary); + }; + user_assert(constraint_ok(param.extent_constraint(i), primary_output_buffer.extent_constraint(i)) && + constraint_ok(param.min_constraint(i), primary_output_buffer.min_constraint(i))) << "Can't constrain the min or extent of an output buffer beyond the " << "first. They are implicitly constrained to have the same min and extent " << "as the first output buffer.\n"; diff --git a/src/Generator.cpp b/src/Generator.cpp index 54ce574cb107..41ac1356824d 100644 --- a/src/Generator.cpp +++ b/src/Generator.cpp @@ -152,6 +152,7 @@ Func make_param_func(const Parameter &p, const std::string &name) { } f(args) = Internal::Call::make(p, args_expr); } + f.function().schedule().emit_inliner_warnings() = false; return f; } @@ -1165,6 +1166,7 @@ Func GeneratorBase::get_output(const std::string &n) { user_assert(!output->is_array() && output->funcs().size() == 1) << "Output " << n << " must be accessed via get_array_output()\n"; Func f = output->funcs().at(0); user_assert(f.defined()) << "Output " << n << " was not defined.\n"; + f.function().schedule().emit_inliner_warnings() = false; return f; } @@ -1175,6 +1177,7 @@ std::vector GeneratorBase::get_array_output(const std::string &n) { (void) output->array_size(); for (const auto &f : output->funcs()) { user_assert(f.defined()) << "Output " << n << " was not fully defined.\n"; + f.function().schedule().emit_inliner_warnings() = false; } return output->funcs(); } @@ -1390,6 +1393,14 @@ Module GeneratorBase::build_module(const std::string &function_name, ParamInfo &pi = param_info(); std::vector filter_arguments; for (auto input : pi.filter_inputs) { + if (input->kind() == IOKind::Function) { + internal_assert(input->parameters_.size() == input->funcs_.size()); + for (size_t i = 0; i < input->parameters_.size(); ++i) { + Function f = input->funcs_.at(i).function(); + // Transfer the constraints from the Function to the Parameter + input->parameters_.at(i).set_constraints_from_schedule(f); + } + } for (const auto &p : input->parameters_) { filter_arguments.push_back(to_argument(p, p.is_buffer() ? Expr() : input->get_def_expr())); } @@ -1695,6 +1706,7 @@ void GeneratorInputBase::set_inputs(const std::vector &inputs) { if (kind() == IOKind::Function) { auto f = in.func(); user_assert(f.defined()) << "The input for " << name() << " is an undefined Func. Please define it.\n"; + f.function().schedule().emit_inliner_warnings() = false; check_matching_types(f.output_types()); check_matching_dims(f.dimensions()); funcs_.push_back(f); @@ -1998,7 +2010,7 @@ void generator_test() { static_assert(std::is_same::value, "type mismatch"); static_assert(std::is_same::value, "type mismatch"); - static_assert(std::is_same::value, "type mismatch"); + static_assert(std::is_same::value, "type mismatch"); static_assert(std::is_same::value, "type mismatch"); static_assert(std::is_same::value, "type mismatch"); diff --git a/src/Generator.h b/src/Generator.h index 1aa8c8258234..13fc3fe9a11d 100644 --- a/src/Generator.h +++ b/src/Generator.h @@ -1567,6 +1567,13 @@ class GeneratorInput_Buffer : public GeneratorInputImpl { HALIDE_FORWARD_METHOD_CONST(ImageParam, channels) HALIDE_FORWARD_METHOD_CONST(ImageParam, trace_loads) HALIDE_FORWARD_METHOD_CONST(ImageParam, add_trace_tag) + + // Forward scheduling methods that are relevant for storage + HALIDE_FORWARD_METHOD(ImageParam, align_bounds) + HALIDE_FORWARD_METHOD(ImageParam, align_storage) + HALIDE_FORWARD_METHOD(ImageParam, bound) + HALIDE_FORWARD_METHOD(ImageParam, bound_extent) + HALIDE_FORWARD_METHOD(ImageParam, reorder_storage) // }@ }; @@ -1644,6 +1651,18 @@ class GeneratorInput_Func : public GeneratorInputImpl { return ExternFuncArgument(this->parameters_.at(0)); } + template ::value>::type * = nullptr> + Func operator[](size_t i) { + // TODO: we'd like to prevent doing foo[i]=somefunc, but need to allow foo[i].schedule() + return this->funcs()[i]; + } + + template ::value>::type * = nullptr> + Func at(size_t i) { + // TODO: we'd like to prevent doing foo[i]=somefunc, but need to allow foo[i].schedule() + return this->funcs().at(i); + } + GeneratorInput_Func &estimate(Var var, Expr min, Expr extent) { this->estimate_impl(var, min, extent); return *this; @@ -1676,6 +1695,13 @@ class GeneratorInput_Func : public GeneratorInputImpl { HALIDE_FORWARD_METHOD_CONST(Func, update_values) HALIDE_FORWARD_METHOD_CONST(Func, value) HALIDE_FORWARD_METHOD_CONST(Func, values) + + // Forward scheduling methods that are relevant for storage + HALIDE_FORWARD_METHOD(Func, align_bounds) + HALIDE_FORWARD_METHOD(Func, align_storage) + HALIDE_FORWARD_METHOD(Func, bound) + HALIDE_FORWARD_METHOD(Func, bound_extent) + HALIDE_FORWARD_METHOD(Func, reorder_storage) // }@ }; @@ -2286,7 +2312,8 @@ class GeneratorOutput_Func : public GeneratorOutputImpl { // Allow Output = Func template ::value>::type * = nullptr> Func &operator[](size_t i) { - this->check_value_writable(); + // TODO: we'd like to prevent doing foo[i]=somefunc, but need to allow foo[i].schedule() + // this->check_value_writable(); return get_assignable_func_ref(i); } diff --git a/src/ImageParam.cpp b/src/ImageParam.cpp index 8a66a477cdf2..d1b7eabb1b11 100644 --- a/src/ImageParam.cpp +++ b/src/ImageParam.cpp @@ -93,4 +93,46 @@ ImageParam &ImageParam::add_trace_tag(const std::string &trace_tag) { return *this; } +ImageParam &ImageParam::align_bounds(Var var, Expr modulus, Expr remainder) { + internal_assert(func.defined()); + func.align_bounds(var, modulus, remainder); + parameter().set_constraints_from_schedule(func.function()); + return *this; +} + +ImageParam &ImageParam::align_storage(Var dim, Expr alignment) { + internal_assert(func.defined()); + func.align_storage(dim, alignment); + parameter().set_constraints_from_schedule(func.function()); + return *this; +} + +ImageParam &ImageParam::bound(Var var, Expr min, Expr extent) { + internal_assert(func.defined()); + func.bound(var, min, extent); + parameter().set_constraints_from_schedule(func.function()); + return *this; +} + +ImageParam &ImageParam::bound_extent(Var var, Expr extent) { + internal_assert(func.defined()); + func.bound_extent(var, extent); + parameter().set_constraints_from_schedule(func.function()); + return *this; +} + +ImageParam &ImageParam::reorder_storage(const std::vector &dims) { + internal_assert(func.defined()); + func.reorder_storage(dims); + parameter().set_constraints_from_schedule(func.function()); + return *this; +} + +ImageParam &ImageParam::reorder_storage(Var x, Var y) { + internal_assert(func.defined()); + func.reorder_storage(x, y); + parameter().set_constraints_from_schedule(func.function()); + return *this; +} + } // namespace Halide diff --git a/src/ImageParam.h b/src/ImageParam.h index d5b09b112230..518051fa7cec 100644 --- a/src/ImageParam.h +++ b/src/ImageParam.h @@ -129,6 +129,21 @@ class ImageParam : public OutputImageParam { /** Add a trace tag to this ImageParam's Func. */ ImageParam &add_trace_tag(const std::string &trace_tag); + + // Forward scheduling methods that are relevant for storage + ImageParam &align_bounds(Var var, Expr modulus, Expr remainder = 0); + ImageParam &align_storage(Var dim, Expr alignment); + ImageParam &bound(Var var, Expr min, Expr extent); + ImageParam &bound_extent(Var var, Expr extent); + ImageParam &reorder_storage(const std::vector &dims); + ImageParam &reorder_storage(Var x, Var y); + + template + HALIDE_NO_USER_CODE_INLINE typename std::enable_if::value, ImageParam &>::type + reorder_storage(Var x, Var y, Args&&... args) { + std::vector collected_args{x, y, std::forward(args)...}; + return reorder_storage(collected_args); + } }; } // namespace Halide diff --git a/src/Inline.cpp b/src/Inline.cpp index 1fa207f8b6f0..1c0d7264a8d6 100644 --- a/src/Inline.cpp +++ b/src/Inline.cpp @@ -74,19 +74,21 @@ void validate_schedule_inlined_function(Function f) { } } - for (size_t i = 0; i < func_s.bounds().size(); i++) { - if (func_s.bounds()[i].min.defined()) { - user_warning << "It is meaningless to bound dimension " - << func_s.bounds()[i].var << " of function " - << f.name() << " to be within [" - << func_s.bounds()[i].min << ", " - << func_s.bounds()[i].extent << "] because the function is scheduled inline.\n"; - } else if (func_s.bounds()[i].modulus.defined()) { - user_warning << "It is meaningless to align the bounds of dimension " - << func_s.bounds()[i].var << " of function " - << f.name() << " to have modulus/remainder [" - << func_s.bounds()[i].modulus << ", " - << func_s.bounds()[i].remainder << "] because the function is scheduled inline.\n"; + if (func_s.emit_inliner_warnings()) { + for (size_t i = 0; i < func_s.bounds().size(); i++) { + if (func_s.bounds()[i].min.defined()) { + user_warning << "It is meaningless to bound dimension " + << func_s.bounds()[i].var << " of function " + << f.name() << " to be within [" + << func_s.bounds()[i].min << ", " + << func_s.bounds()[i].extent << "] because the function is scheduled inline.\n"; + } else if (func_s.bounds()[i].modulus.defined()) { + user_warning << "It is meaningless to align the bounds of dimension " + << func_s.bounds()[i].var << " of function " + << f.name() << " to have modulus/remainder [" + << func_s.bounds()[i].modulus << ", " + << func_s.bounds()[i].remainder << "] because the function is scheduled inline.\n"; + } } } } diff --git a/src/Lower.cpp b/src/Lower.cpp index 403c00321c8b..2363630ffb7f 100644 --- a/src/Lower.cpp +++ b/src/Lower.cpp @@ -36,6 +36,7 @@ #include "LoopCarry.h" #include "LowerWarpShuffles.h" #include "Memoization.h" +#include "OutputImageParam.h" #include "PartitionLoops.h" #include "PurifyIndexMath.h" #include "Prefetch.h" @@ -100,8 +101,11 @@ Module lower(const vector &output_funcs, const string &pipeline_name, result_module.set_any_strict_float(any_strict_float); // Output functions should all be computed and stored at root. - for (Function f: outputs) { + for (Function f : outputs) { Func(f).compute_root().store_root(); + for (auto output_buffer : Func(f).output_buffers()) { + output_buffer.parameter().set_constraints_from_schedule(f); + } } // Finalize all the LoopLevels diff --git a/src/Parameter.cpp b/src/Parameter.cpp index ac1d3e947f65..87470e99e960 100644 --- a/src/Parameter.cpp +++ b/src/Parameter.cpp @@ -1,4 +1,5 @@ #include "IR.h" +#include "IREquality.h" #include "IROperator.h" #include "ObjectInstanceRegistry.h" #include "Parameter.h" @@ -283,6 +284,103 @@ Expr Parameter::estimate() const { return contents->estimate; } +// Add constraints to a buffer based on the storage scheduling for f. +void Parameter::set_constraints_from_schedule(Function f) { + constexpr int D = 1; + const std::string ¶m_name = this->name(); + const FuncSchedule &schedule = f.schedule(); + const std::vector &storage_dims = schedule.storage_dims(); + const std::vector &args = f.args(); + std::ostringstream o; + + std::vector extents(args.size()); + for (size_t dim = 0; dim < args.size(); dim++) { + extents[dim] = Variable::make(Int(32), param_name + ".extent." + std::to_string(dim), *this); + } + for (size_t dim = 0; dim < args.size(); dim++) { + for (const Bound &b : schedule.bounds()) { + if (b.var == args[dim]) { + Expr min = Variable::make(Int(32), param_name + ".min." + std::to_string(dim), *this); + if (b.min.defined()) { + min = b.min; + } + if (b.extent.defined()) { + extents[dim] = b.extent; + } + if (b.modulus.defined()) { + Expr max_plus_one = min + extents[dim]; + min = (min / b.modulus) * b.modulus; + max_plus_one = (max_plus_one / b.modulus) * b.modulus; + // simplify() mainly to strip out constant-zero remainders and the like + extents[dim] = simplify(max_plus_one - min); + min = simplify(min + b.remainder); + } + // TODO: these warnings should be upgraded into errors. + if (min_constraint(dim).defined() && !equal(min, simplify(min_constraint(dim)))) { + user_error << "Inferred value for parameter \"" << f.name() << "\" min[" << dim << "] does not match" + " value explicitly specified; using the explicit value, but you should revise the schedule to" + " avoid this warning. (inferred " << min << " vs explicit " << min_constraint(dim) << ")\n"; + } else { + if (debug::debug_level() >= D) { + o << " min." << dim << " -> " << min << "\n"; + } + set_min_constraint(dim, min); + } + if (extent_constraint(dim).defined() && !equal(extents[dim], simplify(extent_constraint(dim)))) { + user_error << "Inferred value for parameter \"" << f.name() << "\" extent[" << dim << "] does not match" + " value explicitly specified; using the explicit value, but you should revise the schedule to" + " avoid this warning. (inferred " << extents[dim] << " vs explicit " << extent_constraint(dim) << ")\n"; + } else { + if (debug::debug_level() >= D) { + o << " extents." << dim << " -> " << extents[dim] << "\n"; + } + set_extent_constraint(dim, extents[dim]); + } + break; + } + } + } + + // stride[0] defaults to 1, not Expr(). + const auto is_default = [](int dim, const Expr &e) -> bool { + return (dim == 0) ? is_one(e) : !e.defined(); + }; + Expr stride = 1; + for (const StorageDim &storage_dim : storage_dims) { + for (size_t dim = 0; dim < args.size(); dim++) { + if (args[dim] == storage_dim.var) { + if (!is_default(dim, stride)) { + if (storage_dim.alignment.defined()) { + stride = (stride / storage_dim.alignment) / storage_dim.alignment; + } + Expr s = stride_constraint(dim); + if (!is_default(dim, s) && !equal(stride, simplify(s))) { + user_error << "Inferred value for parameter \"" << f.name() << "\" stride[" << dim << "] does not match" + " value explicitly specified; using the explicit value, but you should revise the schedule to" + " avoid this warning. (inferred " << stride << " vs explicit " << s << ")\n"; + } else { + set_stride_constraint(dim, stride); + if (debug::debug_level() >= D) { + o << " stride." << dim << " -> " << stride << " (was " << s << ")\n"; + } + } + } + Expr extent = extents[dim]; + if (stride.defined() && is_const(extent)) { + stride = simplify(stride * extent); + } else { + stride = Expr(); + } + break; + } + } + } + + if (!o.str().empty()) { + debug(D) << "set_constraints_from_schedule(" << f.name() << "):\n" << o.str(); + } +} + void check_call_arg_types(const std::string &name, std::vector *args, int dims) { user_assert(args->size() == (size_t)dims) << args->size() << "-argument call to \"" diff --git a/src/Parameter.h b/src/Parameter.h index f307e9376275..bf803b8cab8d 100644 --- a/src/Parameter.h +++ b/src/Parameter.h @@ -14,6 +14,7 @@ class OutputImageParam; namespace Internal { +class Function; struct ParameterContents; /** A reference-counted handle to a parameter to a halide @@ -42,9 +43,7 @@ class Parameter { * the third argument. If the second argument is true, this is a * buffer parameter, otherwise, it is a scalar parameter. The * third argument gives the dimensionality of the buffer - * parameter. It should be zero for scalar parameters. If the - * fifth argument is true, the the name being passed in was - * explicitly specified (as opposed to autogenerated). */ + * parameter. It should be zero for scalar parameters. */ Parameter(Type t, bool is_buffer, int dimensions, const std::string &name); virtual ~Parameter() = default; @@ -156,6 +155,8 @@ class Parameter { bool operator<(const Parameter &other) const { return contents < other.contents; } + + void set_constraints_from_schedule(Function f); }; /** Validate arguments to a call to a func, image or imageparam. */ diff --git a/src/Schedule.cpp b/src/Schedule.cpp index ab4b29f0b3db..5b3df4a8f805 100644 --- a/src/Schedule.cpp +++ b/src/Schedule.cpp @@ -213,11 +213,11 @@ struct FuncScheduleContents { std::vector estimates; std::map wrappers; MemoryType memory_type; - bool memoized, async; + bool memoized, async, emit_inliner_warnings; FuncScheduleContents() : store_level(LoopLevel::inlined()), compute_level(LoopLevel::inlined()), - memory_type(MemoryType::Auto), memoized(false), async(false) {}; + memory_type(MemoryType::Auto), memoized(false), async(false), emit_inliner_warnings(true) {}; // Pass an IRMutator2 through to all Exprs referenced in the FuncScheduleContents void mutate(IRMutator2 *mutator) { @@ -329,6 +329,7 @@ FuncSchedule FuncSchedule::deep_copy( copy.contents->memory_type = contents->memory_type; copy.contents->memoized = contents->memoized; copy.contents->async = contents->async; + copy.contents->emit_inliner_warnings = contents->emit_inliner_warnings; // Deep-copy wrapper functions. for (const auto &iter : contents->wrappers) { @@ -356,6 +357,14 @@ bool FuncSchedule::memoized() const { return contents->memoized; } +bool &FuncSchedule::emit_inliner_warnings() { + return contents->emit_inliner_warnings; +} + +bool FuncSchedule::emit_inliner_warnings() const { + return contents->emit_inliner_warnings; +} + bool &FuncSchedule::async() { return contents->async; } diff --git a/src/Schedule.h b/src/Schedule.h index c7a088822a57..ecb9e0d028e8 100644 --- a/src/Schedule.h +++ b/src/Schedule.h @@ -458,6 +458,13 @@ class FuncSchedule { /** Pass an IRMutator2 through to all Exprs referenced in the * Schedule. */ void mutate(IRMutator2 *); + + /** If emit_inliner_warnings is false, warnings about meaningless-when-inlined + * schedule issues should be suppressed. */ + // @{ + bool emit_inliner_warnings() const; + bool &emit_inliner_warnings(); + // @} }; diff --git a/test/correctness/constraints.cpp b/test/correctness/constraints.cpp index 3879a08cdf68..52b6d7555fb0 100644 --- a/test/correctness/constraints.cpp +++ b/test/correctness/constraints.cpp @@ -12,67 +12,130 @@ void my_error_handler(void *user_context, const char *msg) { } int main(int argc, char **argv) { - Func f, g; Var x, y; - ImageParam param(Int(32), 2); - Buffer image1(128, 73); - Buffer image2(144, 23); + Buffer image_128x73(128, 73); + Buffer image_144x72(144, 72); - f(x, y) = param(x, y)*2; + // Test setting input constraints via ImageParam.dim().set_xxx() + ImageParam param1(Int(32), 2); + param1.dim(0).set_bounds(0, 128); - param.dim(0).set_bounds(0, 128); - - f.set_error_handler(my_error_handler); + Func f1; + f1(x, y) = param1(x, y)*2; + f1.set_error_handler(my_error_handler); // This should be fine - param.set(image1); + param1.set(image_128x73); error_occurred = false; - f.realize(20, 20); + f1.realize(20, 20); + if (error_occurred) { + printf("Error incorrectly raised when constraining input buffer %d\n", __LINE__); + return -1; + } + + // This should be an error, because dimension 0 of image 2 is not from 0 to 128 like we promised + param1.set(image_144x72); + error_occurred = false; + f1.realize(20, 20); + if (!error_occurred) { + printf("Error incorrectly not raised when constraining input buffer %d\n", __LINE__); + return -1; + } + + // Test setting input constraints via scheduling language + ImageParam param2(Int(32), 2); + param2.bound(Halide::_0, 0, 128); + + Func f2; + f2(x, y) = param2(x, y)*2; + f2.set_error_handler(my_error_handler); + // This should be fine + param2.set(image_128x73); + error_occurred = false; + f2.realize(20, 20); if (error_occurred) { - printf("Error incorrectly raised\n"); + printf("Error incorrectly raised when constraining input buffer %d\n", __LINE__); return -1; } + // This should be an error, because dimension 0 of image 2 is not from 0 to 128 like we promised - param.set(image2); + param2.set(image_144x72); error_occurred = false; - f.realize(20, 20); + f2.realize(20, 20); + if (!error_occurred) { + printf("Error incorrectly not raised when constraining input buffer %d\n", __LINE__); + return -1; + } + // Test setting constraints via output_buffer().dim().set_xxx() + Func h1; + h1(x, y) = x*y; + h1.set_error_handler(my_error_handler); + h1.output_buffer().dim(0).set_bounds(0, ((h1.output_buffer().dim(0).extent())/64)*64); + error_occurred = false; + h1.realize(image_128x73); + if (error_occurred) { + printf("Error incorrectly raised when constraining output buffer %d\n", __LINE__); + return -1; + } + error_occurred = false; + h1.realize(image_144x72); if (!error_occurred) { - printf("Error incorrectly not raised\n"); + printf("Error incorrectly not raised when constraining output buffer %d\n", __LINE__); return -1; } - // Now try constraining the output buffer of a function - g(x, y) = x*y; - g.set_error_handler(my_error_handler); - g.output_buffer().dim(0).set_stride(2); + // Test setting constraints via scheduling language + Func h2; + h2(x, y) = x*y; + h2.set_error_handler(my_error_handler); + h2.align_bounds(x, 64); + error_occurred = false; + h2.realize(image_128x73); + if (error_occurred) { + printf("Error incorrectly raised when constraining output buffer %d\n", __LINE__); + return -1; + } error_occurred = false; - g.realize(image1); + h2.realize(image_144x72); if (!error_occurred) { - printf("Error incorrectly not raised when constraining output buffer\n"); + printf("Error incorrectly not raised when constraining output buffer %d\n", __LINE__); return -1; } - Func h; - h(x, y) = x*y; - h.set_error_handler(my_error_handler); - h.output_buffer() - .dim(0) - .set_stride(1) - .set_bounds(0, ((h.output_buffer().dim(0).extent())/8)*8) - .dim(1) - .set_bounds(0, image1.dim(1).extent()); + // Test setting constraints via output_buffer() *and* scheduling language() + // (in this case, explict values must match inferred values) + Func h3; + h3(x, y) = x*y; + h3.set_error_handler(my_error_handler); + // TODO: no way to do align_bounds() on just extent, but constrain min to (say) zero, + // hence this complex expression to match what align_bounds() does + Expr aligned_min = h3.output_buffer().dim(0).min() / 64 * 64; + Expr aligned_max = (h3.output_buffer().dim(0).min() + h3.output_buffer().dim(0).extent()) / 64 * 64; + h3.output_buffer().dim(0).set_bounds(aligned_min, aligned_max - aligned_min); + h3.align_bounds(x, 64); error_occurred = false; - h.realize(image1); + h3.realize(image_128x73); + if (error_occurred) { + printf("Error incorrectly raised when constraining output buffer %d\n", __LINE__); + return -1; + } + error_occurred = false; + h3.realize(image_144x72); + if (!error_occurred) { + printf("Error incorrectly not raised when constraining output buffer %d\n", __LINE__); + return -1; + } std::string assembly_file = Internal::get_test_tmp_dir() + "h.s"; Internal::ensure_no_file_exists(assembly_file); // Also check it compiles ok without an inferred argument list - h.compile_to_assembly(assembly_file, {image1}, "h"); + error_occurred = false; + h1.compile_to_assembly(assembly_file, {image_128x73}, "h"); if (error_occurred) { - printf("Error incorrectly raised when constraining output buffer\n"); + printf("Error incorrectly raised when constraining output buffer %d\n", __LINE__); return -1; } diff --git a/test/correctness/interleave.cpp b/test/correctness/interleave.cpp index 1b71fcaf6ee3..1a0394284070 100644 --- a/test/correctness/interleave.cpp +++ b/test/correctness/interleave.cpp @@ -134,20 +134,12 @@ int main(int argc, char **argv) { interleaved .reorder(y, x) + .reorder_storage(y, x) .bound(y, 0, 3) .bound(x, 0, 16) .fuse(y, x, xy) .vectorize(xy, 12); - interleaved - .output_buffer() - .dim(0) - .set_stride(3) - .dim(1) - .set_min(0) - .set_stride(1) - .set_extent(3); - Buffer buff3(3, 16); buff3.transpose(0, 1); @@ -184,18 +176,11 @@ int main(int argc, char **argv) { output4 .reorder(y, x) + .reorder_storage(y, x) .bound(y, 0, 4) .unroll(y) .vectorize(x, 4); - output4.output_buffer() - .dim(0) - .set_stride(4) - .dim(1) - .set_min(0) - .set_stride(1) - .set_extent(4); - check_interleave_count(output4, 1); Buffer buff4(4, 16); @@ -224,19 +209,11 @@ int main(int argc, char **argv) { output5 .reorder(y, x) + .reorder_storage(y, x) .bound(y, 0, 5) .unroll(y) .vectorize(x, 4); - output5.output_buffer() - .dim(0) - .set_stride(5) - .dim(1) - .set_min(0) - .set_stride(1) - .set_extent(5); - - check_interleave_count(output5, 1); Buffer buff5(5, 16); @@ -367,26 +344,6 @@ int main(int argc, char **argv) { .unroll(x) .vectorize(y); - trans1.output_buffer() - .dim(0) - .set_min(0) - .set_stride(1) - .set_extent(8) - .dim(1) - .set_min(0) - .set_stride(8) - .set_extent(8); - - trans2.output_buffer() - .dim(0) - .set_min(0) - .set_stride(1) - .set_extent(8) - .dim(1) - .set_min(0) - .set_stride(8) - .set_extent(8); - Buffer result6(8, 8); Buffer result7(8, 8); trans1.realize(result6); diff --git a/test/correctness/interleave_rgb.cpp b/test/correctness/interleave_rgb.cpp index c9285f230bdc..b181ffe11bd8 100644 --- a/test/correctness/interleave_rgb.cpp +++ b/test/correctness/interleave_rgb.cpp @@ -15,10 +15,7 @@ bool test_interleave() { Target target = get_jit_target_from_environment(); input.compute_root(); - interleaved.reorder(c, x, y).bound(c, 0, 3); - interleaved.output_buffer() - .dim(0).set_stride(3) - .dim(2).set_stride(1).set_extent(3); + interleaved.reorder(c, x, y).reorder_storage(c, x, y).bound(c, 0, 3); if (target.has_gpu_feature()) { Var xi("xi"), yi("yi"); diff --git a/test/correctness/storage_folding.cpp b/test/correctness/storage_folding.cpp index 3640f664b0fc..56c660b03acc 100644 --- a/test/correctness/storage_folding.cpp +++ b/test/correctness/storage_folding.cpp @@ -372,17 +372,20 @@ int main(int argc, char **argv) { g(x, y, c) = f(x-1, y+1, c) + f(x, y-1, c); f.store_root().compute_at(g, y).fold_storage(y, 3); + Buffer im; if (interleave) { f.reorder(c, x, y).reorder_storage(c, x, y); g.reorder(c, x, y).reorder_storage(c, x, y); + im = Buffer::make_interleaved(100, 1000, 3); + } else { + im = Buffer(100, 1000, 3); } // Make sure we can explicitly fold something with an outer // loop. g.set_custom_allocator(my_malloc, my_free); - - Buffer im = g.realize(100, 1000, 3); + g.realize(im); size_t expected_size; if (interleave) { diff --git a/test/generator/error_codes_aottest.cpp b/test/generator/error_codes_aottest.cpp index 5c7118817ed1..642668b6141e 100644 --- a/test/generator/error_codes_aottest.cpp +++ b/test/generator/error_codes_aottest.cpp @@ -43,7 +43,7 @@ int main(int argc, char **argv) { // Passing 50 as the second arg violates the call to Func::bound // in the generator result = error_codes(&in, 50, &out); - correct = halide_error_code_explicit_bounds_too_small; + correct = halide_error_code_constraint_violated; check(result, correct); // Would read out of bounds on the input @@ -103,7 +103,7 @@ int main(int argc, char **argv) { // The second argument is supposed to be between 0 and 64. result = error_codes(&in, -23, &out); - correct = halide_error_code_param_too_small; + correct = halide_error_code_constraint_violated; check(result, correct); shape[0].extent = 108; diff --git a/test/generator/nested_externs_generator.cpp b/test/generator/nested_externs_generator.cpp index f248c7ad8997..761bb5426e7e 100644 --- a/test/generator/nested_externs_generator.cpp +++ b/test/generator/nested_externs_generator.cpp @@ -92,7 +92,7 @@ class NestedExternsRoot : public Generator { f.compute_at(root, y).reorder_storage(args[2], args[0], args[1]); } set_interleaved(root); - root.reorder_storage(c, x, y); + root.reorder_storage(c, x, y).bound(c, 0, 3); } private: diff --git a/test/generator/stubtest_aottest.cpp b/test/generator/stubtest_aottest.cpp index b333a1615449..aac629b8cdf4 100644 --- a/test/generator/stubtest_aottest.cpp +++ b/test/generator/stubtest_aottest.cpp @@ -9,11 +9,11 @@ using Halide::Runtime::Buffer; const int kSize = 32; template -Buffer make_image(int extra) { - Buffer im(kSize, kSize, 3); +Buffer make_image(int extra, int channels = 3) { + Buffer im(kSize, kSize, channels); for (int x = 0; x < kSize; x++) { for (int y = 0; y < kSize; y++) { - for (int c = 0; c < 3; c++) { + for (int c = 0; c < channels; c++) { im(x, y, c) = static_cast(x + y + c + extra); } } @@ -44,17 +44,17 @@ void verify(const Buffer &input, float float_arg, int int_arg, const } int main(int argc, char **argv) { - Buffer buffer_input = make_image(0); - Buffer simple_input = make_image(0); - Buffer array_input0 = make_image(0); - Buffer array_input1 = make_image(1); + Buffer buffer_input = make_image(0, 5); + Buffer simple_input = make_image(0, 7); + Buffer array_input0 = make_image(0, 9); + Buffer array_input1 = make_image(1, 11); Buffer typed_buffer_output(kSize, kSize, 3); Buffer untyped_buffer_output(kSize, kSize, 3); Buffer tupled_output0(kSize, kSize, 3); Buffer tupled_output1(kSize, kSize, 3); Buffer array_buffer_input0 = make_image(0); Buffer array_buffer_input1 = make_image(1); - Buffer simple_output(kSize, kSize, 3); + Buffer simple_output(kSize, kSize, 5); Buffer tuple_output0(kSize, kSize, 3), tuple_output1(kSize, kSize, 3); Buffer array_output0(kSize, kSize), array_output1(kSize, kSize); Buffer static_compiled_buffer_output(kSize, kSize, 3); diff --git a/test/generator/stubtest_generator.cpp b/test/generator/stubtest_generator.cpp index 4edca7879b7d..6db9fda8bac3 100644 --- a/test/generator/stubtest_generator.cpp +++ b/test/generator/stubtest_generator.cpp @@ -88,6 +88,29 @@ class StubTest : public Halide::Generator { void schedule() { intermediate.compute_at(intermediate_level); intermediate.specialize(vectorize).vectorize(x, natural_vector_size()); + + typed_buffer_input.bound(Halide::_2, 0, 5); + + // When AOT-compiling, set constraints on the Buffer for this input: + // - require that we have 7 'channels' + // - require that the width is an even multiple of 32 + simple_input.bound(Halide::_2, 0, 7).align_bounds(Halide::_0, 32); + + // Ditto for array inputs + for (size_t i = 0; i < array_input.size(); ++i) { + array_input[i].bound(Halide::_2, 0, (int)(9 + i * 2)).align_bounds(Halide::_0, 32); + } + + // Also set some constraints on Output + // - require that we have 5 'channels' + simple_output.bound(c, 0, 5); + // - require that the width is an even multiple of 32 + simple_output.align_bounds(x, 32); + + // Ditto for array outputs + for (size_t i = 0; i < array_output.size(); ++i) { + array_output[i].align_bounds(x, 32); + } } private: diff --git a/test/generator/stubtest_jittest.cpp b/test/generator/stubtest_jittest.cpp index 34e7805838d4..970860366fd5 100644 --- a/test/generator/stubtest_jittest.cpp +++ b/test/generator/stubtest_jittest.cpp @@ -11,11 +11,11 @@ const int kSize = 32; Var x, y, c; template -Buffer make_image(int extra) { - Buffer im(kSize, kSize, 3); +Buffer make_image(const std::string &name, int channels, int extra = 0) { + Buffer im(kSize, kSize, channels, name); for (int x = 0; x < kSize; x++) { for (int y = 0; y < kSize; y++) { - for (int c = 0; c < 3; c++) { + for (int c = 0; c < channels; c++) { im(x, y, c) = static_cast(x + y + c + extra); } } @@ -48,11 +48,11 @@ void verify(const Buffer &input, float float_arg, int int_arg, const int main(int argc, char **argv) { constexpr int kArrayCount = 2; - Buffer buffer_input = make_image(0); - Buffer simple_input = make_image(0); + Buffer buffer_input = make_image("buffer_inputz", 3); + Buffer simple_input = make_image("simple_inputz", 5); Buffer array_input[kArrayCount] = { - make_image(0), - make_image(1) + make_image("array_input_0z", 3, 0), + make_image("array_input_1z", 3, 1) }; std::vector int_args = { 33, 66 }; @@ -67,19 +67,19 @@ int main(int argc, char **argv) { GeneratorContext(get_jit_target_from_environment()), // Use aggregate-initialization syntax to fill in an Inputs struct. { - buffer_input, // typed_buffer_input - buffer_input, // untyped_buffer_input - { buffer_input, buffer_input }, - Func(simple_input), - { Func(array_input[0]), Func(array_input[1]) }, - 1.25f, - int_args_expr + buffer_input, // typed_buffer_input + buffer_input, // untyped_buffer_input + { buffer_input, buffer_input }, // array_buffer_input + Func(simple_input), // simple_input + { Func(array_input[0]), Func(array_input[1]) }, // array_input + 1.25f, // float_arg + int_args_expr // int_arg }, gp); gp.intermediate_level.set(LoopLevel(gen.tuple_output, gen.tuple_output.args().at(1))); - Realization simple_output_realized = gen.simple_output.realize(kSize, kSize, 3); + Realization simple_output_realized = gen.simple_output.realize(kSize, kSize, 5); Buffer s0 = simple_output_realized; verify(array_input[0], 1.f, 0, s0); From 07c326904685cb3ddb4ca0a9be969443ff4bbc20 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Wed, 7 Nov 2018 10:25:26 -0800 Subject: [PATCH 2/8] Fix correctness_rfactor --- test/correctness/rfactor.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/correctness/rfactor.cpp b/test/correctness/rfactor.cpp index 443b850d0fe1..75c3024fa16f 100644 --- a/test/correctness/rfactor.cpp +++ b/test/correctness/rfactor.cpp @@ -45,7 +45,10 @@ int simple_rfactor_test(bool compile_module) { return -1; } } else { - Buffer im = g.realize(80, 80); + Buffer im(80, 80); + // Since we reordered the storage we must also reorder the output buffer + im.transpose(0, 1); + g.realize(im); auto func = [](int x, int y, int z) { return (10 <= x && x <= 29) && (30 <= y && y <= 69) ? std::max(40 + x + y, 40) : 40; }; From 9cc7a97b919fdcc6937aaa3a902656ebaac1ccf3 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Wed, 7 Nov 2018 15:16:39 -0800 Subject: [PATCH 3/8] Fix correctness_sliding_window --- test/correctness/sliding_window.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/correctness/sliding_window.cpp b/test/correctness/sliding_window.cpp index dc27be84ab00..bc446e47b215 100644 --- a/test/correctness/sliding_window.cpp +++ b/test/correctness/sliding_window.cpp @@ -82,7 +82,9 @@ int main(int argc, char **argv) { h.reorder(c, x).reorder_storage(c, x).bound(c, 0, 4).vectorize(c); - Buffer im = h.realize(100, 4); + Buffer im(4, 100); + im.transpose(0, 1); + h.realize(im); if (count != 404) { printf("f was called %d times instead of %d times\n", count, 404); return -1; From db13ff643c9870d33bb44e31bce1bd0cb5be1ea3 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Wed, 7 Nov 2018 15:30:15 -0800 Subject: [PATCH 4/8] Revise error messages --- src/Parameter.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Parameter.cpp b/src/Parameter.cpp index 87470e99e960..3c3460bebdb5 100644 --- a/src/Parameter.cpp +++ b/src/Parameter.cpp @@ -315,11 +315,9 @@ void Parameter::set_constraints_from_schedule(Function f) { extents[dim] = simplify(max_plus_one - min); min = simplify(min + b.remainder); } - // TODO: these warnings should be upgraded into errors. if (min_constraint(dim).defined() && !equal(min, simplify(min_constraint(dim)))) { user_error << "Inferred value for parameter \"" << f.name() << "\" min[" << dim << "] does not match" - " value explicitly specified; using the explicit value, but you should revise the schedule to" - " avoid this warning. (inferred " << min << " vs explicit " << min_constraint(dim) << ")\n"; + " value explicitly specified (inferred " << min << " vs explicit " << min_constraint(dim) << ").\n"; } else { if (debug::debug_level() >= D) { o << " min." << dim << " -> " << min << "\n"; @@ -328,8 +326,7 @@ void Parameter::set_constraints_from_schedule(Function f) { } if (extent_constraint(dim).defined() && !equal(extents[dim], simplify(extent_constraint(dim)))) { user_error << "Inferred value for parameter \"" << f.name() << "\" extent[" << dim << "] does not match" - " value explicitly specified; using the explicit value, but you should revise the schedule to" - " avoid this warning. (inferred " << extents[dim] << " vs explicit " << extent_constraint(dim) << ")\n"; + " value explicitly specified (inferred " << extents[dim] << " vs explicit " << extent_constraint(dim) << ").\n"; } else { if (debug::debug_level() >= D) { o << " extents." << dim << " -> " << extents[dim] << "\n"; @@ -356,8 +353,7 @@ void Parameter::set_constraints_from_schedule(Function f) { Expr s = stride_constraint(dim); if (!is_default(dim, s) && !equal(stride, simplify(s))) { user_error << "Inferred value for parameter \"" << f.name() << "\" stride[" << dim << "] does not match" - " value explicitly specified; using the explicit value, but you should revise the schedule to" - " avoid this warning. (inferred " << stride << " vs explicit " << s << ")\n"; + " value explicitly specified (inferred " << stride << " vs explicit " << s << ").\n"; } else { set_stride_constraint(dim, stride); if (debug::debug_level() >= D) { From 681e5506d2d3360b0c44a8d3542da74a4e85937b Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Thu, 8 Nov 2018 11:14:50 -0800 Subject: [PATCH 5/8] patch in 3423 --- python_bindings/correctness/buffer.py | 46 +++++++++++++++++ python_bindings/src/PyBuffer.cpp | 19 +++++-- src/Buffer.h | 12 +++++ src/runtime/HalideBuffer.h | 71 +++++++++++++++++++++------ test/correctness/halide_buffer.cpp | 44 +++++++++++++++++ 5 files changed, 172 insertions(+), 20 deletions(-) diff --git a/python_bindings/correctness/buffer.py b/python_bindings/correctness/buffer.py index 42feb856ba22..7b9fdd8cbdcd 100644 --- a/python_bindings/correctness/buffer.py +++ b/python_bindings/correctness/buffer.py @@ -173,6 +173,51 @@ def test_interleaved_ndarray(): assert b.dim(2).extent() == c assert b.dim(2).stride() == 1 +def test_reorder(): + W = 7 + H = 5 + C = 3 + Z = 2 + + a = hl.Buffer(type = hl.UInt(8), sizes = [W, H, C], storage_order = [2, 0, 1]) + assert a.dim(0).extent() == W + assert a.dim(1).extent() == H + assert a.dim(2).extent() == C + assert a.dim(2).stride() == 1 + assert a.dim(0).stride() == C + assert a.dim(1).stride() == W * C + + b = hl.Buffer(hl.UInt(8), [W, H, C, Z], [2, 3, 0, 1]) + assert b.dim(0).extent() == W + assert b.dim(1).extent() == H + assert b.dim(2).extent() == C + assert b.dim(3).extent() == Z + assert b.dim(2).stride() == 1 + assert b.dim(3).stride() == C + assert b.dim(0).stride() == C * Z + assert b.dim(1).stride() == W * C * Z + + b2 = hl.Buffer(hl.UInt(8), [C, Z, W, H]) + assert b.dim(0).extent() == b2.dim(2).extent() + assert b.dim(1).extent() == b2.dim(3).extent() + assert b.dim(2).extent() == b2.dim(0).extent() + assert b.dim(3).extent() == b2.dim(1).extent() + assert b.dim(0).stride() == b2.dim(2).stride() + assert b.dim(1).stride() == b2.dim(3).stride() + assert b.dim(2).stride() == b2.dim(0).stride() + assert b.dim(3).stride() == b2.dim(1).stride() + + b2.transpose([2, 3, 0, 1]) + assert b.dim(0).extent() == b2.dim(0).extent() + assert b.dim(1).extent() == b2.dim(1).extent() + assert b.dim(2).extent() == b2.dim(2).extent() + assert b.dim(3).extent() == b2.dim(3).extent() + assert b.dim(0).stride() == b2.dim(0).stride() + assert b.dim(1).stride() == b2.dim(1).stride() + assert b.dim(2).stride() == b2.dim(2).stride() + assert b.dim(3).stride() == b2.dim(3).stride() + + if __name__ == "__main__": test_make_interleaved() test_interleaved_ndarray() @@ -182,4 +227,5 @@ def test_interleaved_ndarray(): test_fill_all_equal() test_bufferinfo_sharing() test_float16() + test_reorder() diff --git a/python_bindings/src/PyBuffer.cpp b/python_bindings/src/PyBuffer.cpp index 3f7f315d420b..0a3d0c3f41a4 100644 --- a/python_bindings/src/PyBuffer.cpp +++ b/python_bindings/src/PyBuffer.cpp @@ -304,6 +304,10 @@ void define_buffer(py::module &m) { return Buffer<>(type, sizes, name); }), py::arg("type"), py::arg("sizes"), py::arg("name") = "") + .def(py::init([](Type type, const std::vector &sizes, const std::vector &storage_order, const std::string &name) -> Buffer<> { + return Buffer<>(type, sizes, storage_order, name); + }), py::arg("type"), py::arg("sizes"), py::arg("storage_order"), py::arg("name") = "") + // Note that this exists solely to allow you to create a Buffer with a null host ptr; // this is necessary for some bounds-query operations (e.g. Func::infer_input_bounds). .def_static("make_bounds_query", [](Type type, const std::vector &sizes, const std::string &name) -> Buffer<> { @@ -419,10 +423,17 @@ void define_buffer(py::module &m) { b.transpose(d1, d2); }, py::arg("d1"), py::arg("d2")) - // Present in Runtime::Buffer but not Buffer - // .def("transposed", [](Buffer<> &b, int d1, int d2) -> Buffer<> { - // return b.transposed(d1, d2); - // }, py::arg("d1"), py::arg("d2")) + .def("transposed", [](Buffer<> &b, int d1, int d2) -> Buffer<> { + return b.transposed(d1, d2); + }, py::arg("d1"), py::arg("d2")) + + .def("transpose", [](Buffer<> &b, const std::vector &order) -> void { + b.transpose(order); + }, py::arg("order")) + + .def("transposed", [](Buffer<> &b, const std::vector &order) -> Buffer<> { + return b.transposed(order); + }, py::arg("order")) .def("dim", [](Buffer<> &b, int dimension) -> BufferDimension { return b.dim(dimension); diff --git a/src/Buffer.h b/src/Buffer.h index 000e75921990..197bdc0a63aa 100644 --- a/src/Buffer.h +++ b/src/Buffer.h @@ -185,10 +185,21 @@ class Buffer { const std::string &name = "") : Buffer(Runtime::Buffer(t, sizes), name) {} + explicit Buffer(Type t, + const std::vector &sizes, + const std::vector &storage_order, + const std::string &name = "") : + Buffer(Runtime::Buffer(t, sizes, storage_order), name) {} + explicit Buffer(const std::vector &sizes, const std::string &name = "") : Buffer(Runtime::Buffer(sizes), name) {} + explicit Buffer(const std::vector &sizes, + const std::vector &storage_order, + const std::string &name = "") : + Buffer(Runtime::Buffer(sizes, storage_order), name) {} + template explicit Buffer(Array (&vals)[N], const std::string &name = "") : @@ -395,6 +406,7 @@ class Buffer { HALIDE_BUFFER_FORWARD(translate) HALIDE_BUFFER_FORWARD_INITIALIZER_LIST(translate, std::vector) HALIDE_BUFFER_FORWARD(transpose) + HALIDE_BUFFER_FORWARD(transposed) HALIDE_BUFFER_FORWARD(add_dimension) HALIDE_BUFFER_FORWARD(copy_to_host) HALIDE_BUFFER_FORWARD(copy_to_device) diff --git a/src/runtime/HalideBuffer.h b/src/runtime/HalideBuffer.h index 6733e7715722..b17d60dff336 100644 --- a/src/runtime/HalideBuffer.h +++ b/src/runtime/HalideBuffer.h @@ -903,17 +903,32 @@ class Buffer { } /** Allocate a new image of known type using a vector of ints as the size. */ - Buffer(const std::vector &sizes) { - buf.type = static_halide_type(); - buf.dimensions = (int)sizes.size(); - make_shape_storage(); - initialize_shape(sizes); - if (!any_zero(sizes)) { - check_overflow(); - allocate(); + explicit Buffer(const std::vector &sizes) : Buffer(halide_type_of(), sizes) {} + +private: + // Create a copy of the sizes vector, ordered as specified by order. + static std::vector make_ordered_sizes(const std::vector &sizes, const std::vector &order) { + assert(order.size() == sizes.size()); + std::vector ordered_sizes(sizes.size()); + for (size_t i = 0; i < sizes.size(); ++i) { + ordered_sizes[i] = sizes.at(order[i]); } + return ordered_sizes; + } + +public: + /** Allocate a new image of unknown type using a vector of ints as the size and + * a vector of indices indicating the storage order for each dimension. The + * length of the sizes vector and the storage-order vector must match. For instance, + * to allocate an interleaved RGB buffer, you would pass {2, 0, 1} for storage_order. */ + Buffer(halide_type_t t, const std::vector &sizes, const std::vector &storage_order) + : Buffer(t, make_ordered_sizes(sizes, storage_order)) { + transpose(storage_order); } + Buffer(const std::vector &sizes, const std::vector &storage_order) + : Buffer(halide_type_of(), sizes, storage_order) {} + /** Make an Buffer that refers to a statically sized array. Does not * take ownership of the data, and does not set the host_dirty flag. */ template @@ -1381,6 +1396,34 @@ class Buffer { std::swap(buf.dim[d1], buf.dim[d2]); } + /** A generalized transpose: instead of swapping two dimensions, + * pass a vector that lists each dimension index exactly once, in the desired order. + * For instance, to transpose a 3-dimensional planar image to be interleaved, + * pass {2, 0, 1} for order */ + void transpose(const std::vector &order) { + assert((int) order.size() == dimensions()); + if (dimensions() < 2) { + // My, that was easy + return; + } + + std::vector order_sorted = order; + for (size_t i = 1; i < order_sorted.size(); i++) { + for (size_t j = i; j > 0 && order_sorted[j-1] > order_sorted[j]; j--) { + std::swap(order_sorted[j], order_sorted[j-1]); + transpose(j, j-1); + } + } + } + + /** Make an image which refers to the same data using a different + * ordering of the dimensions. */ + Buffer transposed(const std::vector &order) const { + Buffer im = *this; + im.transpose(order); + return im; + } + /** Make a lower-dimensional image that refers to one slice of this image. */ Buffer sliced(int d, int pos) const { Buffer im = *this; @@ -1642,6 +1685,8 @@ class Buffer { * known as packed or chunky) memory layouts. */ static Buffer make_interleaved(halide_type_t t, int width, int height, int channels) { Buffer im(t, channels, width, height); + // Note that this is equivalent to calling transpose({2, 0, 1}), + // but slightly more efficient. im.transpose(0, 1); im.transpose(1, 2); return im; @@ -1654,10 +1699,7 @@ class Buffer { * generator has been compiled with support for interleaved (also * known as packed or chunky) memory layouts. */ static Buffer make_interleaved(int width, int height, int channels) { - Buffer im(channels, width, height); - im.transpose(0, 1); - im.transpose(1, 2); - return im; + return make_interleaved(halide_type_of(), width, height, channels); } /** Wrap an existing interleaved image. */ @@ -1671,10 +1713,7 @@ class Buffer { /** Wrap an existing interleaved image. */ static Buffer make_interleaved(T *data, int width, int height, int channels) { - Buffer im(data, channels, width, height); - im.transpose(0, 1); - im.transpose(1, 2); - return im; + return make_interleaved(halide_type_of(), data, width, height, channels); } /** Make a zero-dimensional Buffer */ diff --git a/test/correctness/halide_buffer.cpp b/test/correctness/halide_buffer.cpp index 1e4501bc9d50..7d0d0659b1ba 100644 --- a/test/correctness/halide_buffer.cpp +++ b/test/correctness/halide_buffer.cpp @@ -1,3 +1,4 @@ +#include // Don't include Halide.h: it is not necessary for this test. #include "HalideBuffer.h" @@ -355,6 +356,49 @@ int main(int argc, char **argv) { assert(d.all_equal(4)); } + { + constexpr int W = 7, H = 5, C = 3, Z = 2; + + // test reorder() and the related ctors + auto a = Buffer({W, H, C}, {2, 0, 1}); + assert(a.dim(0).extent() == W); + assert(a.dim(1).extent() == H); + assert(a.dim(2).extent() == C); + assert(a.dim(2).stride() == 1); + assert(a.dim(0).stride() == C); + assert(a.dim(1).stride() == W * C); + + auto b = Buffer({W, H, C, Z}, {2, 3, 0, 1}); + assert(b.dim(0).extent() == W); + assert(b.dim(1).extent() == H); + assert(b.dim(2).extent() == C); + assert(b.dim(3).extent() == Z); + assert(b.dim(2).stride() == 1); + assert(b.dim(3).stride() == C); + assert(b.dim(0).stride() == C * Z); + assert(b.dim(1).stride() == W * C * Z); + + auto b2 = Buffer(C, Z, W, H); + assert(b.dim(0).extent() == b2.dim(2).extent()); + assert(b.dim(1).extent() == b2.dim(3).extent()); + assert(b.dim(2).extent() == b2.dim(0).extent()); + assert(b.dim(3).extent() == b2.dim(1).extent()); + assert(b.dim(0).stride() == b2.dim(2).stride()); + assert(b.dim(1).stride() == b2.dim(3).stride()); + assert(b.dim(2).stride() == b2.dim(0).stride()); + assert(b.dim(3).stride() == b2.dim(1).stride()); + + b2.transpose({2, 3, 0, 1}); + assert(b.dim(0).extent() == b2.dim(0).extent()); + assert(b.dim(1).extent() == b2.dim(1).extent()); + assert(b.dim(2).extent() == b2.dim(2).extent()); + assert(b.dim(3).extent() == b2.dim(3).extent()); + assert(b.dim(0).stride() == b2.dim(0).stride()); + assert(b.dim(1).stride() == b2.dim(1).stride()); + assert(b.dim(2).stride() == b2.dim(2).stride()); + assert(b.dim(3).stride() == b2.dim(3).stride()); + } + printf("Success!\n"); return 0; } From b9fb19bd2921c69d036d76a9299e35c308593496 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Thu, 8 Nov 2018 11:19:19 -0800 Subject: [PATCH 6/8] Pipeline --- src/Pipeline.cpp | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/Pipeline.cpp b/src/Pipeline.cpp index b0eff6ca9b9c..3817900e84ff 100644 --- a/src/Pipeline.cpp +++ b/src/Pipeline.cpp @@ -540,11 +540,31 @@ Realization Pipeline::realize(vector sizes, const Target &target, const ParamMap ¶m_map) { user_assert(defined()) << "Pipeline is undefined\n"; vector> bufs; - for (auto & out : contents->outputs) { - user_assert(out.has_pure_definition() || out.has_extern_definition()) << - "Can't realize Pipeline with undefined output Func: " << out.name() << ".\n"; - for (Type t : out.output_types()) { - bufs.emplace_back(t, sizes); + for (auto & f : contents->outputs) { + user_assert(f.has_pure_definition() || f.has_extern_definition()) << + "Can't realize Pipeline with undefined output Func: " << f.name() << ".\n"; + + // Attempt to create a Buffer that has the storage laid out in the + // same order as that specified by our schedule. + std::vector storage_order(sizes.size()); + { + // Should this be moved into (say) Function::get_storage_order()? + const FuncSchedule &schedule = f.schedule(); + const std::vector &storage_dims = schedule.storage_dims(); + const std::vector &args = f.args(); + + for (size_t s = 0; s < storage_dims.size(); ++s) { + for (size_t a = 0; a < args.size(); ++a) { + if (args[a] == storage_dims[s].var) { + storage_order[s] = a; + break; + } + } + } + } + + for (Type t : f.output_types()) { + bufs.emplace_back(t, sizes, storage_order); } } Realization r(bufs); From 1e164ede357ccba3c623aae7f9233702c21aa72c Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Thu, 8 Nov 2018 11:34:34 -0800 Subject: [PATCH 7/8] fixes --- test/correctness/rfactor.cpp | 5 +---- test/correctness/sliding_window.cpp | 4 +--- test/correctness/storage_folding.cpp | 7 ++----- test/opengl/copy_pixels.cpp | 3 +-- test/opengl/copy_to_device.cpp | 3 +-- test/opengl/float_texture.cpp | 3 +-- test/opengl/lut.cpp | 4 +--- test/opengl/produce.cpp | 3 +-- test/opengl/rewrap_texture.cpp | 10 ++++++---- test/opengl/save_state.cpp | 3 +-- test/opengl/set_pixels.cpp | 3 +-- test/opengl/shifted_domains.cpp | 7 ++++--- test/opengl/special_funcs.cpp | 8 ++++++-- test/opengl/tuples.cpp | 3 +-- test/opengl/varying.cpp | 4 +--- 15 files changed, 29 insertions(+), 41 deletions(-) diff --git a/test/correctness/rfactor.cpp b/test/correctness/rfactor.cpp index 75c3024fa16f..443b850d0fe1 100644 --- a/test/correctness/rfactor.cpp +++ b/test/correctness/rfactor.cpp @@ -45,10 +45,7 @@ int simple_rfactor_test(bool compile_module) { return -1; } } else { - Buffer im(80, 80); - // Since we reordered the storage we must also reorder the output buffer - im.transpose(0, 1); - g.realize(im); + Buffer im = g.realize(80, 80); auto func = [](int x, int y, int z) { return (10 <= x && x <= 29) && (30 <= y && y <= 69) ? std::max(40 + x + y, 40) : 40; }; diff --git a/test/correctness/sliding_window.cpp b/test/correctness/sliding_window.cpp index bc446e47b215..dc27be84ab00 100644 --- a/test/correctness/sliding_window.cpp +++ b/test/correctness/sliding_window.cpp @@ -82,9 +82,7 @@ int main(int argc, char **argv) { h.reorder(c, x).reorder_storage(c, x).bound(c, 0, 4).vectorize(c); - Buffer im(4, 100); - im.transpose(0, 1); - h.realize(im); + Buffer im = h.realize(100, 4); if (count != 404) { printf("f was called %d times instead of %d times\n", count, 404); return -1; diff --git a/test/correctness/storage_folding.cpp b/test/correctness/storage_folding.cpp index 56c660b03acc..de0c4b78ca08 100644 --- a/test/correctness/storage_folding.cpp +++ b/test/correctness/storage_folding.cpp @@ -372,20 +372,17 @@ int main(int argc, char **argv) { g(x, y, c) = f(x-1, y+1, c) + f(x, y-1, c); f.store_root().compute_at(g, y).fold_storage(y, 3); - Buffer im; + ; if (interleave) { f.reorder(c, x, y).reorder_storage(c, x, y); g.reorder(c, x, y).reorder_storage(c, x, y); - im = Buffer::make_interleaved(100, 1000, 3); - } else { - im = Buffer(100, 1000, 3); } // Make sure we can explicitly fold something with an outer // loop. g.set_custom_allocator(my_malloc, my_free); - g.realize(im); + Buffer im = g.realize(100, 1000, 3); size_t expected_size; if (interleave) { diff --git a/test/opengl/copy_pixels.cpp b/test/opengl/copy_pixels.cpp index 97cacecd32e1..41c77617a8f2 100644 --- a/test/opengl/copy_pixels.cpp +++ b/test/opengl/copy_pixels.cpp @@ -18,10 +18,9 @@ int main() { Func g; g(x, y, c) = input(x, y, c); - Buffer out(255, 10, 3); g.bound(c, 0, 3); g.glsl(x, y, c); - g.realize(out, target); + Buffer out = g.realize(255, 10, 3, target); out.copy_to_host(); if (!Testing::check_result(out, [&](int x, int y, int c) { return input(x, y, c); })) { diff --git a/test/opengl/copy_to_device.cpp b/test/opengl/copy_to_device.cpp index 7a476b248ea2..7599e562bcb2 100644 --- a/test/opengl/copy_to_device.cpp +++ b/test/opengl/copy_to_device.cpp @@ -25,8 +25,7 @@ int main() { g.bound(c, 0, 3); g.glsl(x, y, c); - Buffer out(255, 10, 3); - g.realize(out, target); + Buffer out = g.realize(255, 10, 3, target); out.copy_to_host(); if (!Testing::check_result(out, [&](int x, int y, int c) { return input(x, y, c); })) { diff --git a/test/opengl/float_texture.cpp b/test/opengl/float_texture.cpp index 166863d559ea..66ba34a7ff92 100644 --- a/test/opengl/float_texture.cpp +++ b/test/opengl/float_texture.cpp @@ -22,10 +22,9 @@ int main() { Func g; g(x, y, c) = input(x, y, c); - Buffer out(255, 255, 3); g.bound(c, 0, 3); g.glsl(x, y, c); - g.realize(out, target); + Buffer out = g.realize(255, 255, 3, target); out.copy_to_host(); if (!Testing::check_result(out, [&](int x, int y, int c) { return input(x, y, c); })) { diff --git a/test/opengl/lut.cpp b/test/opengl/lut.cpp index 7543db96d80f..2c513a3070f8 100644 --- a/test/opengl/lut.cpp +++ b/test/opengl/lut.cpp @@ -44,11 +44,9 @@ int test_lut1d() { f0(x, y, c) = lut1d(clamp(e, 0, 7), 0, c); - Buffer out0(8, 8, 3); - f0.bound(c, 0, 3); f0.glsl(x, y, c); - f0.realize(out0, target); + Buffer out0 = f0.realize(8, 8, 3, target); out0.copy_to_host(); if (!Testing::check_result(out0, [](int x, int y, int c) { diff --git a/test/opengl/produce.cpp b/test/opengl/produce.cpp index d00411642b6e..5c4e0db7a986 100644 --- a/test/opengl/produce.cpp +++ b/test/opengl/produce.cpp @@ -40,8 +40,7 @@ int test_lut1d() { f0.bound(c, 0, 3); f0.glsl(x, y, c); - Buffer out0(8, 8, 3); - f0.realize(out0, target); + Buffer out0 = f0.realize(8, 8, 3, target); out0.copy_to_host(); diff --git a/test/opengl/rewrap_texture.cpp b/test/opengl/rewrap_texture.cpp index 5993f0dccde5..d7d445fb9fed 100644 --- a/test/opengl/rewrap_texture.cpp +++ b/test/opengl/rewrap_texture.cpp @@ -32,10 +32,12 @@ int main() { const int width = 255; const int height = 10; - Buffer input(width, height, 3); - Buffer out1(width, height, 3); - Buffer out2(width, height, 3); - Buffer out3(width, height, 3); + // Create with the interleaved storage order needed by GLSL + const std::vector glsl_order{2, 0, 1}; + Buffer input({width, height, 3}, glsl_order); + Buffer out1({width, height, 3}, glsl_order); + Buffer out2({width, height, 3}, glsl_order); + Buffer out3({width, height, 3}, glsl_order); Var x, y, c; Func g; diff --git a/test/opengl/save_state.cpp b/test/opengl/save_state.cpp index a7006d75c78c..24e3f8766658 100644 --- a/test/opengl/save_state.cpp +++ b/test/opengl/save_state.cpp @@ -305,14 +305,13 @@ int main() { KnownState known_state; Buffer input(255, 10, 3); - Buffer out(UInt(8), 255, 10, 3); Var x, y, c; Func g; g(x, y, c) = input(x, y, c); g.bound(c, 0, 3); g.glsl(x, y, c); - g.realize(out, target); // let Halide initialize OpenGL + Buffer out = g.realize(255, 10, 3, target); // let Halide initialize OpenGL known_state.setup(true); g.realize(out, target); diff --git a/test/opengl/set_pixels.cpp b/test/opengl/set_pixels.cpp index 7c282878af0b..9b849ba75fc8 100644 --- a/test/opengl/set_pixels.cpp +++ b/test/opengl/set_pixels.cpp @@ -14,9 +14,8 @@ int main() { f(x, y, c) = cast(42); - Buffer out(10, 10, 3); f.bound(c, 0, 3).glsl(x, y, c); - f.realize(out, target); + Buffer out = f.realize(10, 10, 3, target); out.copy_to_host(); if (!Testing::check_result(out, [](int x, int y, int c) { return 42; })) { diff --git a/test/opengl/shifted_domains.cpp b/test/opengl/shifted_domains.cpp index 9ebd025c39b9..6ce176c1950d 100644 --- a/test/opengl/shifted_domains.cpp +++ b/test/opengl/shifted_domains.cpp @@ -23,14 +23,15 @@ int shifted_domains() { gradient.glsl(x, y, c); printf("Evaluating gradient from (0, 0) to (7, 7)\n"); - Buffer result(8, 8, 1); - gradient.realize(result, target); + Buffer result = gradient.realize(8, 8, 1, target); result.copy_to_host(); if (!Testing::check_result(result, 5e-5f, [](int x, int y) { return float(x + y); })) errors++; - Buffer shifted(5, 7, 1); + // Create with the interleaved storage order needed by GLSL + const std::vector glsl_order{2, 0, 1}; + Buffer shifted({5, 7, 1}, glsl_order); shifted.set_min(100, 50); printf("Evaluating gradient from (100, 50) to (104, 56)\n"); diff --git a/test/opengl/special_funcs.cpp b/test/opengl/special_funcs.cpp index 76441d7eb02e..844bf8b89b20 100644 --- a/test/opengl/special_funcs.cpp +++ b/test/opengl/special_funcs.cpp @@ -35,7 +35,9 @@ bool test_exact(Expr r, Expr g, Expr b) { b)); const int W = 256, H = 256; Buffer cpu_result(W, H, 3); - Buffer gpu_result(W, H, 3); + // Create with the interleaved storage order needed by GLSL + const std::vector glsl_order{2, 0, 1}; + Buffer gpu_result({W, H, 3}, glsl_order); test_function(e, cpu_result, gpu_result); for (int y = 0; y < gpu_result.height(); y++) { @@ -64,7 +66,9 @@ bool test_approx(Expr r, Expr g, Expr b, double rms_error) { Expr e = cast(select(c == 0, r, c == 1, g, b)); const int W = 256, H = 256; Buffer cpu_result(W, H, 3); - Buffer gpu_result(W, H, 3); + // Create with the interleaved storage order needed by GLSL + const std::vector glsl_order{2, 0, 1}; + Buffer gpu_result({W, H, 3}, glsl_order); test_function(e, cpu_result, gpu_result); double err = 0.0; diff --git a/test/opengl/tuples.cpp b/test/opengl/tuples.cpp index 59856f4bd020..55e7c0e28a9e 100644 --- a/test/opengl/tuples.cpp +++ b/test/opengl/tuples.cpp @@ -24,11 +24,10 @@ int main() { Func h; h(x, y, c) = min(g(x, y, c)[0], g(x, y, c)[1]); - Buffer out(255, 10, 3); g.compute_root(); h.compute_root().bound(c, 0, 3).glsl(x, y, c); - h.realize(out, target); + Buffer out = h.realize(255, 10, 3, target); out.copy_to_host(); if (!Testing::check_result(out, [&](int x, int y, int c) { return input(x, y, c) / 2; })) { diff --git a/test/opengl/varying.cpp b/test/opengl/varying.cpp index 058f56d6974e..e7856e531333 100644 --- a/test/opengl/varying.cpp +++ b/test/opengl/varying.cpp @@ -55,11 +55,9 @@ class CountVarying : public IRMutator2 { bool perform_test(const char *label, const Target target, Func f, int expected_nvarying, float tol, std::function expected_val) { fprintf(stderr, "%s\n", label); - Buffer out(8, 8, 3); - varyings.clear(); f.add_custom_lowering_pass(new CountVarying); - f.realize(out, target); + Buffer out = f.realize(8, 8, 3, target); // Check for the correct number of varying attributes if ((int)varyings.size() != expected_nvarying) { From 5ad7ebde14d3ebb84352bf49bd50ecea762f3d46 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Thu, 8 Nov 2018 14:48:40 -0800 Subject: [PATCH 8/8] More fixes --- apps/fft/main.cpp | 6 +++++ src/Func.cpp | 6 +++++ src/Func.h | 6 +++++ src/Function.cpp | 28 +++++++++++++++++++++ src/Function.h | 13 ++++++++++ src/Parameter.cpp | 64 ++++++++++++++++++++++++++--------------------- src/Pipeline.cpp | 18 +------------ 7 files changed, 96 insertions(+), 45 deletions(-) diff --git a/apps/fft/main.cpp b/apps/fft/main.cpp index 46ede08ed7da..1098afc8694d 100644 --- a/apps/fft/main.cpp +++ b/apps/fft/main.cpp @@ -153,6 +153,10 @@ int main(int argc, char **argv) { // locality. c2c_in(x, y, rep) = {re_in(x, y), im_in(x, y)}; Func bench_c2c = fft2d_c2c(c2c_in, W, H, -1, target, fwd_desc); + // We must disable inference of buffer constraints for this Func, + // since we are going to use unorthodox strides later on. (We can + // still set buffer constraints manually.) + bench_c2c.infer_buffer_constraints(false); bench_c2c.compile_to_lowered_stmt(output_dir + "c2c.html", bench_c2c.infer_arguments(), HTML); Realization R_c2c = bench_c2c.realize(W, H, reps, target); // Write all reps to the same place in memory. This means the @@ -184,6 +188,7 @@ int main(int argc, char **argv) { // All reps read from the same input. See notes on c2c_in. r2c_in(x, y, rep) = re_in(x, y); Func bench_r2c = fft2d_r2c(r2c_in, W, H, target, fwd_desc); + bench_r2c.infer_buffer_constraints(false); bench_r2c.compile_to_lowered_stmt(output_dir + "r2c.html", bench_r2c.infer_arguments(), HTML); Realization R_r2c = bench_r2c.realize(W, H/2 + 1, reps, target); // Write all reps to the same place in memory. See notes on R_c2c. @@ -210,6 +215,7 @@ int main(int argc, char **argv) { // All reps read from the same input. See notes on c2c_in. c2r_in(x, y, rep) = {re_in(x, y), im_in(x, y)}; Func bench_c2r = fft2d_c2r(c2r_in, W, H, target, inv_desc); + bench_c2r.infer_buffer_constraints(false); bench_c2r.compile_to_lowered_stmt(output_dir + "c2r.html", bench_c2r.infer_arguments(), HTML); Realization R_c2r = bench_c2r.realize(W, H, reps, target); // Write all reps to the same place in memory. See notes on R_c2c. diff --git a/src/Func.cpp b/src/Func.cpp index 5d3e64329286..b0abc8944d72 100644 --- a/src/Func.cpp +++ b/src/Func.cpp @@ -2480,6 +2480,12 @@ Func &Func::add_trace_tag(const std::string &trace_tag) { return *this; } +Func &Func::infer_buffer_constraints(bool infer) { + invalidate_cache(); + func.infer_buffer_constraints() = infer; + return *this; +} + void Func::debug_to_file(const string &filename) { invalidate_cache(); func.debug_file() = filename; diff --git a/src/Func.h b/src/Func.h index 367e5bf8043a..0c590163ade0 100644 --- a/src/Func.h +++ b/src/Func.h @@ -2301,6 +2301,12 @@ class Func { */ Func &add_trace_tag(const std::string &trace_tag); + /** Specify whether constraints for the buffer underlying this function + * should be inferred from other scheduling directives (e.g. to determine + * storage layout, strides, etc). Defaults to true. It should be quite + * rare to need to disable this. */ + Func &infer_buffer_constraints(bool infer); + /** Get a handle on the internal halide function that this Func * represents. Useful if you want to do introspection on Halide * functions */ diff --git a/src/Function.cpp b/src/Function.cpp index cfc74201f391..94d2655e75af 100644 --- a/src/Function.cpp +++ b/src/Function.cpp @@ -91,6 +91,7 @@ struct FunctionContents { std::vector trace_tags; bool frozen = false; + bool infer_buffer_constraints = true; void accept(IRVisitor *visitor) const { func_schedule.accept(visitor); @@ -343,6 +344,7 @@ void Function::deep_copy(FunctionPtr copy, DeepCopyMap &copied_map) const { copy->trace_realizations = contents->trace_realizations; copy->trace_tags = contents->trace_tags; copy->frozen = contents->frozen; + copy->infer_buffer_constraints = contents->infer_buffer_constraints; copy->output_buffers = contents->output_buffers; copy->func_schedule = contents->func_schedule.deep_copy(copied_map); @@ -796,6 +798,24 @@ const FuncSchedule &Function::schedule() const { return contents->func_schedule; } +std::vector Function::storage_order() const { + const std::vector &storage_dims = contents->func_schedule.storage_dims(); + const std::vector &args = contents->args; + internal_assert(storage_dims.size() == args.size()); + + std::vector storage_order; + for (const auto &s : storage_dims) { + for (size_t a = 0; a < args.size(); ++a) { + if (args[a] == s.var) { + storage_order.push_back((int) a); + break; + } + } + } + internal_assert(storage_order.size() == args.size()); + return storage_order; +} + const std::vector &Function::output_buffers() const { return contents->output_buffers; } @@ -896,6 +916,14 @@ std::string &Function::debug_file() { return contents->debug_file; } +bool Function::infer_buffer_constraints() const { + return contents->infer_buffer_constraints; +} + +bool &Function::infer_buffer_constraints() { + return contents->infer_buffer_constraints; +} + void Function::trace_loads() { contents->trace_loads = true; } diff --git a/src/Function.h b/src/Function.h index a2337c21728e..77402753a55a 100644 --- a/src/Function.h +++ b/src/Function.h @@ -186,6 +186,11 @@ class Function { /** Get a const handle to the function-specific schedule for inspecting it. */ const FuncSchedule &schedule() const; + /** Return a vector containing the order of storage for each argument, + * based on the scheduling directives. Each entry is an index into the args array. + */ + std::vector storage_order() const; + /** Get a handle on the output buffer used for setting constraints * on it. */ const std::vector &output_buffers() const; @@ -267,6 +272,14 @@ class Function { /** Get a handle to the debug filename. */ std::string &debug_file(); + /** If true, attempt to infer constraints on the buffer shape based + * on the scheduling directives. (Default = true; it is very rare to + * need to disable this.) */ + // @{ + bool infer_buffer_constraints() const; + bool &infer_buffer_constraints(); + // @} + /** Use an an extern argument to another function. */ operator ExternFuncArgument() const { return ExternFuncArgument(contents); diff --git a/src/Parameter.cpp b/src/Parameter.cpp index 3c3460bebdb5..074a5a1b494a 100644 --- a/src/Parameter.cpp +++ b/src/Parameter.cpp @@ -287,12 +287,26 @@ Expr Parameter::estimate() const { // Add constraints to a buffer based on the storage scheduling for f. void Parameter::set_constraints_from_schedule(Function f) { constexpr int D = 1; + if (!f.infer_buffer_constraints()) { + debug(D) << "set_constraints_from_schedule(" << f.name() << "): ignored due to infer_buffer_constraints = false\n"; + return; + } + const std::string ¶m_name = this->name(); const FuncSchedule &schedule = f.schedule(); const std::vector &storage_dims = schedule.storage_dims(); const std::vector &args = f.args(); + const std::vector storage_order = f.storage_order(); std::ostringstream o; + const auto emit_error = [&f](int dim, const std::string &s, Expr inferred, Expr expl) -> void { + user_error << "Inferred value for " << f.name() << "." << s << "." << dim + << " does not match the value explicitly specified." + << " (In very unusual cases, you may want to use infer_buffer_constraints(false) to disable this.)\n" + << " inferred:" << inferred << "\n" + << " explicit: " << expl << "\n"; + }; + std::vector extents(args.size()); for (size_t dim = 0; dim < args.size(); dim++) { extents[dim] = Variable::make(Int(32), param_name + ".extent." + std::to_string(dim), *this); @@ -316,8 +330,7 @@ void Parameter::set_constraints_from_schedule(Function f) { min = simplify(min + b.remainder); } if (min_constraint(dim).defined() && !equal(min, simplify(min_constraint(dim)))) { - user_error << "Inferred value for parameter \"" << f.name() << "\" min[" << dim << "] does not match" - " value explicitly specified (inferred " << min << " vs explicit " << min_constraint(dim) << ").\n"; + emit_error(dim, "min", min, min_constraint(dim)); } else { if (debug::debug_level() >= D) { o << " min." << dim << " -> " << min << "\n"; @@ -325,8 +338,7 @@ void Parameter::set_constraints_from_schedule(Function f) { set_min_constraint(dim, min); } if (extent_constraint(dim).defined() && !equal(extents[dim], simplify(extent_constraint(dim)))) { - user_error << "Inferred value for parameter \"" << f.name() << "\" extent[" << dim << "] does not match" - " value explicitly specified (inferred " << extents[dim] << " vs explicit " << extent_constraint(dim) << ").\n"; + emit_error(dim, "extent", extents[dim], extent_constraint(dim)); } else { if (debug::debug_level() >= D) { o << " extents." << dim << " -> " << extents[dim] << "\n"; @@ -342,34 +354,30 @@ void Parameter::set_constraints_from_schedule(Function f) { const auto is_default = [](int dim, const Expr &e) -> bool { return (dim == 0) ? is_one(e) : !e.defined(); }; + Expr stride = 1; - for (const StorageDim &storage_dim : storage_dims) { - for (size_t dim = 0; dim < args.size(); dim++) { - if (args[dim] == storage_dim.var) { - if (!is_default(dim, stride)) { - if (storage_dim.alignment.defined()) { - stride = (stride / storage_dim.alignment) / storage_dim.alignment; - } - Expr s = stride_constraint(dim); - if (!is_default(dim, s) && !equal(stride, simplify(s))) { - user_error << "Inferred value for parameter \"" << f.name() << "\" stride[" << dim << "] does not match" - " value explicitly specified (inferred " << stride << " vs explicit " << s << ").\n"; - } else { - set_stride_constraint(dim, stride); - if (debug::debug_level() >= D) { - o << " stride." << dim << " -> " << stride << " (was " << s << ")\n"; - } - } - } - Expr extent = extents[dim]; - if (stride.defined() && is_const(extent)) { - stride = simplify(stride * extent); - } else { - stride = Expr(); + for (size_t i = 0; i < storage_order.size(); ++i) { + const int dim = storage_order[i]; + if (!is_default(dim, stride)) { + const auto &storage_dim = storage_dims[i]; + if (storage_dim.alignment.defined()) { + stride = (stride / storage_dim.alignment) / storage_dim.alignment; + } + Expr s = stride_constraint(dim); + if (!is_default(dim, s) && !equal(stride, simplify(s))) { + emit_error(dim, "stride", stride, s); + } else { + if (debug::debug_level() >= D) { + o << " stride." << dim << " -> " << stride << " (was " << s << ")\n"; } - break; + set_stride_constraint(dim, stride); } } + if (stride.defined() && is_const(extents[dim])) { + stride = simplify(stride * extents[dim]); + } else { + stride = Expr(); + } } if (!o.str().empty()) { diff --git a/src/Pipeline.cpp b/src/Pipeline.cpp index 3817900e84ff..2f687828c1da 100644 --- a/src/Pipeline.cpp +++ b/src/Pipeline.cpp @@ -546,23 +546,7 @@ Realization Pipeline::realize(vector sizes, const Target &target, // Attempt to create a Buffer that has the storage laid out in the // same order as that specified by our schedule. - std::vector storage_order(sizes.size()); - { - // Should this be moved into (say) Function::get_storage_order()? - const FuncSchedule &schedule = f.schedule(); - const std::vector &storage_dims = schedule.storage_dims(); - const std::vector &args = f.args(); - - for (size_t s = 0; s < storage_dims.size(); ++s) { - for (size_t a = 0; a < args.size(); ++a) { - if (args[a] == storage_dims[s].var) { - storage_order[s] = a; - break; - } - } - } - } - + const std::vector storage_order = f.storage_order(); for (Type t : f.output_types()) { bufs.emplace_back(t, sizes, storage_order); }