Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various fixes to static-dimensioned Buffer #6589

Merged
merged 2 commits into from
Jan 27, 2022
Merged

Various fixes to static-dimensioned Buffer #6589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b38343e
Various fixes to static-dimensioned Buffer
steven-johnson Jan 27, 2022
79dcb45
Update Generator.h
steven-johnson Jan 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/Buffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,16 +139,16 @@ class Buffer {
} else {
// Don't delegate to
// Runtime::Buffer<T>::assert_can_convert_from. It might
// not assert is NDEBUG is defined. user_assert is
// not assert if NDEBUG is defined. user_assert is
// friendlier anyway because it reports line numbers when
// debugging symbols are found, it throws an exception
// when exceptions are enabled, and we can print the
// actual types in question.
using BufType = Runtime::Buffer<T, Dims>; // alias because commas in user_assert() macro confuses compiler
user_assert(BufType::can_convert_from(*(other.get())))
<< "Type mismatch constructing Buffer. Can't construct Buffer<"
<< Internal::buffer_type_name<T>() << "> from Buffer<"
<< type_to_c_type(other.type(), false) << ">\n";
<< Internal::buffer_type_name<T>() << ", " << Dims << "> from Buffer<"
<< type_to_c_type(other.type(), false) << ", " << D2 << ">, dimensions() = " << other.dimensions() << "\n";
}
}

