Uses of `halide_assert`/`HALIDE_CHECK` in runtime should be audited

[steven-johnson]

See https://github.com/halide/Halide/pull/6382 and https://github.com/halide/Halide/pull/6381 -- the existing halide_assert macro in runtime_internal.h is not like the C assert() macro, in that halide_assert() is never compiled away, but lots of code in our runtime uses it in overly-generous ways that suggest that perhaps this wasn't clear to everyone.

6382 proposes a mechanical rename to make this behavior more clear, but IMHO we need more work here:

• We should add something like HALIDE_DEBUG_ASSERT() halide_debug_assert() to runtime_internal, which would be more like assert() in that it would be compiled away entirely unless DEBUG_RUNTIME is defined.
• All uses of halide_assert()/HALIDE_CHECK() should be audited:
  • Many (most?) should probably be converted to use halide_debug_assert(), as they are clearly not firing in any reasonable failure mode (otherwise we'd have massive complaints about Halide-using apps aborting in odd conditions), and are just a waste of code space
  • Some should perhaps be converted to a runtime-check-and-return-error (if it is a recoverable error)
  • Some should remain as HALIDE_CHECK() (ie, errors which are unrecoverable and aborting is better than continuing)

[mcourteaux]

I'd like to add that in my experience, I run into quite a lot of halide_assert()s, during execution of generators. These are most likely Halide bugs, so I'd like to suggest to NEVER compile them away when dealing with generators.

[steven-johnson]

It would be quite extraordinary to have halide_assert (now renamed halide_abort_if_false) firing during Generator execution. Are you sure you don't mean internal_assert or user_assert?

[mcourteaux]

You are right, I think! Was running into internal_asserts() a few times.

[steven-johnson]

Note that the old halide_assert() was renamed to halide_abort_if_false(), but the underlying issue remains that I think we call it far too aggressively: it should really be reserved for truly unrecoverable errors, in which the host app should be taken down for its own safety. We should prefer to do the usual runtime error handling (i.e.: call error() and return a nonzero code) in the vast majority of cases, I suspect.