Autoscheduled code doesn't work on buffers smaller than estimates

[slepa]

1. Generate function

#include <Halide.h>
using namespace Halide;

int main() {
  ImageParam input(type_of<float>(), 2);
  Func f, in_b = BoundaryConditions::constant_exterior(input, 0.f);
  Var x, y, xo, xi;
  f(x, y) = 0.f;
  in_b.compute_at(f, xo);
  f.compute_root().split(x, xo, xi, 64);
  f.compile_to_static_library("make_zero", { input }, "make_zero");
}

2. Run function

#include "make_zero.h"
#include "HalideBuffer.h"

int main() {
  float in[64], out[64];
  Halide::Runtime::Buffer<float, 1> input(in, 8, 8), output(out, 8, 8);
  make_zero(input, output);
}

Error: Output buffer f1 is accessed at -56, which is before the min (0) in dimension 0
Aborted (core dumped)

Master 21f3be8 built with clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-18.04 by gcc 7.4.0-1ubuntu1~18.04._

[abadams]

This is not wrong code - this is the expected behavior. When you split something by a factor of 64 you can only evaluate at least 64 values of it, because you have an inner loop of size 64. By default we ensure the inner loop is constant-sized to enable things like vectorization and unrolling.

We recently changed the behavior of realize(int, int) to not require this, not by changing how the code is compiled, but by changing the implementation of realize to internally realize into a larger buffer and then return a cropped view of the result.

If you would like different behavior, there are two ways to get it:

1. Use split(x, xo, xi, 64, TailStrategy::GuardWithIf). This produces a tail case for the loop that only evaluates in-bounds points (it inserts an if statement inside the split loop).

2. Use Func::specialize to apply scheduling directives only in the case where the output is sufficiently large:

f.compute_root();
f.specialize(f.output_buffer().dim(0).extent() >= 64).split(x, xo, xi, 64);

I encourage you to use compile_to_lowered_stmt, or set the environment variable HL_DEBUG_CODEGEN=1 to see how this affects the generated code.

[slepa]

Thanks for the quick reply.

But this schedule was auto-generated.

The real problem is that for more complicated function auto-scheduler produce code that can crash on some image sizes, that is quite dangerous behavior.
I'm new to Halide. And after I read doc and tutorials, I expected that changing scheduler is safe to computation.

.realize doesn't produce this crash that makes testing harder.

[abadams]

Ah, I thought you were writing these by hand. There's a newer autoscheduler in apps/autoscheduler which is less likely to want large inputs/outputs, but in general autoschedulers may produce schedules that require the inputs/outputs to be at least the same size as the estimate given, which makes small inputs used for testing tricky.

The safe approach is to do the same thing that Func::realize now does - a bounds query. This is where you call the generated code with all your buffer host/device fields null. The min/extent fields of the buffers will get rewritten to the minimum size that isn't going to throw an error, and then the function returns without doing any actual computation. Then you allocate the inputs/outputs and call the generated function again to actually run the pipeline. It would look roughly like the generic version of this in the implementation of realize:

https://github.com/halide/Halide/blob/master/src/Pipeline.cpp#L644

So, roughly speaking:

  Halide::Runtime::Buffer<float> input(nullptr, 8, 8), output(nullptr, 8, 8);
  // bounds query to increase sizes to the minimum necessary
  make_zero(input, output);
  input.allocate();
  // the code that creates the input. No need to fill in values beyond the top left 8x8
  ...
  // Now allocate the output and run the pipeline for real
  output.allocate();
  make_zero(input, output);

[slepa]

The fastest user support I ever have.

[slepa]

Not a bug.

[abadams]

Fixing this could be a nice feature for a contributor though, so I'm going to retitle and repoen. To summarize:

The autoschedulers should ideally produce schedules that work for sizes smaller than the estimates given, including very small sizes, and there should be tests that verify this.

[steven-johnson]

This bug is likely much more important than the wording would suggest; I can think of many pipelines where estimates would be in the several-megapixel range, but plenty of inputs would be smaller than that; having pipelines that potentially fail for those would make use of the autoscheduler fraught with unexpected surprises for the unwary. (IMHO the goal should be for generated schedules to be "safe" for all input sizes; allowing for other sizes to fail would be better as an opt-in behavior.)

[abadams]

The new autoscheduler attempts to be safe on all input sizes (e.g. using guardwithif whenever something might touch the input or output).

[steven-johnson]

Ah, so this is only a known issue for the 'classic' autoscheduler? good to know.

[benoitsteiner]

I've stumbled on a strange autoscheduler bug: I've scheduled a large model by providing an input size estimate of 64 for the innermost dimension on a machine with avx instructions. I've then run the model with innermost dimension of 2 first, and then 15. In the first case, the code trigger an assertion stating that the innermost dimension was accessed at index 6, past the maximum of 1. This makes sense if the code was blindly vectorizing the innermost loop. However, in the second case I didn't trigger the assertion, thus indicating that the proper guards were in place. It looks like the code that's generated assumes that it can leverage an avx instruction at least once, and only subsequent loop iterations are guarded.