Deprecate/remove Generator::get_externs_map() and friends (halide#6844)

* Deprecate/remove Generator::get_externs_map() and friends This is a feature of Generator that was added years ago to allow adding external code libraries in LLVM bitcode form (rather than simply as extern "C" or similar). In theory it allow for better codegen for external code modules (since LLVM has access to all the bitcode for optimization); in practice, we only know of one project that ever used it, and that project no longer exists. Additionally, it tended to be fairly flaky in terms of actual use -- e.g., missing symbols tended to crop up unpredictably. The issues with this feature are likely fixable, but since it hasn't (AFAICT) been used in ~years, we're better off deprecating it for Halide 15 and removing for Halide 16. (If anyone out there is still relying on this feature, obviously you should speak up ASAP.) * Also remove ExternalCode.h & friends * Also remove correctness/external_code.cpp * HALIDE_ALLOW_GENERATOR_EXTERNS_MAP -> HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
ardier · Mar 3, 2024 · 1278276 · 1278276
1 parent 420910c
commit 1278276
Show file tree

Hide file tree

Showing 18 changed files with 85 additions and 203 deletions.
diff --git a/src/AbstractGenerator.cpp b/src/AbstractGenerator.cpp
@@ -41,9 +41,11 @@ Module AbstractGenerator::build_module(const std::string &function_name) {
  }
 
  Module result = pipeline.compile_to_module(filter_arguments, function_name, context.target(), linkage_type);
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  for (const auto &map_entry : external_code_map()) {
  result.append(map_entry.second);
  }
+#endif
 
  for (const auto &a : arg_infos) {
  if (a.dir != ArgInfoDirection::Output) {
@@ -221,8 +223,10 @@ Module AbstractGenerator::build_gradient_module(const std::string &function_name
  }
 
  Module result = grad_pipeline.compile_to_module(gradient_inputs, function_name, context.target(), linkage_type);
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  user_assert(external_code_map().empty())
  << "Building a gradient-descent module for a Generator with ExternalCode is not supported.\n";
+#endif
 
  result.set_auto_scheduler_results(auto_schedule_results);
  return result;

diff --git a/src/AbstractGenerator.h b/src/AbstractGenerator.h
@@ -30,7 +30,9 @@ enum class ArgInfoKind { Scalar,
 enum class ArgInfoDirection { Input,
  Output };
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
 using ExternsMap = std::map<std::string, ExternalCode>;
+#endif
 
 /**
  * AbstractGenerator is an ABC that defines the API a Generator must provide
@@ -151,12 +153,14 @@ class AbstractGenerator {
  */
  virtual std::vector<Func> output_func(const std::string &name) = 0;
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  /** Return the ExternsMap for the Generator, if any.
  *
  * CALL-AFTER: build_pipeline()
  * CALL-BEFORE: n/a
  */
  virtual ExternsMap external_code_map() = 0;
+#endif
 
  /** Rebind a specified Input to refer to the given piece of IR, replacing the
  * default ImageParam / Param in place for that Input. Basic type-checking is

diff --git a/src/CodeGen_C.cpp b/src/CodeGen_C.cpp
@@ -1751,6 +1751,7 @@ void CodeGen_C::compile(const Module &input) {
  stream << "\n";
 
  if (!is_header_or_extern_decl()) {
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  // Emit any external-code blobs that are C++.
  for (const ExternalCode &code_blob : input.external_code()) {
  if (code_blob.is_c_plus_plus_source()) {
@@ -1762,6 +1763,7 @@ void CodeGen_C::compile(const Module &input) {
  stream << "\n";
  }
  }
+#endif
 
  add_vector_typedefs(type_info.vector_types_used);
 

diff --git a/src/CodeGen_LLVM.cpp b/src/CodeGen_LLVM.cpp
@@ -338,13 +338,15 @@ void CodeGen_LLVM::init_module() {
  module = get_initial_module_for_target(target, context);
 }
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
 void CodeGen_LLVM::add_external_code(const Module &halide_module) {
  for (const ExternalCode &code_blob : halide_module.external_code()) {
  if (code_blob.is_for_cpu_target(get_target())) {
  add_bitcode_to_module(context, *module, code_blob.contents(), code_blob.name());
  }
  }
 }
+#endif
 
 CodeGen_LLVM::~CodeGen_LLVM() {
  delete builder;
@@ -503,7 +505,9 @@ std::unique_ptr<llvm::Module> CodeGen_LLVM::compile(const Module &input) {
  internal_assert(module && context && builder)
  << "The CodeGen_LLVM subclass should have made an initial module before calling CodeGen_LLVM::compile\n";
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  add_external_code(input);
+#endif
 
  // Generate the code for this module.
  debug(1) << "Generating llvm bitcode...\n";

diff --git a/src/CodeGen_LLVM.h b/src/CodeGen_LLVM.h
@@ -177,8 +177,10 @@ class CodeGen_LLVM : public IRVisitor {
  * multiple related modules (e.g. multiple device kernels). */
  virtual void init_module();
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  /** Add external_code entries to llvm module. */
  void add_external_code(const Module &halide_module);
+#endif
 
  /** Run all of llvm's optimization passes on the module. */
  void optimize_module();

diff --git a/src/ExternalCode.h b/src/ExternalCode.h
@@ -1,6 +1,8 @@
 #ifndef HALIDE_EXTERNAL_CODE_H
 #define HALIDE_EXTERNAL_CODE_H
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
+
 #include <vector>
 
 #include "Expr.h"
@@ -131,4 +133,10 @@ class ExternalCode {
 
 } // namespace Halide
 
+#else
+
+#error "ExternalCode is deprecated in Halide 15 and will be removed in Halide 16"
+
+#endif // HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
+
 #endif
diff --git a/src/Generator.cpp b/src/Generator.cpp
@@ -19,6 +19,7 @@
 
 namespace Halide {
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
 GeneratorContext::GeneratorContext(const Target &target,
  bool auto_schedule,
  const MachineParams &machine_params,
@@ -28,18 +29,27 @@ GeneratorContext::GeneratorContext(const Target &target,
  machine_params_(machine_params),
  externs_map_(std::move(externs_map)) {
 }
+#endif
 
 GeneratorContext::GeneratorContext(const Target &target,
  bool auto_schedule,
  const MachineParams &machine_params)
- : GeneratorContext(target,
- auto_schedule,
- machine_params,
- std::make_shared<ExternsMap>()) {
+ : target_(target),
+ auto_schedule_(auto_schedule),
+ machine_params_(machine_params)
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
+ ,
+ externs_map_(std::make_shared<ExternsMap>())
+#endif
+{
 }
 
 GeneratorContext GeneratorContext::with_target(const Target &t) const {
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  return GeneratorContext(t, auto_schedule_, machine_params_, externs_map_);
+#else
+ return GeneratorContext(t, auto_schedule_, machine_params_);
+#endif
 }
 
 namespace Internal {
@@ -1292,15 +1302,21 @@ GeneratorOutputBase *GeneratorBase::find_output_by_name(const std::string &name)
 }
 
 GeneratorContext GeneratorBase::context() const {
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  return GeneratorContext(target, auto_schedule, machine_params, externs_map);
+#else
+ return GeneratorContext(target, auto_schedule, machine_params);
+#endif
 }
 
 void GeneratorBase::init_from_context(const Halide::GeneratorContext &context) {
  target.set(context.target_);
  auto_schedule.set(context.auto_schedule_);
  machine_params.set(context.machine_params_);
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  externs_map = context.externs_map_;
+#endif
 
  // pre-emptively build our param_info now
  internal_assert(param_info_ptr == nullptr);
@@ -1534,10 +1550,12 @@ std::vector<Func> GeneratorBase::output_func(const std::string &n) {
  return output->funcs();
 }
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
 ExternsMap GeneratorBase::external_code_map() {
  // get_externs_map() returns a std::shared_ptr<ExternsMap>
  return *get_externs_map();
 }
+#endif
 
 void GeneratorBase::bind_input(const std::string &name, const std::vector<Parameter> &v) {
  ensure_configure_has_been_called();

diff --git a/src/Generator.h b/src/Generator.h
@@ -272,7 +272,9 @@
 #include <vector>
 
 #include "AbstractGenerator.h"
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
 #include "ExternalCode.h"
+#endif
 #include "Func.h"
 #include "ImageParam.h"
 #include "Introspection.h"
@@ -3028,7 +3030,9 @@ class GeneratorContext {
 public:
  friend class Internal::GeneratorBase;
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  using ExternsMap = std::map<std::string, ExternalCode>;
+#endif
 
  explicit GeneratorContext(const Target &t,
  bool auto_schedule = false,
@@ -3083,12 +3087,16 @@ class GeneratorContext {
  Target target_;
  bool auto_schedule_ = false;
  MachineParams machine_params_ = MachineParams::generic();
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  std::shared_ptr<ExternsMap> externs_map_;
+#endif
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  GeneratorContext(const Target &target,
  bool auto_schedule,
  const MachineParams &machine_params,
  std::shared_ptr<ExternsMap> externs_map);
+#endif
 };
 
 class NamesInterface {
@@ -3514,6 +3522,8 @@ class GeneratorBase : public NamesInterface, public AbstractGenerator {
  MachineParams get_machine_params() const {
  return machine_params;
  }
+
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  /** Generators can register ExternalCode objects onto
  * themselves. The Generator infrastructure will arrange to have
  * this ExternalCode appended to the Module that is finally
@@ -3532,6 +3542,11 @@ class GeneratorBase : public NamesInterface, public AbstractGenerator {
  std::shared_ptr<GeneratorContext::ExternsMap> get_externs_map() const {
  return externs_map;
  }
+#else
+ /** ExternalCode objects in Generator are deprecated in Halide 15 and will
+ * be removed in Halide 16. You may continue to use them in Halide 15
+ * by defining HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE when building Halide. */
+#endif
 
  // These must remain here for legacy code that access the fields directly.
  GeneratorParam<Target> target{"target", Target()};
@@ -3548,7 +3563,9 @@ class GeneratorBase : public NamesInterface, public AbstractGenerator {
  friend class StubOutputBufferBase;
 
  const size_t size;
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  std::shared_ptr<GeneratorContext::ExternsMap> externs_map;
+#endif
 
  // Lazily-allocated-and-inited struct with info about our various Params.
  // Do not access directly: use the param_info() getter.
@@ -3746,7 +3763,9 @@ class GeneratorBase : public NamesInterface, public AbstractGenerator {
  std::vector<Parameter> input_parameter(const std::string &name) override;
  std::vector<Func> output_func(const std::string &name) override;
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  ExternsMap external_code_map() override;
+#endif
 
  // This is overridden in the concrete Generator<> subclass.
  // Pipeline build_pipeline() override;

diff --git a/src/Module.cpp b/src/Module.cpp
@@ -329,7 +329,9 @@ struct ModuleContents {
  std::vector<Buffer<>> buffers;
  std::vector<Internal::LoweredFunc> functions;
  std::vector<Module> submodules;
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  std::vector<ExternalCode> external_code;
+#endif
  MetadataNameMap metadata_name_map;
  bool any_strict_float{false};
  std::unique_ptr<AutoSchedulerResults> auto_scheduler_results;
@@ -415,9 +417,11 @@ const std::vector<Module> &Module::submodules() const {
  return contents->submodules;
 }
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
 const std::vector<ExternalCode> &Module::external_code() const {
  return contents->external_code;
 }
+#endif
 
 Internal::LoweredFunc Module::get_function_by_name(const std::string &name) const {
  for (const auto &f : functions()) {
@@ -441,9 +445,11 @@ void Module::append(const Module &module) {
  contents->submodules.push_back(module);
 }
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
 void Module::append(const ExternalCode &external_code) {
  contents->external_code.push_back(external_code);
 }
+#endif
 
 Module link_modules(const std::string &name, const std::vector<Module> &modules) {
  Module output(name, modules.front().target());
@@ -511,12 +517,15 @@ Module Module::resolve_submodules() const {
  for (const auto &buf : buffers()) {
  lowered_module.append(buf);
  }
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  for (const auto &ec : external_code()) {
  lowered_module.append(ec);
  }
+#endif
  for (const auto &m : submodules()) {
  Module copy(m.resolve_submodules());
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  // Propagate external code blocks.
  for (const auto &ec : external_code()) {
  // TODO(zalman): Is this the right thing to do?
@@ -531,6 +540,7 @@ Module Module::resolve_submodules() const {
  copy.append(ec);
  }
  }
+#endif
 
  auto buf = copy.compile_to_buffer();
  lowered_module.append(buf);

diff --git a/src/Module.h b/src/Module.h
@@ -13,7 +13,9 @@
 
 #include "Argument.h"
 #include "Expr.h"
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
 #include "ExternalCode.h"
+#endif
 #include "Function.h" // for NameMangling
 #include "ModulusRemainder.h"
 
@@ -161,7 +163,9 @@ class Module {
  const std::vector<Internal::LoweredFunc> &functions() const;
  std::vector<Internal::LoweredFunc> &functions();
  const std::vector<Module> &submodules() const;
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  const std::vector<ExternalCode> &external_code() const;
+#endif
  // @}
 
  /** Return the function with the given name. If no such function
@@ -173,7 +177,9 @@ class Module {
  void append(const Buffer<void> &buffer);
  void append(const Internal::LoweredFunc &function);
  void append(const Module &module);
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
  void append(const ExternalCode &external_code);
+#endif
  // @}
 
  /** Compile a halide Module to variety of outputs, depending on

diff --git a/src/Pipeline.h b/src/Pipeline.h
@@ -12,7 +12,9 @@
 #include <memory>
 #include <vector>
 
+#ifdef HALIDE_ALLOW_GENERATOR_EXTERNAL_CODE
 #include "ExternalCode.h"
+#endif
 #include "IROperator.h"
 #include "IntrusivePtr.h"
 #include "JITModule.h"

diff --git a/test/correctness/CMakeLists.txt b/test/correctness/CMakeLists.txt
@@ -95,7 +95,6 @@ tests(GROUPS correctness
  extern_reorder_storage.cpp
  extern_sort.cpp
  extern_stage_on_device.cpp
- external_code.cpp
  failed_unroll.cpp
  fast_trigonometric.cpp
  fibonacci.cpp