From 5e7fa09309d7434085c7a70e65c01319972fa80d Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Thu, 7 Apr 2022 12:12:04 -0700 Subject: [PATCH] Remove deprecated `build()` support from Generators This was deprecated in Halide 14; let's remove it entirely for Halide 15. --- src/Generator.cpp | 121 +----------------- src/Generator.h | 156 +---------------------- test/generator/CMakeLists.txt | 4 - test/generator/buildmethod_aottest.cpp | 34 ----- test/generator/buildmethod_generator.cpp | 46 ------- 5 files changed, 5 insertions(+), 356 deletions(-) delete mode 100644 test/generator/buildmethod_aottest.cpp delete mode 100644 test/generator/buildmethod_generator.cpp diff --git a/src/Generator.cpp b/src/Generator.cpp index 65c3bb2843c8..867eb46f1960 100644 --- a/src/Generator.cpp +++ b/src/Generator.cpp @@ -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, @@ -717,23 +721,10 @@ std::vector> GeneratorStub::generate(const GeneratorParamsMap std::vector> 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{output}); - } - } -#else internal_assert(!pi.outputs().empty()); for (auto *output : pi.outputs()) { v.push_back(get_outputs(output->name())); } -#endif return v; } @@ -1174,13 +1165,8 @@ 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 { @@ -1188,13 +1174,8 @@ void GeneratorParamBase::check_value_writable() const { 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) { @@ -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()) { @@ -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. @@ -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 { - public: - GeneratorParam gp0{"gp0", 0}; - GeneratorParam gp1{"gp1", 1.f}; - GeneratorParam gp2{"gp2", 2}; - GeneratorParam gp_uint8{"gp_uint8", 65}; - GeneratorParam gp_int8{"gp_int8", 66}; - GeneratorParam gp_char{"gp_char", 97}; - GeneratorParam gp_schar{"gp_schar", 98}; - GeneratorParam gp_uchar{"gp_uchar", 99}; - GeneratorParam gp_bool{"gp_bool", true}; - - Input 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 { diff --git a/src/Generator.h b/src/Generator.h index 98dc0940a7c5..4e7ec193304e 100644 --- a/src/Generator.h +++ b/src/Generator.h @@ -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 with dynamic type & dimensions templateGenerateCalled 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, @@ -3684,14 +3668,6 @@ class Generator : public Internal::GeneratorBase { template 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::value, "apply() is not supported for old-style Generators."); -#endif -#endif call_configure(); set_inputs(args...); call_generate(); @@ -3734,118 +3710,6 @@ class Generator : public Internal::GeneratorBase { template struct has_schedule_method().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::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::value, "The configure() method is ignored if you define a build() method; use generate() instead."); - static_assert(!has_schedule_method::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().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().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::value, "Did not expect a configure method here."); - post_configure(); - } - - template().generate()), - typename = decltype(std::declval().configure())> - void call_configure_impl(int, int) { - T *t = (T *)this; - static_assert(std::is_voidconfigure())>::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().generate())> - void call_generate_impl(int) { - T *t = (T *)this; - static_assert(std::is_voidgenerate())>::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().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().generate()), - typename = decltype(std::declval().schedule())> - void call_schedule_impl(int, int) { - T *t = (T *)this; - static_assert(std::is_voidschedule())>::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 @@ -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(); } @@ -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(); diff --git a/test/generator/CMakeLists.txt b/test/generator/CMakeLists.txt index 48cd8c333230..bef502f21461 100644 --- a/test/generator/CMakeLists.txt +++ b/test/generator/CMakeLists.txt @@ -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) diff --git a/test/generator/buildmethod_aottest.cpp b/test/generator/buildmethod_aottest.cpp deleted file mode 100644 index 3eca8be57764..000000000000 --- a/test/generator/buildmethod_aottest.cpp +++ /dev/null @@ -1,34 +0,0 @@ -#include "HalideBuffer.h" -#include "HalideRuntime.h" - -#include -#include - -#include "buildmethod.h" - -using namespace Halide::Runtime; - -const int kSize = 32; - -int main(int argc, char **argv) { - Buffer input(kSize, kSize, 3); - Buffer output(kSize, kSize, 3); - - const float compiletime_factor = 1.0f; - const float runtime_factor = 3.25f; - - input.for_each_element([&](int x, int y, int c) { - input(x, y, c) = std::max(x, y) * c; - }); - - buildmethod(input, runtime_factor, output); - - output.for_each_element([=](int x, int y, int c) { - int expected = (int32_t)(compiletime_factor * runtime_factor * c * std::max(x, y)); - int actual = output(x, y, c); - assert(expected == actual); - }); - - printf("Success!\n"); - return 0; -} diff --git a/test/generator/buildmethod_generator.cpp b/test/generator/buildmethod_generator.cpp deleted file mode 100644 index d56f7db26ecc..000000000000 --- a/test/generator/buildmethod_generator.cpp +++ /dev/null @@ -1,46 +0,0 @@ -#include "Halide.h" - -namespace { - -#ifdef HALIDE_ALLOW_GENERATOR_BUILD_METHOD -// This Generator exists solely to test old-style generators (using the -// build() method, rather than generate()/schedule()). -// Do not convert it to new-style until/unless we decide to entirely remove support -// for those Generators. -class BuildMethod : public Halide::Generator { -public: - GeneratorParam compiletime_factor{"compiletime_factor", 1, 0, 100}; - - Input> input{"input"}; - Input runtime_factor{"runtime_factor", 1.0}; - - Func build() { - Var x, y, c; - - Func g; - g(x, y, c) = cast(input(x, y, c) * compiletime_factor * runtime_factor); - return g; - } -}; -#else -// Provide a placeholder here that uses generate(), just to allow this test to -// succeed even if build() is disabled. -class BuildMethod : public Halide::Generator { -public: - GeneratorParam compiletime_factor{"compiletime_factor", 1, 0, 100}; - - Input> input{"input"}; - Input runtime_factor{"runtime_factor", 1.0}; - Output> output{"output"}; - - void generate() { - Var x, y, c; - - output(x, y, c) = cast(input(x, y, c) * compiletime_factor * runtime_factor); - } -}; -#endif - -} // namespace - -HALIDE_REGISTER_GENERATOR(BuildMethod, buildmethod)