Merge pull request #1 from halide/master

sync with master
jn80842 · Jul 17, 2018 · 69cf6fd · 69cf6fd
2 parents cab1adb + 7bc00ba
commit 69cf6fd
Show file tree

Hide file tree

Showing 20 changed files with 300 additions and 43 deletions.
diff --git a/python_bindings/Makefile b/python_bindings/Makefile
@@ -109,15 +109,22 @@ $(BIN)/runtime.gen: $(HALIDE_PATH)/tools/GenGen.cpp $(LIBHALIDE)
 	@mkdir -p $(@D)
 	$(CXX) $(CCFLAGS) $(LDFLAGS) $^ -o $@
 
-# Make the generator generate a Python extension:
-$(BIN)/%.py.c $(BIN)/%.cpp $(BIN)/%.h: $(BIN)/%.gen
-	$< -e h,cpp,py.c -g $(notdir $(basename $<)) -o $(BIN) target=host-no_runtime
-
 # Generate a runtime:
 $(BIN)/runtime.a: $(BIN)/runtime.gen
 	@mkdir -p $(@D)
 	$< -r runtime -o $(BIN) target=host
 
+# Which target features to use for which test targets.
+target_features_addconstant=-legacy_buffer_wrappers-no_runtime
+target_features_bit=-no_runtime
+target_features_user_context=-user_context-legacy_buffer_wrappers-no_runtime
+
+# Make the generator generate a Python extension:
+$(BIN)/%.py.c $(BIN)/%.cpp $(BIN)/%.h: $(BIN)/%.gen
+	LD_LIBRARY_PATH=$(HALIDE_DISTRIB_PATH)/bin $< -e h,cpp,py.c \
+	    -g $(notdir $(basename $<)) -o $(BIN) \
+	    target=host$(target_features_$(notdir $(basename $<)))
+
 # Compile the generated Python extension(s):
 $(BIN)/%.o: $(BIN)/%.cpp
 	$(CXX) -c $(FPIC) $(CCFLAGS) $^ -o $@
@@ -137,9 +144,10 @@ $(BIN)/ext/%.so: $(BIN)/%.py.o $(BIN)/%.o $(BIN)/runtime.a
 
 # TODO: In the optimal case, we'd do %.run on all our generators. Unfortunately,
 # every generator needs its own settings. See https://github.com/halide/Halide/issues/2977.
-.PHONY: test_correctness_bit_test test_correctness_addconstant_test test_correctness_pystub
+.PHONY: test_correctness_bit_test test_correctness_addconstant_test test_correctness_pystub test_correctness_user_context
 test_correctness_addconstant_test: addconstant.run ;
 test_correctness_bit_test: bit.run ;