Expand Down Expand Up @@ -509,13 +509,13 @@ class Buffer {

static constexpr bool has_static_halide_type = Runtime::Buffer<T, Dims>::has_static_halide_type;

static halide_type_t static_halide_type() {
static constexpr halide_type_t static_halide_type() {
return Runtime::Buffer<T, Dims>::static_halide_type();
}

static constexpr bool has_static_dimensions = Runtime::Buffer<T, Dims>::has_static_dimensions;

static int static_dimensions() {
static constexpr int static_dimensions() {
return Runtime::Buffer<T, Dims>::static_dimensions();
}

Expand All @@ -533,7 +533,7 @@ class Buffer {
}

template<typename T2, int D2 = Dims>
Buffer<T2, Dims> as() const {
Buffer<T2, D2> as() const {
return Buffer<T2, D2>(*this);
}

Expand Down
4 changes: 2 additions & 2 deletions src/ExternFuncArgument.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ struct ExternFuncArgument {
: arg_type(FuncArg), func(std::move(f)) {
}

template<typename T>
ExternFuncArgument(Buffer<T> b)
template<typename T, int Dims>
ExternFuncArgument(Buffer<T, Dims> b)
: arg_type(BufferArg), buffer(b) {
}
ExternFuncArgument(Expr e)
Expand Down
8 changes: 4 additions & 4 deletions src/Func.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,8 +740,8 @@ class Func {
explicit Func(Internal::Function f);

/** Construct a new Func to wrap a Buffer. */
template<typename T>
HALIDE_NO_USER_CODE_INLINE explicit Func(Buffer<T> &im)
template<typename T, int Dims>
HALIDE_NO_USER_CODE_INLINE explicit Func(Buffer<T, Dims> &im)
: Func() {
(*this)(_) = im(_);
}
Expand Down Expand Up @@ -2560,7 +2560,7 @@ HALIDE_NO_USER_CODE_INLINE T evaluate(JITUserContext *ctx, const Expr &e) {
<< " as a scalar of type " << type_of<T>() << "\n";
Func f;
f() = e;
Buffer<T> im = f.realize(ctx);
Buffer<T, 0> im = f.realize(ctx);
return im();
}

Expand Down Expand Up @@ -2615,7 +2615,7 @@ HALIDE_NO_USER_CODE_INLINE T evaluate_may_gpu(const Expr &e) {
Func f;
f() = e;
Internal::schedule_scalar(f);
Buffer<T> im = f.realize();
Buffer<T, 0> im = f.realize();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a minor stack space optimization, right? I don't see that this zero makes much of a difference because the dims array is never accessed. I have no objection to adding it, I just want to make sure I understand what's happening.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, it's just a tiny optimization.

(On that note: the code for Buffer currently always allocates space for at least one halide_dimension_t, even for zero-dim buffers, on the assumption that there is probably sloppy code out there that assumes that buffer.dim always points to something valid...)

return im();
}

Expand Down
106 changes: 82 additions & 24 deletions src/Generator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,7 @@ enum class IOKind { Scalar,
* -- Assignment of a Buffer<>, with compatible type and dimensions,
* causing the Input<Buffer<>> to become a precompiled buffer in the generated code.
*/
template<typename T = void>
template<typename T = void, int Dims = Buffer<>::BufferDimsUnconstrained>
class StubInputBuffer {
friend class StubInput;
template<typename T2>
Expand All @@ -1286,13 +1286,13 @@ class StubInputBuffer {
// which we'll use only to pass to can_convert_from() to verify this
// Parameter is compatible with our constraints.
Buffer<> other(p.type(), nullptr, std::vector<int>(p.dimensions(), 1));
internal_assert((Buffer<T>::can_convert_from(other)));
internal_assert((Buffer<T, Dims>::can_convert_from(other)));
}

template<typename T2>
HALIDE_NO_USER_CODE_INLINE static Parameter parameter_from_buffer(const Buffer<T2> &b) {
template<typename T2, int D2>
HALIDE_NO_USER_CODE_INLINE static Parameter parameter_from_buffer(const Buffer<T2, D2> &b) {
internal_assert(b.defined());
user_assert((Buffer<T>::can_convert_from(b)));
user_assert((Buffer<T, Dims>::can_convert_from(b)));
Parameter p(b.type(), true, b.dimensions());
p.set_buffer(b);
return p;
Expand All @@ -1305,8 +1305,8 @@ class StubInputBuffer {
// to pass a literal Buffer<> for a Stub Input; this Buffer<> will be
// compiled into the Generator's product, rather than becoming
// a runtime Parameter.
template<typename T2>
StubInputBuffer(const Buffer<T2> &b)
template<typename T2, int D2>
StubInputBuffer(const Buffer<T2, D2> &b)
: parameter_(parameter_from_buffer(b)) {
}
};
Expand Down Expand Up @@ -2555,8 +2555,8 @@ class GeneratorOutput_Buffer : public GeneratorOutputImpl<T> {
// TODO: This used to take the buffer as a const ref. This no longer works as
// using it in a Pipeline might change the dev field so it is currently
// not considered const. We should consider how this really ought to work.
template<typename T2>
HALIDE_NO_USER_CODE_INLINE GeneratorOutput_Buffer<T> &operator=(Buffer<T2> &buffer) {
template<typename T2, int D2>
HALIDE_NO_USER_CODE_INLINE GeneratorOutput_Buffer<T> &operator=(Buffer<T2, D2> &buffer) {
this->check_gio_access();
this->check_value_writable();

Expand Down Expand Up @@ -2819,8 +2819,8 @@ class GeneratorOutput : public Internal::GeneratorOutputImplBase<T> {
// TODO: This used to take the buffer as a const ref. This no longer works as
// using it in a Pipeline might change the dev field so it is currently
// not considered const. We should consider how this really ought to work.
template<typename T2>
GeneratorOutput<T> &operator=(Buffer<T2> &buffer) {
template<typename T2, int D2>
GeneratorOutput<T> &operator=(Buffer<T2, D2> &buffer) {
Super::operator=(buffer);
return *this;
}
Expand Down Expand Up @@ -3259,9 +3259,9 @@ class GeneratorBase : public NamesInterface, public GeneratorContext {
Pipeline get_pipeline();
#endif

// Create Input<Buffer> or Input<Func> with dynamic type
// Create Input<Func> with dynamic type & dimensions
template<typename T,
typename std::enable_if<!std::is_arithmetic<T>::value>::type * = nullptr>
typename std::enable_if<std::is_same<T, Halide::Func>::value>::type * = nullptr>
GeneratorInput<T> *add_input(const std::string &name, const Type &t, int dimensions) {
check_exact_phase(GeneratorBase::ConfigureCalled);
auto *p = new GeneratorInput<T>(name, t, dimensions);
Expand All @@ -3271,10 +3271,26 @@ class GeneratorBase : public NamesInterface, public GeneratorContext {
return p;
}

// Create a Input<Buffer> or Input<Func> with compile-time type
// Create Input<Buffer> with dynamic type & dimensions
template<typename T,
typename std::enable_if<T::has_static_halide_type>::type * = nullptr>
typename std::enable_if<!std::is_arithmetic<T>::value && !std::is_same<T, Halide::Func>::value>::type * = nullptr>
GeneratorInput<T> *add_input(const std::string &name, const Type &t, int dimensions) {
static_assert(!T::has_static_halide_type, "You can only call this version of add_input() for a Buffer<T, D> where T is void or omitted .");
static_assert(!T::has_static_dimensions, "You can only call this version of add_input() for a Buffer<T, D> where D is -1 or omitted.");
check_exact_phase(GeneratorBase::ConfigureCalled);
auto *p = new GeneratorInput<T>(name, t, dimensions);
p->generator = this;
param_info_ptr->owned_extras.push_back(std::unique_ptr<Internal::GIOBase>(p));
param_info_ptr->filter_inputs.push_back(p);
return p;
}

// Create Input<Buffer> with compile-time type
template<typename T,
typename std::enable_if<!std::is_arithmetic<T>::value && !std::is_same<T, Halide::Func>::value>::type * = nullptr>
GeneratorInput<T> *add_input(const std::string &name, int dimensions) {
static_assert(T::has_static_halide_type, "You can only call this version of add_input() for a Buffer<T, D> where T is not void.");
static_assert(!T::has_static_dimensions, "You can only call this version of add_input() for a Buffer<T, D> where D is -1 or omitted.");
check_exact_phase(GeneratorBase::ConfigureCalled);
auto *p = new GeneratorInput<T>(name, dimensions);
p->generator = this;
Expand All @@ -3283,6 +3299,19 @@ class GeneratorBase : public NamesInterface, public GeneratorContext {
return p;
}

// Create Input<Buffer> with compile-time type & dimensions
template<typename T,
typename std::enable_if<!std::is_arithmetic<T>::value && !std::is_same<T, Halide::Func>::value>::type * = nullptr>
GeneratorInput<T> *add_input(const std::string &name) {
static_assert(T::has_static_halide_type, "You can only call this version of add_input() for a Buffer<T, D> where T is not void.");
static_assert(T::has_static_dimensions, "You can only call this version of add_input() for a Buffer<T, D> where D is not -1.");
check_exact_phase(GeneratorBase::ConfigureCalled);
auto *p = new GeneratorInput<T>(name);
p->generator = this;
param_info_ptr->owned_extras.push_back(std::unique_ptr<Internal::GIOBase>(p));
param_info_ptr->filter_inputs.push_back(p);
return p;
}
// Create Input<scalar>
template<typename T,
typename std::enable_if<std::is_arithmetic<T>::value>::type * = nullptr>
Expand All @@ -3294,7 +3323,6 @@ class GeneratorBase : public NamesInterface, public GeneratorContext {
param_info_ptr->filter_inputs.push_back(p);
return p;
}

// Create Input<Expr> with dynamic type
template<typename T,
typename std::enable_if<std::is_same<T, Expr>::value>::type * = nullptr>
Expand All @@ -3308,10 +3336,24 @@ class GeneratorBase : public NamesInterface, public GeneratorContext {
return p;
}

// Create Output<Buffer> or Output<Func> with dynamic type
// Create Output<Func> with dynamic type & dimensions
template<typename T,
typename std::enable_if<std::is_same<T, Halide::Func>::value>::type * = nullptr>
GeneratorOutput<T> *add_output(const std::string &name, const Type &t, int dimensions) {
check_exact_phase(GeneratorBase::ConfigureCalled);
auto *p = new GeneratorOutput<T>(name, t, dimensions);
p->generator = this;
param_info_ptr->owned_extras.push_back(std::unique_ptr<Internal::GIOBase>(p));
param_info_ptr->filter_outputs.push_back(p);
return p;
}

// Create Output<Buffer> with dynamic type & dimensions
template<typename T,
typename std::enable_if<!std::is_arithmetic<T>::value>::type * = nullptr>
typename std::enable_if<!std::is_arithmetic<T>::value && !std::is_same<T, Halide::Func>::value>::type * = nullptr>
GeneratorOutput<T> *add_output(const std::string &name, const Type &t, int dimensions) {
static_assert(!T::has_static_halide_type, "You can only call this version of add_output() for a Buffer<T, D> where T is void or omitted .");
static_assert(!T::has_static_dimensions, "You can only call this version of add_output() for a Buffer<T, D> where D is -1 or omitted.");
check_exact_phase(GeneratorBase::ConfigureCalled);
auto *p = new GeneratorOutput<T>(name, t, dimensions);
p->generator = this;
Expand All @@ -3320,10 +3362,12 @@ class GeneratorBase : public NamesInterface, public GeneratorContext {
return p;
}

// Create a Output<Buffer> or Output<Func> with compile-time type
// Create Output<Buffer> with compile-time type
template<typename T,
typename std::enable_if<T::has_static_halide_type>::type * = nullptr>
typename std::enable_if<!std::is_arithmetic<T>::value && !std::is_same<T, Halide::Func>::value>::type * = nullptr>
GeneratorOutput<T> *add_output(const std::string &name, int dimensions) {
static_assert(T::has_static_halide_type, "You can only call this version of add_output() for a Buffer<T, D> where T is not void.");
static_assert(!T::has_static_dimensions, "You can only call this version of add_output() for a Buffer<T, D> where D is -1 or omitted.");
check_exact_phase(GeneratorBase::ConfigureCalled);
auto *p = new GeneratorOutput<T>(name, dimensions);
p->generator = this;
Expand All @@ -3332,6 +3376,20 @@ class GeneratorBase : public NamesInterface, public GeneratorContext {
return p;
}

// Create Output<Buffer> with compile-time type & dimensions
template<typename T,
typename std::enable_if<!std::is_arithmetic<T>::value && !std::is_same<T, Halide::Func>::value>::type * = nullptr>
GeneratorOutput<T> *add_output(const std::string &name) {
static_assert(T::has_static_halide_type, "You can only call this version of add_output() for a Buffer<T, D> where T is not void.");
static_assert(T::has_static_dimensions, "You can only call this version of add_output() for a Buffer<T, D> where D is not -1.");
check_exact_phase(GeneratorBase::ConfigureCalled);
auto *p = new GeneratorOutput<T>(name);
p->generator = this;
param_info_ptr->owned_extras.push_back(std::unique_ptr<Internal::GIOBase>(p));
param_info_ptr->filter_outputs.push_back(p);
return p;
}

template<typename... Args>
HALIDE_NO_USER_CODE_INLINE void add_requirement(Expr condition, Args &&...args) {
get_pipeline().add_requirement(condition, std::forward<Args>(args)...);
Expand Down Expand Up @@ -3459,8 +3517,8 @@ class GeneratorBase : public NamesInterface, public GeneratorContext {
// -- we are assigning it to an Input<Buffer<>> (with compatible type and dimensions),
// causing the Input<Buffer<>> to become a precompiled buffer in the generated code.
// -- we are assigningit to an Input<Func>, in which case we just Func-wrap the Buffer<>.
template<typename T>
std::vector<StubInput> build_input(size_t i, const Buffer<T> &arg) {
template<typename T, int Dims>
std::vector<StubInput> build_input(size_t i, const Buffer<T, Dims> &arg) {
auto *in = param_info().inputs().at(i);
check_input_is_singular(in);
const auto k = in->kind();
Expand All @@ -3484,8 +3542,8 @@ class GeneratorBase : public NamesInterface, public GeneratorContext {
// -- we are assigning it to another Input<Buffer<>> (with compatible type and dimensions),
// allowing us to simply pipe a parameter from an enclosing Generator to the Invoker.
// -- we are assigningit to an Input<Func>, in which case we just Func-wrap the Input<Buffer<>>.
template<typename T>
std::vector<StubInput> build_input(size_t i, const GeneratorInput<Buffer<T>> &arg) {
template<typename T, int Dims>
std::vector<StubInput> build_input(size_t i, const GeneratorInput<Buffer<T, Dims>> &arg) {
auto *in = param_info().inputs().at(i);
check_input_is_singular(in);
const auto k = in->kind();
Expand Down
8 changes: 4 additions & 4 deletions src/ParamMap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,17 @@ class ParamMap {
: image_param(&p), buf(buf), buf_out_param(nullptr) {
}

template<typename T>
ParamMapping(const ImageParam &p, Buffer<T> &buf)
template<typename T, int Dims>
ParamMapping(const ImageParam &p, Buffer<T, Dims> &buf)
: image_param(&p), buf(buf), buf_out_param(nullptr) {
}

ParamMapping(const ImageParam &p, Buffer<> *buf_ptr)
: image_param(&p), buf_out_param(buf_ptr) {
}

template<typename T>
ParamMapping(const ImageParam &p, Buffer<T> *buf_ptr)
template<typename T, int Dims>
ParamMapping(const ImageParam &p, Buffer<T, Dims> *buf_ptr)
: image_param(&p), buf_out_param((Buffer<> *)buf_ptr) {
}
};
Expand Down
12 changes: 6 additions & 6 deletions src/Pipeline.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,17 +113,17 @@ class Pipeline {
RealizationArg(halide_buffer_t *buf)
: buf(buf) {
}
template<typename T, int D>
RealizationArg(Runtime::Buffer<T, D> &dst)
template<typename T, int Dims>
RealizationArg(Runtime::Buffer<T, Dims> &dst)
: buf(dst.raw_buffer()) {
}
template<typename T>
HALIDE_NO_USER_CODE_INLINE RealizationArg(Buffer<T> &dst)
template<typename T, int Dims>
HALIDE_NO_USER_CODE_INLINE RealizationArg(Buffer<T, Dims> &dst)
: buf(dst.raw_buffer()) {
}
template<typename T, typename... Args,
template<typename T, int Dims, typename... Args,
typename = typename std::enable_if<Internal::all_are_convertible<Buffer<>, Args...>::value>::type>
RealizationArg(Buffer<T> &a, Args &&...args)
RealizationArg(Buffer<T, Dims> &a, Args &&...args)
: buffer_list(std::make_unique<std::vector<Buffer<>>>(std::initializer_list<Buffer<>>{a, std::forward<Args>(args)...})) {
}
RealizationArg(RealizationArg &&from) = default;
Expand Down
6 changes: 3 additions & 3 deletions src/runtime/HalideBuffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1493,7 +1493,7 @@ class Buffer {

/** Make a lower-dimensional buffer that refers to one slice of
* this buffer. */
Buffer<T, (Dims == BufferDimsUnconstrained ? BufferDimsUnconstrained : Dims - 1), InClassDimStorage>
Buffer<T, (Dims == BufferDimsUnconstrained ? BufferDimsUnconstrained : Dims - 1)>
sliced(int d, int pos) const {
static_assert(Dims == BufferDimsUnconstrained || Dims > 0, "Cannot slice a 0-dimensional buffer");
assert(dimensions() > 0);
Expand All @@ -1514,7 +1514,7 @@ class Buffer {

/** Make a lower-dimensional buffer that refers to one slice of this
* buffer at the dimension's minimum. */
Buffer<T, (Dims == BufferDimsUnconstrained ? BufferDimsUnconstrained : Dims - 1), InClassDimStorage>
Buffer<T, (Dims == BufferDimsUnconstrained ? BufferDimsUnconstrained : Dims - 1)>
sliced(int d) const {
static_assert(Dims == BufferDimsUnconstrained || Dims > 0, "Cannot slice a 0-dimensional buffer");
assert(dimensions() > 0);
Expand Down Expand Up @@ -1557,7 +1557,7 @@ class Buffer {
&im(x, y, c) == &im2(x, 17, y, c);
\endcode
*/
Buffer<T, (Dims == BufferDimsUnconstrained ? BufferDimsUnconstrained : Dims + 1), InClassDimStorage>
Buffer<T, (Dims == BufferDimsUnconstrained ? BufferDimsUnconstrained : Dims + 1)>
embedded(int d, int pos = 0) const {
Buffer<T, BufferDimsUnconstrained, InClassDimStorage> im(*this);
im.embed(d, pos);
Expand Down
Loading