Skip to content

Commit

Permalink
Fix two minor bugs triggered by an or reduction with early-out (#6807)
Browse files Browse the repository at this point in the history
* Fix two minor bugs triggered by a or reduction with early-out

* Gotta print success

* Appease clang-tidy
  • Loading branch information
abadams authored Jun 14, 2022
1 parent ce75862 commit fc0f1f7
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/Bounds.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2640,7 +2640,7 @@ class BoxesTouched : public IRGraphVisitor {
void visit(const IfThenElse *op) override {
TRACK_BOXES_TOUCHED;
op->condition.accept(this);
if (expr_uses_vars(op->condition, scope)) {
if (expr_uses_vars(op->condition, scope) || !is_pure(op->condition)) {
// We need to simplify the condition to get it into a
// canonical form (e.g. (a < b) instead of !(a >= b))
vector<pair<Expr, Stmt>> cases;
Expand Down
5 changes: 5 additions & 0 deletions src/InferArguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ class InferArguments : public IRGraphVisitor {
}
}
}

// It also misses wrappers
for (const auto &p : func.wrappers()) {
Function(p.second).accept(this);
}
}

void include_parameter(const Parameter &p) {
Expand Down
1 change: 1 addition & 0 deletions test/correctness/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ tests(GROUPS correctness
dynamic_allocation_in_gpu_kernel.cpp
dynamic_reduction_bounds.cpp
embed_bitcode.cpp
early_out.cpp
erf.cpp
exception.cpp
explicit_inline_reductions.cpp
Expand Down
49 changes: 49 additions & 0 deletions test/correctness/early_out.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#include "Halide.h"

using namespace Halide;

int main(int argc, char **argv) {
// This is a test case that performs an or reduction using a where clause to
// get early-out behavior on the reduction loop. It triggered two bugs.
//
// First, there's a param that's only used in a specialization of a wrapper
// func, and this wasn't picked up by InferArguments.
//
// Second, there's a variable-free condition
// that feeds into bounds inference (test()), and bounds inference assumed
// that being variable-free meant it only depended on params and could be
// lifted out into a bounds expression.
//
// Both of these bugs caused compilation failures, so this test just
// verifies that things compile.

Param<int> height;

Var y;

Func test_rows("test_rows");
test_rows(y) = y < 100;

Func test("test");
test() = cast<bool>(false);
RDom ry(0, 1024);
ry.where(!test());
test() = test_rows(ry);

Func output;
output() = select(test(), cast<uint8_t>(0), cast<uint8_t>(1));

Expr num_slices = (height + 255) / 256;
Expr slice_size = (height + num_slices - 1) / num_slices;

test_rows.in()
.compute_root()
.specialize(height > slice_size)
.parallel(y, slice_size, TailStrategy::ShiftInwards);

output.compile_jit();

printf("Success!\n");

return 0;
}

0 comments on commit fc0f1f7

Please sign in to comment.