Merge branch 'main' of github.com:halide/Halide

.github/workflows/pip.yml

@@ -12,7 +12,7 @@ on:
 
 env:
   # TODO: detect this from repo somehow: https://github.com/halide/Halide/issues/8406
-  LLVM_VERSION: 19.1.0
+  LLVM_VERSION: 19.1.4
   FLATBUFFERS_VERSION: 23.5.26
   WABT_VERSION: 1.0.36
 

.lldbinit

@@ -0,0 +1 @@
+command script import ./tools/lldbhalide.py

README.md

@@ -436,3 +436,4 @@ code to Halide:
 |------------------------------------------|---------------------------------------------------------------------------------------------------------------|
 | [CMake developer](doc/CodeStyleCMake.md) | Guidelines for authoring new CMake code.                                                                      |
 | [FuzzTesting](doc/FuzzTesting.md)        | Information about fuzz testing the Halide compiler (rather than pipelines). Intended for internal developers. |
+| [Testing](doc/Testing.md)                | Information about our test organization and debugging tips. Intended for internal developers.                 |

doc/Testing.md

@@ -0,0 +1,125 @@
+# Testing
+
+Halide uses CTest as its primary test platform and runner.
+
+## Organization
+
+Halide's tests are organized beneath the top-level `test/` directory. These
+folders are described below:
+
+| Folder               | Description                                                                      |
+|----------------------|----------------------------------------------------------------------------------|
+| `autoschedulers/$AS` | Test for the `$AS` (e.g. `adams2019`) autoscheduler                              |
+| `common`             | Code that may be shared across multiple tests                                    |
+| `correctness`        | Tests that check correctness of various compiler properties                      |
+| `error`              | Tests that expect an exception to be thrown (or `abort()` to be called)          |
+| `failing_with_issue` | Correctness tests that are associated with a particular issue on GitHub          |
+| `fuzz`               | Fuzz tests. Read more at [FuzzTesting.md](FuzzTesting.md)                        |
+| `generator`          | Tests of Halide's AOT compilation infrastructure.                                |
+| `integration`        | Tests of Halide's CMake package for downstream use, including cross compilation. |
+| `performance`        | Tests that check that certain schedules indeed improve performance.              |
+| `runtime`            | Unit tests for the Halide runtime library                                        |
+| `warning`            | Tests that expected warnings are indeed issued.                                  |
+
+The tests in each of these directories are given CTest labels corresponding to
+the directory name. Thus, one can use `ctest -L generator` to run only the
+`generator` tests. The `performance` tests configure CTest to not run them
+concurrently with other tests (including each other).
+
+The vast majority of our tests are simple C++ executables that link to Halide,
+perform some checks, and print the special line `Success!` upon successful
+completion. There are three main exceptions to this:
+
+First, the `warning` tests are expected to print a line that reads
+`Warning:` and do not look for `Success!`.
+
+Second, some tests cannot run in all scenarios; for example, a test that
+measures CUDA performance requires a CUDA-capable GPU. In these cases, tests are
+expected to print `[SKIP]` and exit and not print `Success!` or `Warning:`.
+
+Finally, the `error` tests are expected to throw an (uncaught) exception that is
+not a `Halide::InternalError` (i.e. from a failing `internal_assert`). The logic
+for translating uncaught exceptions into successful tests is in
+`test/common/expect_abort.cpp`.
+
+## Debugging with LLDB
+
+We provide helpers for pretty-printing Halide's IR types in LLDB. The
+`.lldbinit` file at the repository root will load automatically if you launch
+`lldb` from this directory and your `~/.lldbinit` file contains the line,
+
+```
+settings set target.load-cwd-lldbinit true
+```
+
+If you prefer to avoid such global configuration, you can directly load the
+helpers with the LLDB command,
+
+```
+command script import ./tools/lldbhalide.py
+```
+
+again assuming that the repository root is your current working directory.
+
+To see the benefit of using these helpers, let us debug `correctness_bounds`:
+
+```
+$ lldb ./build/test/correctness/correctness_bounds
+(lldb) breakpoint set --file bounds.cpp --line 18
+Breakpoint 1: where = correctness_bounds`main + 864 at bounds.cpp:18:12, address = 0x0000000100002054
+(lldb) run
+Process 29325 launched: '/Users/areinking/dev/Halide/build/test/correctness/correctness_bounds' (arm64)
+Defining function...
+Process 29325 stopped
+* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
+    frame #0: 0x0000000100002054 correctness_bounds`main(argc=1, argv=0x000000016fdff160) at bounds.cpp:18:12
+   15       g(x, y) = min(x, y);
+   16       h(x, y) = clamp(x + y, 20, 100);
+   17   
+-> 18       Var xo("xo"), yo("yo"), xi("xi"), yi("yi");
+   19   
+   20       Target target = get_jit_target_from_environment();
+   21       if (target.has_gpu_feature()) {
+Target 0: (correctness_bounds) stopped.
+(lldb) 
+```
+
+Now we can try to inspect the Func `h`. Without the helpers, we see:
+
+```
+(lldb) v h
+(Halide::Func) {
+  func = {
+    contents = {
+      strong = (ptr = 0x0000600002486a20)
+      weak = nullptr
+      idx = 0
+    }
+  }
+  pipeline_ = {
+    contents = (ptr = 0x0000000000000000)
+  }
+}
+```
+
+But if we load the helpers and try again, we get a much more useful output:
+
+```
+(lldb) command script import ./tools/lldbhalide.py
+(lldb) v h
+... lots of output ...
+```
+
+The amount of output here is maybe a bit _too_ much, but we gain the ability to
+more narrowly inspect data about the func:
+
+```
+(lldb) v h.func.init_def.values
+...
+(std::vector<Halide::Expr>) h.func.init_def.values = size=1 {
+  [0] = max(min(x + y, 100), 20)
+}
+```
+
+These helpers are particularly useful when using graphical debuggers, such as
+the one found in CLion.

src/ApplySplit.cpp

@@ -11,13 +11,14 @@ using std::map;
 using std::string;
 using std::vector;
 
-vector<ApplySplitResult> apply_split(const Split &split, bool is_update, const string &prefix,
+vector<ApplySplitResult> apply_split(const Split &split, const string &prefix,
                                      map<string, Expr> &dim_extent_alignment) {
     vector<ApplySplitResult> result;
 
     Expr outer = Variable::make(Int(32), prefix + split.outer);
     Expr outer_max = Variable::make(Int(32), prefix + split.outer + ".loop_max");
-    if (split.is_split()) {
+    switch (split.split_type) {
+    case Split::SplitVar: {
         Expr inner = Variable::make(Int(32), prefix + split.inner);
         Expr old_max = Variable::make(Int(32), prefix + split.old_var + ".loop_max");
         Expr old_min = Variable::make(Int(32), prefix + split.old_var + ".loop_min");
@@ -129,8 +130,8 @@ vector<ApplySplitResult> apply_split(const Split &split, bool is_update, const s
         // Define the original variable as the base value computed above plus the inner loop variable.
         result.emplace_back(old_var_name, base_var + inner, ApplySplitResult::LetStmt);
         result.emplace_back(base_name, base, ApplySplitResult::LetStmt);
-
-    } else if (split.is_fuse()) {
+    } break;
+    case Split::FuseVars: {
         // Define the inner and outer in terms of the fused var
         Expr fused = Variable::make(Int(32), prefix + split.old_var);
         Expr inner_min = Variable::make(Int(32), prefix + split.inner + ".loop_min");
@@ -154,10 +155,12 @@ vector<ApplySplitResult> apply_split(const Split &split, bool is_update, const s
             outer_dim != dim_extent_alignment.end()) {
             dim_extent_alignment[split.old_var] = inner_dim->second * outer_dim->second;
         }
-    } else {
-        // rename or purify
+    } break;
+    case Split::RenameVar:
+    case Split::PurifyRVar:
         result.emplace_back(prefix + split.old_var, outer, ApplySplitResult::Substitution);
         result.emplace_back(prefix + split.old_var, outer, ApplySplitResult::LetStmt);
+        break;
     }
 
     return result;
@@ -173,7 +176,8 @@ vector<std::pair<string, Expr>> compute_loop_bounds_after_split(const Split &spl
     Expr old_var_extent = Variable::make(Int(32), prefix + split.old_var + ".loop_extent");
     Expr old_var_max = Variable::make(Int(32), prefix + split.old_var + ".loop_max");
     Expr old_var_min = Variable::make(Int(32), prefix + split.old_var + ".loop_min");
-    if (split.is_split()) {
+    switch (split.split_type) {
+    case Split::SplitVar: {
         Expr inner_extent = split.factor;
         Expr outer_extent = (old_var_max - old_var_min + split.factor) / split.factor;
         let_stmts.emplace_back(prefix + split.inner + ".loop_min", 0);
@@ -182,20 +186,25 @@ vector<std::pair<string, Expr>> compute_loop_bounds_after_split(const Split &spl
         let_stmts.emplace_back(prefix + split.outer + ".loop_min", 0);
         let_stmts.emplace_back(prefix + split.outer + ".loop_max", outer_extent - 1);
         let_stmts.emplace_back(prefix + split.outer + ".loop_extent", outer_extent);
-    } else if (split.is_fuse()) {
+    } break;
+    case Split::FuseVars: {
         // Define bounds on the fused var using the bounds on the inner and outer
         Expr inner_extent = Variable::make(Int(32), prefix + split.inner + ".loop_extent");
         Expr outer_extent = Variable::make(Int(32), prefix + split.outer + ".loop_extent");
         Expr fused_extent = inner_extent * outer_extent;
         let_stmts.emplace_back(prefix + split.old_var + ".loop_min", 0);
         let_stmts.emplace_back(prefix + split.old_var + ".loop_max", fused_extent - 1);
         let_stmts.emplace_back(prefix + split.old_var + ".loop_extent", fused_extent);
-    } else if (split.is_rename()) {
+    } break;
+    case Split::RenameVar:
         let_stmts.emplace_back(prefix + split.outer + ".loop_min", old_var_min);
         let_stmts.emplace_back(prefix + split.outer + ".loop_max", old_var_max);
         let_stmts.emplace_back(prefix + split.outer + ".loop_extent", old_var_extent);
+        break;
+    case Split::PurifyRVar:
+        // Do nothing for purify
+        break;
     }
-    // Do nothing for purify
 
     return let_stmts;
 }

src/ApplySplit.h

@@ -46,31 +46,6 @@ struct ApplySplitResult {
     ApplySplitResult(Expr val, Type t = Predicate)
         : name(""), value(std::move(val)), type(t) {
     }
-
-    bool is_substitution() const {
-        return (type == Substitution);
-    }
-    bool is_substitution_in_calls() const {
-        return (type == SubstitutionInCalls);
-    }
-    bool is_substitution_in_provides() const {
-        return (type == SubstitutionInProvides);
-    }
-    bool is_let() const {
-        return (type == LetStmt);
-    }
-    bool is_predicate() const {
-        return (type == Predicate);
-    }
-    bool is_predicate_calls() const {
-        return (type == PredicateCalls);
-    }
-    bool is_predicate_provides() const {
-        return (type == PredicateProvides);
-    }
-    bool is_blend_provides() const {
-        return (type == BlendProvides);
-    }
 };
 
 /** Given a Split schedule on a definition (init or update), return a list of
@@ -79,7 +54,7 @@ struct ApplySplitResult {
  * defined the values of variables referred by the predicates and substitutions
  * (ordered from innermost to outermost let). */
 std::vector<ApplySplitResult> apply_split(
-    const Split &split, bool is_update, const std::string &prefix,
+    const Split &split, const std::string &prefix,
     std::map<std::string, Expr> &dim_extent_alignment);
 
 /** Compute the loop bounds of the new dimensions resulting from applying the

src/AsyncProducers.cpp

@@ -543,20 +543,20 @@ class InitializeSemaphores : public IRMutator {
                 body = LetStmt::make(op->name, std::move(sema_allocate), std::move(body));
 
                 // Re-wrap any other lets
-                for (auto it = lets.rbegin(); it != lets.rend(); it++) {
-                    body = LetStmt::make(it->first, it->second, std::move(body));
+                for (const auto &[var, value] : reverse_view(lets)) {
+                    body = LetStmt::make(var, value, std::move(body));
                 }
             }
         } else {
             body = mutate(frames.back()->body);
         }
 
-        for (auto it = frames.rbegin(); it != frames.rend(); it++) {
-            Expr value = mutate((*it)->value);
-            if (value.same_as((*it)->value) && body.same_as((*it)->body)) {
-                body = *it;
+        for (const auto *frame : reverse_view(frames)) {
+            Expr value = mutate(frame->value);
+            if (value.same_as(frame->value) && body.same_as(frame->body)) {
+                body = frame;
             } else {
-                body = LetStmt::make((*it)->name, std::move(value), std::move(body));
+                body = LetStmt::make(frame->name, std::move(value), std::move(body));
             }
         }
         return body;
@@ -654,8 +654,8 @@ class TightenProducerConsumerNodes : public IRMutator {
                 body = make_producer_consumer(name, is_producer, body, scope, uses_vars);
             }
 
-            for (auto it = containing_lets.rbegin(); it != containing_lets.rend(); it++) {
-                body = LetStmt::make((*it)->name, (*it)->value, body);
+            for (const auto *container : reverse_view(containing_lets)) {
+                body = LetStmt::make(container->name, container->value, body);
             }
 
             return body;
@@ -846,8 +846,7 @@ class ExpandAcquireNodes : public IRMutator {
         result = mutate(result);
 
         vector<pair<Expr, Expr>> semaphores;
-        for (auto it = stmts.rbegin(); it != stmts.rend(); it++) {
-            Stmt s = *it;
+        for (Stmt s : reverse_view(stmts)) {
             while (const Acquire *a = s.as<Acquire>()) {
                 semaphores.emplace_back(a->semaphore, a->count);
                 s = a->body;
@@ -916,8 +915,8 @@ class ExpandAcquireNodes : public IRMutator {
         }
 
         // Rewrap the rest of the lets
-        for (auto it = frames.rbegin(); it != frames.rend(); it++) {
-            s = LetStmt::make((*it)->name, (*it)->value, s);
+        for (const auto *let : reverse_view(frames)) {
+            s = LetStmt::make(let->name, let->value, s);
         }
 
         return s;

src/BoundConstantExtentLoops.cpp

@@ -61,8 +61,8 @@ class BoundLoops : public IRMutator {
             if (e == nullptr) {
                 // We're about to hard fail. Get really aggressive
                 // with the simplifier.
-                for (auto it = lets.rbegin(); it != lets.rend(); it++) {
-                    extent = Let::make(it->first, it->second, extent);
+                for (const auto &[var, value] : reverse_view(lets)) {
+                    extent = Let::make(var, value, extent);
                 }
                 extent = remove_likelies(extent);
                 extent = substitute_in_all_lets(extent);

src/BoundSmallAllocations.cpp

@@ -38,8 +38,8 @@ class BoundSmallAllocations : public IRMutator {
 
         result = mutate(result);
 
-        for (auto it = frames.rbegin(); it != frames.rend(); it++) {
-            result = T::make(it->op->name, it->op->value, result);
+        for (const auto &frame : reverse_view(frames)) {
+            result = T::make(frame.op->name, frame.op->value, result);
         }
 
         return result;