Various fixes to static-dimensioned Buffer#6589
Conversation
- Buffer needed to make some methods constexpr, and also had a broken return value for `as<>()` - Various templated methods needed to add `int Dims` as a parameter - Generator::add_input() and add_output() needed specializations for static-buffer types - sliced() and embedded() should use the default values for InClassDimStorage - halide_image_io should use a named constant
| 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_dimensions, "You can only call this version of add_input() for a Buffer<T, D> where T is void or omitted ."); |
There was a problem hiding this comment.
These static assert conditions seem to be the same on both lines
There was a problem hiding this comment.
whoops, first should be has_static_halide_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) { | ||
| static_assert(T::has_static_dimensions, "You can only call this version of add_input() for a Buffer<T, D> where T is not void."); |
| 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_dimensions, "You can only call this version of add_output() for a Buffer<T, D> where T is void or omitted ."); |
| 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_dimensions, "You can only call this version of add_output() for a Buffer<T, D> where T is not void."); |
There was a problem hiding this comment.
and here, unless I'm really not understanding something
| // as documentation and a backstop against breakage. | ||
| static_assert(!ImageType::has_static_halide_type, | ||
| "This variant of convert_image() requires a dynamically-typed image"); | ||
| constexpr int DimsUnconstrained = Internal::DimsUnconstrained; |
There was a problem hiding this comment.
Would "AnyDims" be a better name than "DimsUnconstrained"?
There was a problem hiding this comment.
Unconstrained is the term we use in HalideBuffer.h so that's what I used here.
There was a problem hiding this comment.
I know, I was reconsidering the name in general. Seeing it in usage makes it seem clunky.
There was a problem hiding this comment.
Ah, gotcha. I think I originally called it "DynamicDims" and @zvookin suggested "DimsUnconstrained". "AnyDims" is fine with me too and definitely terser. I'll make the change everywhere if you like.
|
PTAL |
| f() = e; | ||
| Internal::schedule_scalar(f); | ||
| Buffer<T> im = f.realize(); | ||
| Buffer<T, 0> im = f.realize(); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...)
* Various fixes to static-dimensioned Buffer - Buffer needed to make some methods constexpr, and also had a broken return value for `as<>()` - Various templated methods needed to add `int Dims` as a parameter - Generator::add_input() and add_output() needed specializations for static-buffer types - sliced() and embedded() should use the default values for InClassDimStorage - halide_image_io should use a named constant * Update Generator.h
2 participants
More details that should have been in #6574:
as<>()int Dimsas a parameter