+test_correctness_user_context_test: user_context.run ;
 test_correctness_pystub: $(BIN)/simplestub.so $(BIN)/complexstub.so $(BIN)/buildmethod.so $(BIN)/partialbuildmethod.so $(BIN)/nobuildmethod.so
 
 APPS = $(shell ls $(ROOT_DIR)/apps/*.py)

diff --git a/python_bindings/correctness/user_context_generator.cpp b/python_bindings/correctness/user_context_generator.cpp
@@ -0,0 +1,20 @@
+#include "Halide.h"
+
+using namespace Halide;
+
+class UserContextGenerator : public Halide::Generator<UserContextGenerator> {
+public:
+    Input<uint8_t> constant{"constant"};
+    Output<Buffer<uint8_t>> output{"output", 1};
+
+    Var x;
+
+    void generate() {
+        output(x) = constant;
+    }
+
+    void schedule() {
+    }
+};
+
+HALIDE_REGISTER_GENERATOR(UserContextGenerator, user_context)
diff --git a/python_bindings/correctness/user_context_test.py b/python_bindings/correctness/user_context_test.py
@@ -0,0 +1,12 @@
+import array
+import user_context
+
+
+def test():
+    output = bytearray("\0\0\0\0", "ascii")
+    user_context.user_context(None, ord('q'), output)
+    assert output == bytearray("qqqq", "ascii")
+
+
+if __name__ == "__main__":
+    test()
diff --git a/src/AutoSchedule.cpp b/src/AutoSchedule.cpp
@@ -2517,7 +2517,7 @@ void Partitioner::reorder_dims(Stage f_handle, int stage_num, Definition def,
                                map<string, Expr> strides, AutoSchedule &sched) {
     vector<Dim> &dims = def.schedule().dims();
     internal_assert(dims.size() > 1);
-    vector<pair<string, bool>> order;
+    vector<pair<string, int>> order;
 
     for (int d = 0; d < (int)dims.size() - 1; d++) {
         internal_assert(strides.find(dims[d].var) != strides.end());

diff --git a/src/Bounds.cpp b/src/Bounds.cpp
@@ -339,8 +339,10 @@ class Bounds : public IRVisitor {
 
         if (a.is_single_point(op->a) && b.is_single_point(op->b)) {
             interval = Interval::single_point(op);
+            return;
         } else if (a.is_single_point() && b.is_single_point()) {
             interval = Interval::single_point(a.min * b.min);
+            return;
         } else if (b.is_single_point()) {
             Expr e1 = a.has_lower_bound() ? a.min * b.min : a.min;
             Expr e2 = a.has_upper_bound() ? a.max * b.min : a.max;

diff --git a/src/Generator.cpp b/src/Generator.cpp
@@ -1277,9 +1277,12 @@ void GeneratorBase::track_parameter_values(bool include_outputs) {
                 internal_assert(!output->funcs().empty());
                 for (auto &f : output->funcs()) {
                     user_assert(f.defined()) << "Output " << output->name() << " is not fully defined.";
-                    Parameter p = f.output_buffer().parameter();
-                    // This must use p.name(), *not* output->name()
-                    get_value_tracker()->track_values(p.name(), parameter_constraints(p));
+                    auto output_buffers = f.output_buffers();
+                    for (auto &o : output_buffers) {
+                        Parameter p = o.parameter();
+                        // This must use p.name(), *not* output->name()
+                        get_value_tracker()->track_values(p.name(), parameter_constraints(p));
+                    }
                 }
             }
         }

diff --git a/src/Generator.h b/src/Generator.h
@@ -2074,18 +2074,21 @@ class GeneratorOutput_Buffer : public GeneratorOutputImpl<T> {
 
         internal_assert(f.defined());
 
-        const auto &output_types = f.output_types();
-        user_assert(output_types.size() == 1)
-            << "Output " << this->name() << " should have size=1 but saw size=" << output_types.size() << "\n";
-
-        Buffer<> other(output_types.at(0), nullptr, std::vector<int>(f.dimensions(), 1));
-        user_assert(T::can_convert_from(other))
-            << "Cannot assign to the Output \"" << this->name()
-            << "\": the expression is not convertible to the same Buffer type and/or dimensions.\n";
+        if (TBase::has_static_halide_type) {
+            Buffer<> other(f.output_types().at(0), nullptr, std::vector<int>(f.dimensions(), 1));
+            user_assert(T::can_convert_from(other))
+                << "Cannot assign to the Output \"" << this->name()
+                << "\": the expression is not convertible to the same Buffer type and/or dimensions.\n";
+        }
 
         if (this->types_defined()) {
-            user_assert(output_types.at(0) == this->type())
-                << "Output " << this->name() << " should have type=" << this->type() << " but saw type=" << output_types.at(0) << "\n";
+            const auto &my_types = this->types();
+            user_assert(my_types.size() == f.output_types().size())
+                << "Output " << this->name() << " requires a Func with " << my_types.size() << " type(s) but tried to assign one with " << f.output_types().size() << " type(s)\n";
+            for (size_t i = 0; i < my_types.size(); i++) {
+                user_assert(my_types[i] == f.output_types().at(i))
+                    << "Output " << this->name() << " should have type[" << i << "]=" << my_types[i] << " but saw type[" << i << "]=" << f.output_types().at(i) << "\n";
+            }
         }
         if (this->dims_defined()) {
             user_assert(f.dimensions() == this->dims())
@@ -2100,19 +2103,21 @@ class GeneratorOutput_Buffer : public GeneratorOutputImpl<T> {
 protected:
     using TBase = typename Super::TBase;
 
-    static std::vector<Type> my_types() {
-        return TBase::has_static_halide_type ? std::vector<Type>{ TBase::static_halide_type() } : std::vector<Type>{};
+    static std::vector<Type> my_types(const std::vector<Type> &t) {
+        if (TBase::has_static_halide_type) {
+            user_assert(t.empty()) << "Cannot pass a Type argument for an Output<Buffer> with a non-void static type\n";
+            return std::vector<Type>{ TBase::static_halide_type() };
+        }
+        return t;
     }
 
 protected:
     GeneratorOutput_Buffer(const std::string &name, const std::vector<Type> &t = {}, int d = -1)
-        : Super(name, IOKind::Buffer, my_types(), d) {
-        user_assert(t.empty()) << "You cannot specify a Type argument for Output<Buffer<>>\n";
+        : Super(name, IOKind::Buffer, my_types(t), d) {
     }
 
     GeneratorOutput_Buffer(size_t array_size, const std::string &name, const std::vector<Type> &t = {}, int d = -1)
-        : Super(array_size, name, IOKind::Buffer, my_types(), d) {
-        user_assert(t.empty()) << "You cannot specify a Type argument for Output<Buffer<>>\n";
+        : Super(array_size, name, IOKind::Buffer, my_types(t), d) {
     }
 
     HALIDE_NO_USER_CODE_INLINE std::string get_c_type() const override {

diff --git a/src/PythonExtensionGen.cpp b/src/PythonExtensionGen.cpp
@@ -47,7 +47,16 @@ static bool has_legacy_buffers(const LoweredFunc& func) {
 }
 
 static bool can_convert(const LoweredArgument* arg) {
-  if (arg->type.is_handle() || arg->type.is_vector()) {
+  if (arg->type.is_handle()) {
+      if (arg->name == "__user_context") {
+          /* __user_context is a void* pointer to a user supplied memory region.
+           * We allow the Python callee to pass PyObject* pointers to that. */
+          return true;
+      } else {
+          return false;
+      }
+  }
+  if (arg->type.is_vector()) {
       return false;
   }
   if (arg->type.is_float() && arg->type.bits() != 32 && arg->type.bits() != 64) {
@@ -68,10 +77,13 @@ static bool can_convert(const LoweredArgument* arg) {
 
 std::pair<string, string> print_type(const LoweredArgument* arg) {
     // Excluded by can_convert() above:
-    assert(!arg->type.is_handle());
     assert(!arg->type.is_vector());
 
-    if (arg->is_buffer()) {
+    if (arg->type.is_handle()) {
+        /* Handles can be any pointer. However, from Python, all you can pass to
+         * a function is a PyObject*, so we can restrict to that. */
+        return std::make_pair("O", "PyObject*");
+    } else if (arg->is_buffer()) {
         return std::make_pair("O", "PyObject*");
     } else if (arg->type.is_float() && arg->type.bits() == 32) {
         return std::make_pair("f", "float");

diff --git a/src/UnpackBuffers.cpp b/src/UnpackBuffers.cpp
@@ -23,7 +23,6 @@ class FindBufferSymbols : public IRVisitor {
 
     void visit_param(const string &ref_name, const Parameter &param) {
         if (param.defined() && param.is_buffer()) {
-            symbols.insert(ref_name);
             string name = param.name();
             buffers[name] =
                 BufferInfo {Variable::make(type_of<buffer_t *>(), name + ".buffer", param),
@@ -33,7 +32,6 @@ class FindBufferSymbols : public IRVisitor {
 
     void visit_buffer(const string &ref_name, const Buffer<> &buffer) {
         if (buffer.defined()) {
-            symbols.insert(ref_name);
             string name = buffer.name();
             buffers[name] =
                 BufferInfo {Variable::make(type_of<buffer_t *>(), name + ".buffer", buffer),
@@ -44,16 +42,19 @@ class FindBufferSymbols : public IRVisitor {
     void visit(const Variable *op) {
         visit_param(op->name, op->param);
         visit_buffer(op->name, op->image);
+        symbols.insert(op->name);
     }
 
     void visit(const Load *op) {
         visit_param(op->name, op->param);
         visit_buffer(op->name, op->image);
+        symbols.insert(op->name);
         IRVisitor::visit(op);
     }
 
     void visit(const Store *op) {
         visit_param(op->name, op->param);
+        symbols.insert(op->name);
         IRVisitor::visit(op);
     }
 

diff --git a/src/runtime/printer.h b/src/runtime/printer.h
@@ -44,7 +44,12 @@ class Printer {
     }
 
     Printer &operator<<(const char *arg) {
-        dst = halide_string_to_string(dst, end, arg);
+        // Crashing on NULL here is a big debugging time sink.
+        if (arg == NULL) {
+            dst = halide_string_to_string(dst, end, "<NULL>");
+        } else {
+            dst = halide_string_to_string(dst, end, arg);
+        }
         return *this;
     }
 

diff --git a/test/correctness/bounds_of_multiply.cpp b/test/correctness/bounds_of_multiply.cpp
@@ -0,0 +1,33 @@
+// See https://github.com/halide/Halide/issues/3070
+
+#include <stdio.h>
+
+#include "Halide.h"
+
+using namespace Halide;
+
+template <typename T>
+void test() {
+    Param<T> bound;
+    ImageParam in(UInt(8), 1);
+    Var x;
+    Func f;
+
+    f(x) = in(clamp(x, 0, bound * 2 - 1));
+
+    Buffer<uint8_t> foo(10);
+    foo.fill(0);
+    in.set(foo);
+    bound.set(5);
+
+    auto result = f.realize(200);
+}
+
+int main(int argc, char **argv) {
+    printf("Trying int32_t\n");
+    test<int32_t>();
+    printf("Trying int16_t\n");
+    test<int16_t>();
+    printf("Success!\n");
+    return 0;
+}
diff --git a/test/correctness/for_each_element.cpp b/test/correctness/for_each_element.cpp
@@ -1,5 +1,7 @@
 #include "HalideBuffer.h"
 
+#include <stdio.h>
+
 using namespace Halide::Runtime;
 
 int main(int argc, char **argv) {

diff --git a/test/correctness/halide_buffer.cpp b/test/correctness/halide_buffer.cpp
@@ -1,6 +1,8 @@
 // Don't include Halide.h: it is not necessary for this test.
 #include "HalideBuffer.h"
 
+#include <stdio.h>
+
 using namespace Halide::Runtime;
 
 template<typename T1, typename T2>