Skip to content

Remove deprecated build() support from Generators #6684

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

Merged
steven-johnson merged 2 commits into from
Apr 8, 2022
Merged

Remove deprecated build() support from Generators #6684

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
5e7fa09
Remove deprecated `build()` support from Generators
steven-johnson Apr 7, 2022
dc85cf5
Merge branch 'main' into srj/debuild
steven-johnson Apr 7, 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
121 changes: 4 additions & 117 deletions src/Generator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
#include "Module.h"
#include "Simplify.h"

#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
#pragma message "Support for Generator build() methods has been removed in Halide version 15."
#endif

namespace Halide {

GeneratorContext::GeneratorContext(const Target &target,
Expand Down Expand Up @@ -717,23 +721,10 @@ std::vector<std::vector<Func>> GeneratorStub::generate(const GeneratorParamsMap

std::vector<std::vector<Func>> v;
GeneratorParamInfo &pi = generator->param_info();
#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
if (!pi.outputs().empty()) {
for (auto *output : pi.outputs()) {
v.push_back(get_outputs(output->name()));
}
} else {
// Generators with build() method can't have Output<>, hence can't have array outputs
for (const auto &output : p.outputs()) {
v.push_back(std::vector<Func>{output});
}
}
#else
internal_assert(!pi.outputs().empty());
for (auto *output : pi.outputs()) {
v.push_back(get_outputs(output->name()));
}
#endif
return v;
}

Expand Down Expand Up @@ -1174,27 +1165,17 @@ void GeneratorParamBase::check_value_readable() const {
name() == "machine_params") {
return;
}
#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
user_assert(generator && generator->phase >= GeneratorBase::ConfigureCalled)
<< "The GeneratorParam \"" << name() << "\" cannot be read before build() or configure()/generate() is called.\n";
#else
user_assert(generator && generator->phase >= GeneratorBase::ConfigureCalled)
<< "The GeneratorParam \"" << name() << "\" cannot be read before configure()/generate() is called.\n";
#endif
}

void GeneratorParamBase::check_value_writable() const {
// Allow writing when no Generator is set, to avoid having to special-case ctor initing code
if (!generator) {
return;
}
#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
user_assert(generator->phase < GeneratorBase::GenerateCalled)
<< "The GeneratorParam \"" << name() << "\" cannot be written after build() or generate() is called.\n";
#else
user_assert(generator->phase < GeneratorBase::GenerateCalled)
<< "The GeneratorParam \"" << name() << "\" cannot be written after generate() is called.\n";
#endif
}

void GeneratorParamBase::fail_wrong_type(const char *type) {
Expand Down Expand Up @@ -1531,26 +1512,6 @@ void GeneratorBase::post_schedule() {
track_parameter_values(true);
}

#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
void GeneratorBase::pre_build() {
advance_phase(GenerateCalled);
advance_phase(ScheduleCalled);
GeneratorParamInfo &pi = param_info();
user_assert(pi.outputs().empty()) << "May not use build() method with Output<>.";
if (!inputs_set) {
for (auto *input : pi.inputs()) {
input->init_internals();
}
inputs_set = true;
}
track_parameter_values(false);
}

void GeneratorBase::post_build() {
track_parameter_values(true);
}
#endif

Pipeline GeneratorBase::get_pipeline() {
check_min_phase(GenerateCalled);
if (!pipeline.defined()) {
Expand Down Expand Up @@ -1984,13 +1945,8 @@ void GIOBase::check_gio_access() const {
if (!generator) {
return;
}
#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
user_assert(generator->phase > GeneratorBase::InputsSet)
<< "The " << input_or_output() << " \"" << name() << "\" cannot be examined before build() or generate() is called.\n";
#else
user_assert(generator->phase > GeneratorBase::InputsSet)
<< "The " << input_or_output() << " \"" << name() << "\" cannot be examined before generate() is called.\n";
#endif
}

// If our dims are defined, ensure it matches the one passed in, asserting if not.
Expand Down Expand Up @@ -2270,75 +2226,6 @@ void generator_test() {
// tester.sp2.set(202); // This will assert-fail.
}

#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
// Verify that the Generator's internal phase actually prevents unsupported
// order of operations (with old-style Generator)
{
class Tester : public Generator<Tester> {
public:
GeneratorParam<int> gp0{"gp0", 0};
GeneratorParam<float> gp1{"gp1", 1.f};
GeneratorParam<uint64_t> gp2{"gp2", 2};
GeneratorParam<uint8_t> gp_uint8{"gp_uint8", 65};
GeneratorParam<int8_t> gp_int8{"gp_int8", 66};
GeneratorParam<char> gp_char{"gp_char", 97};
GeneratorParam<signed char> gp_schar{"gp_schar", 98};
GeneratorParam<unsigned char> gp_uchar{"gp_uchar", 99};
GeneratorParam<bool> gp_bool{"gp_bool", true};

Input<int> input{"input"};

Func build() {
internal_assert(gp0 == 1);
internal_assert(gp1 == 2.f);
internal_assert(gp2 == (uint64_t)2); // unchanged
internal_assert(gp_uint8 == 67);
internal_assert(gp_int8 == 68);
internal_assert(gp_bool == false);
internal_assert(gp_char == 107);
internal_assert(gp_schar == 108);
internal_assert(gp_uchar == 109);
Var x;
Func output;
output(x) = input + gp0;
return output;
}
};

Tester tester;
tester.init_from_context(context);
internal_assert(tester.phase == GeneratorBase::Created);

// Verify that calling GeneratorParam::set() works.
tester.gp0.set(1);

// set_inputs_vector() can't be called on an old-style Generator;
// that's OK, since we can skip from Created -> GenerateCalled anyway
// tester.set_inputs_vector({{StubInput(42)}});
// internal_assert(tester.phase == GeneratorBase::InputsSet);

// tester.set_inputs_vector({{StubInput(43)}}); // This will assert-fail.

// Also ok to call in this phase.
tester.gp1.set(2.f);

// Verify that 8-bit non-boolean GP values are parsed as integers, not chars.
tester.gp_int8.set_from_string("68");
tester.gp_uint8.set_from_string("67");
tester.gp_char.set_from_string("107");
tester.gp_schar.set_from_string("108");
tester.gp_uchar.set_from_string("109");
tester.gp_bool.set_from_string("false");

tester.build_pipeline();
internal_assert(tester.phase == GeneratorBase::ScheduleCalled);

// tester.set_inputs_vector({{StubInput(45)}}); // This will assert-fail.
// tester.gp2.set(2); // This will assert-fail.
// tester.sp2.set(202); // This will assert-fail.
}
#endif

// Verify that set_inputs() works properly, even if the specific subtype of Generator is not known.
{
class Tester : public Generator<Tester> {
Expand Down
156 changes: 1 addition & 155 deletions src/Generator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3185,20 +3185,11 @@ class GeneratorBase : public NamesInterface {
get_pipeline().realize(r, get_target());
}

#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
// Return the Pipeline that has been built by the generate() method.
// This method can only be used from a Generator that has a generate()
// method (vs a build() method), and currently can only be called from
// the schedule() method. (This may be relaxed in the future to allow
// calling from generate() as long as all Outputs have been defined.)
Pipeline get_pipeline();
#else
// Return the Pipeline that has been built by the generate() method.
// This method can only be called from the schedule() method.
// (This may be relaxed in the future to allow calling from generate() as
// long as all Outputs have been defined.)
Pipeline get_pipeline();
#endif

// Create Input<Func> with dynamic type & dimensions
template<typename T,
Expand Down Expand Up @@ -3386,15 +3377,8 @@ class GeneratorBase : public NamesInterface {
// in AOT mode, this can be skipped, going Created->GenerateCalled directly.)
InputsSet,

#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
// Generator has had its generate() method called. (For Generators with
// a build() method instead of generate(), this phase will be skipped
// and will advance directly to ScheduleCalled.)
GenerateCalled,
#else
// Generator has had its generate() method called.
GenerateCalled,
#endif

// Generator has had its schedule() method (if any) called.
ScheduleCalled,
Expand Down Expand Up @@ -3684,14 +3668,6 @@ class Generator : public Internal::GeneratorBase {

template<typename... Args>
void apply(const Args &...args) {
#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
#ifndef _MSC_VER
// VS2015 apparently has some SFINAE issues, so this can inappropriately
// trigger there. (We'll still fail when generate() is called, just
// with a less-helpful error message.)
static_assert(has_generate_method<T>::value, "apply() is not supported for old-style Generators.");
#endif
#endif
call_configure();
set_inputs(args...);
call_generate();
Expand Down Expand Up @@ -3734,118 +3710,6 @@ class Generator : public Internal::GeneratorBase {
template<typename T2>
struct has_schedule_method<T2, typename type_sink<decltype(std::declval<T2>().schedule())>::type> : std::true_type {};

#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
// Implementations for build_pipeline_impl(), specialized on whether we
// have build() or generate()/schedule() methods.

// MSVC apparently has some weirdness with the usual sfinae tricks
// for detecting method-shaped things, so we can't actually use
// the helpers above outside of static_assert. Instead we make as
// many overloads as we can exist, and then use C++'s preference
// for treating a 0 as an int rather than a double to choose one
// of them.
template<typename T2 = T,
typename std::enable_if<!has_generate_method<T2>::value>::type * = nullptr>
HALIDE_ATTRIBUTE_DEPRECATED("The build() method is deprecated for Halide Generators and will be removed entirely in future versions of Halide. Please use a generate() method with Output<> members instead.")
Pipeline build_pipeline_impl(double) {
static_assert(!has_configure_method<T2>::value, "The configure() method is ignored if you define a build() method; use generate() instead.");
static_assert(!has_schedule_method<T2>::value, "The schedule() method is ignored if you define a build() method; use generate() instead.");

user_warning << "The build() method is deprecated for Halide Generators and will be removed entirely in future versions of Halide. "
<< "Please use a generate() method with Output<> members instead.\n";

pre_build();
Pipeline p = ((T *)this)->build();
post_build();
return p;
}

template<typename T2 = T,
typename = decltype(std::declval<T2>().generate())>
Pipeline build_pipeline_impl(int) {
// No: configure() must be called prior to this
// (and in fact, prior to calling set_inputs).
//
// ((T *)this)->call_configure_impl(0, 0);

((T *)this)->call_generate_impl(0);
((T *)this)->call_schedule_impl(0, 0);
return get_pipeline();
}

// Implementations for call_configure_impl(), specialized on whether we
// have build() or configure()/generate()/schedule() methods.

void call_configure_impl(double, double) {
pre_configure();
// Called as a side effect for build()-method Generators; quietly do nothing
// (except for pre_configure(), to advance the phase).
post_configure();
}

template<typename T2 = T,
typename = decltype(std::declval<T2>().generate())>
void call_configure_impl(double, int) {
// Generator has a generate() method but no configure() method. This is ok. Just advance the phase.
pre_configure();
static_assert(!has_configure_method<T2>::value, "Did not expect a configure method here.");
post_configure();
}

template<typename T2 = T,
typename = decltype(std::declval<T2>().generate()),
typename = decltype(std::declval<T2>().configure())>
void call_configure_impl(int, int) {
T *t = (T *)this;
static_assert(std::is_void<decltype(t->configure())>::value, "configure() must return void");
pre_configure();
t->configure();
post_configure();
}

// Implementations for call_generate_impl(), specialized on whether we
// have build() or configure()/generate()/schedule() methods.

void call_generate_impl(double) {
user_error << "Unimplemented";
}

template<typename T2 = T,
typename = decltype(std::declval<T2>().generate())>
void call_generate_impl(int) {
T *t = (T *)this;
static_assert(std::is_void<decltype(t->generate())>::value, "generate() must return void");
pre_generate();
t->generate();
post_generate();
}

// Implementations for call_schedule_impl(), specialized on whether we
// have build() or configure()generate()/schedule() methods.

void call_schedule_impl(double, double) {
user_error << "Unimplemented";
}

template<typename T2 = T,
typename = decltype(std::declval<T2>().generate())>
void call_schedule_impl(double, int) {
// Generator has a generate() method but no schedule() method. This is ok. Just advance the phase.
pre_schedule();
post_schedule();
}

template<typename T2 = T,
typename = decltype(std::declval<T2>().generate()),
typename = decltype(std::declval<T2>().schedule())>
void call_schedule_impl(int, int) {
T *t = (T *)this;
static_assert(std::is_void<decltype(t->schedule())>::value, "schedule() must return void");
pre_schedule();
t->schedule();
post_schedule();
}
#else
Pipeline build_pipeline_impl() {
T *t = (T *)this;
// No: configure() must be called prior to this
Expand Down Expand Up @@ -3886,26 +3750,8 @@ class Generator : public Internal::GeneratorBase {
}
post_schedule();
}
#endif

protected:
#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD
Pipeline build_pipeline() override {
return this->build_pipeline_impl(0);
}

void call_configure() override {
this->call_configure_impl(0, 0);
}

void call_generate() override {
this->call_generate_impl(0);
}

void call_schedule() override {
this->call_schedule_impl(0, 0);
}
#else
Pipeline build_pipeline() override {
return this->build_pipeline_impl();
}
Expand All @@ -3921,7 +3767,7 @@ class Generator : public Internal::GeneratorBase {
void call_schedule() override {
this->call_schedule_impl();
}
#endif

private:
friend void ::Halide::Internal::generator_test();
friend void ::Halide::Internal::generator_test();
Expand Down
4 changes: 0 additions & 4 deletions test/generator/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,6 @@ halide_define_aot_test(blur2x2)
# buffer_copy_generator.cpp
halide_define_aot_test(buffer_copy)

# buildmethod_aottest.cpp
# buildmethod_generator.cpp
halide_define_aot_test(buildmethod)

# can_use_target_aottest.cpp
# can_use_target_generator.cpp
halide_define_aot_test(can_use_target)
Expand Down
Loading