Skip to content

Commit

Permalink
Rename halide_assert -> halide_abort_if_false (#6382)
Browse files Browse the repository at this point in the history 
* Rename halide_assert -> HALIDDE_CHECK

A crashing bug got mistakenly inserted because a new contributor (reasonably) assumed that the `halide_assert()` macro in our runtime code was like a C `assert()` (i.e., something that would vanish in optimized builds).

This is not the case; it is a check that happens in all build modes and always triggers an `abort()` if it fires. We should remove any ambiguity about it, so this proposes to rename it to somethingmore like the Google/Abseil-style CHECK() macro, to make it stand out more.

(We may want to do a followup to verify that all of the uses really are unrecoverable errors that aren't better handled by returning an error.)

* clang-format

* Fix for top-of-tree LLVM

* Fix for older versions

* HALIDE_CHECK -> halide_abort_if_false

* Update runtime_internal.h
  • Loading branch information
@steven-johnson
steven-johnson committed Nov 8, 2021
1 parent 1312817 commit d6f1345
Show file tree
Hide file tree
Showing 16 changed files with 307 additions and 312 deletions.
12 changes: 6 additions & 6 deletions src/runtime/cache.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ WEAK void prune_cache() {
while (prev_hash_entry != nullptr && prev_hash_entry->next != prune_candidate) {
prev_hash_entry = prev_hash_entry->next;
}
halide_assert(nullptr, prev_hash_entry != nullptr);
halide_abort_if_false(nullptr, prev_hash_entry != nullptr);
prev_hash_entry->next = prune_candidate->next;
}

Expand Down Expand Up @@ -370,14 +370,14 @@ WEAK int halide_memoization_cache_lookup(void *user_context, const uint8_t *cach

if (all_bounds_equal) {
if (entry != most_recently_used) {
halide_assert(user_context, entry->more_recent != nullptr);
halide_abort_if_false(user_context, entry->more_recent != nullptr);
if (entry->less_recent != nullptr) {
entry->less_recent->more_recent = entry->more_recent;
} else {
halide_assert(user_context, least_recently_used == entry);
halide_abort_if_false(user_context, least_recently_used == entry);
least_recently_used = entry->more_recent;
}
halide_assert(user_context, entry->more_recent != nullptr);
halide_abort_if_false(user_context, entry->more_recent != nullptr);
entry->more_recent->less_recent = entry->less_recent;

entry->more_recent = nullptr;
Expand Down Expand Up @@ -469,7 +469,7 @@ WEAK int halide_memoization_cache_store(void *user_context, const uint8_t *cache
}
}
if (all_bounds_equal) {
halide_assert(user_context, no_host_pointers_equal);
halide_abort_if_false(user_context, no_host_pointers_equal);
// This entry is still in use by the caller. Mark it as having no cache entry
// so halide_memoization_cache_release can free the buffer.
for (int32_t i = 0; i < tuple_count; i++) {
Expand Down Expand Up @@ -547,7 +547,7 @@ WEAK void halide_memoization_cache_release(void *user_context, void *host) {
} else {
ScopedMutexLock lock(&memoization_lock);

halide_assert(user_context, entry->in_use_count > 0);
halide_abort_if_false(user_context, entry->in_use_count > 0);
entry->in_use_count--;
#if CACHE_DEBUGGING
validate_cache();
Expand Down
50 changes: 25 additions & 25 deletions src/runtime/cuda.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ ALWAYS_INLINE T get_cuda_symbol(void *user_context, const char *name, bool optio
// Load a CUDA shared object/dll and get the CUDA API function pointers from it.
WEAK void load_libcuda(void *user_context) {
debug(user_context) << " load_libcuda (user_context: " << user_context << ")\n";
halide_assert(user_context, cuInit == nullptr);
halide_abort_if_false(user_context, cuInit == nullptr);

// clang-format off
#define CUDA_FN(ret, fn, args) fn = get_cuda_symbol<ret(CUDAAPI *) args>(user_context, #fn); // NOLINT(bugprone-macro-parentheses)
Expand Down Expand Up @@ -139,10 +139,10 @@ extern "C" {
WEAK int halide_default_cuda_acquire_context(void *user_context, CUcontext *ctx, bool create = true) {
// TODO: Should we use a more "assertive" assert? these asserts do
// not block execution on failure.
halide_assert(user_context, ctx != nullptr);
halide_abort_if_false(user_context, ctx != nullptr);

// If the context has not been initialized, initialize it now.
halide_assert(user_context, &context != nullptr);
halide_abort_if_false(user_context, &context != nullptr);

// Note that this null-check of the context is *not* locked with
// respect to device_release, so we may get a non-null context
Expand Down Expand Up @@ -276,8 +276,8 @@ class Context {
// overridden, we may still need to load libcuda
ensure_libcuda_init(user_context);

halide_assert(user_context, context != nullptr);
halide_assert(user_context, cuInit != nullptr);
halide_abort_if_false(user_context, context != nullptr);
halide_abort_if_false(user_context, cuInit != nullptr);

error = cuCtxPushCurrent(context);
}
Expand Down Expand Up @@ -578,7 +578,7 @@ WEAK int halide_cuda_initialize_kernels(void *user_context, void **state_ptr, co
compile_kernel, user_context, ptx_src, size)) {
return halide_error_code_generic_error;
}
halide_assert(user_context, loaded_module != nullptr);
halide_abort_if_false(user_context, loaded_module != nullptr);

#ifdef DEBUG_RUNTIME
uint64_t t_after = halide_current_time_ns(user_context);
Expand Down Expand Up @@ -661,7 +661,7 @@ WEAK int halide_cuda_device_free(void *user_context, halide_buffer_t *buf) {
uint64_t t_before = halide_current_time_ns(user_context);
#endif

halide_assert(user_context, validate_device_pointer(user_context, buf));
halide_abort_if_false(user_context, validate_device_pointer(user_context, buf));

CUresult err = CUDA_SUCCESS;
if (halide_can_reuse_device_allocations(user_context)) {
Expand Down Expand Up @@ -735,7 +735,7 @@ WEAK int halide_cuda_device_release(void *user_context) {
if (err != CUDA_SUCCESS) {
err = cuCtxSynchronize();
}
halide_assert(user_context, err == CUDA_SUCCESS || err == CUDA_ERROR_DEINITIALIZED);
halide_abort_if_false(user_context, err == CUDA_SUCCESS || err == CUDA_ERROR_DEINITIALIZED);

// Dump the contents of the free list, ignoring errors.
halide_cuda_release_unused_device_allocations(user_context);
Expand All @@ -754,7 +754,7 @@ WEAK int halide_cuda_device_release(void *user_context) {
debug(user_context) << " cuCtxDestroy " << context << "\n";
err = cuProfilerStop();
err = cuCtxDestroy(context);
halide_assert(user_context, err == CUDA_SUCCESS || err == CUDA_ERROR_DEINITIALIZED);
halide_abort_if_false(user_context, err == CUDA_SUCCESS || err == CUDA_ERROR_DEINITIALIZED);
context = nullptr;
}
} // spinlock
Expand All @@ -779,16 +779,16 @@ WEAK int halide_cuda_device_malloc(void *user_context, halide_buffer_t *buf) {
if (halide_can_reuse_device_allocations(user_context)) {
size = quantize_allocation_size(size);
}
halide_assert(user_context, size != 0);
halide_abort_if_false(user_context, size != 0);
if (buf->device) {
// This buffer already has a device allocation
halide_assert(user_context, validate_device_pointer(user_context, buf, size));
halide_abort_if_false(user_context, validate_device_pointer(user_context, buf, size));
return 0;
}

// Check all strides positive.
for (int i = 0; i < buf->dimensions; i++) {
halide_assert(user_context, buf->dim[i].stride >= 0);
halide_abort_if_false(user_context, buf->dim[i].stride >= 0);
}

debug(user_context) << " allocating " << *buf << "\n";
Expand Down Expand Up @@ -877,7 +877,7 @@ WEAK int halide_cuda_device_malloc(void *user_context, halide_buffer_t *buf) {
debug(user_context) << (void *)p << "\n";
}
}
halide_assert(user_context, p);
halide_abort_if_false(user_context, p);
buf->device = p;
buf->device_interface = &cuda_device_interface;
buf->device_interface->impl->use_module();
Expand Down Expand Up @@ -957,12 +957,12 @@ WEAK int halide_cuda_buffer_copy(void *user_context, struct halide_buffer_t *src
const struct halide_device_interface_t *dst_device_interface,
struct halide_buffer_t *dst) {
// We only handle copies to cuda or to host
halide_assert(user_context, dst_device_interface == nullptr ||
dst_device_interface == &cuda_device_interface);
halide_abort_if_false(user_context, dst_device_interface == nullptr ||
dst_device_interface == &cuda_device_interface);

if ((src->device_dirty() || src->host == nullptr) &&
src->device_interface != &cuda_device_interface) {
halide_assert(user_context, dst_device_interface == &cuda_device_interface);
halide_abort_if_false(user_context, dst_device_interface == &cuda_device_interface);
// This is handled at the higher level.
return halide_error_code_incompatible_device_interface;
}
Expand All @@ -972,8 +972,8 @@ WEAK int halide_cuda_buffer_copy(void *user_context, struct halide_buffer_t *src
(src->host_dirty() && src->host != nullptr);
bool to_host = !dst_device_interface;

halide_assert(user_context, from_host || src->device);
halide_assert(user_context, to_host || dst->device);
halide_abort_if_false(user_context, from_host || src->device);
halide_abort_if_false(user_context, to_host || dst->device);

device_copy c = make_buffer_copy(src, from_host, dst, to_host);

Expand All @@ -991,10 +991,10 @@ WEAK int halide_cuda_buffer_copy(void *user_context, struct halide_buffer_t *src
#ifdef DEBUG_RUNTIME
uint64_t t_before = halide_current_time_ns(user_context);
if (!from_host) {
halide_assert(user_context, validate_device_pointer(user_context, src));
halide_abort_if_false(user_context, validate_device_pointer(user_context, src));
}
if (!to_host) {
halide_assert(user_context, validate_device_pointer(user_context, dst));
halide_abort_if_false(user_context, validate_device_pointer(user_context, dst));
}
#endif

Expand Down Expand Up @@ -1139,7 +1139,7 @@ WEAK int halide_cuda_run(void *user_context,

CUmodule mod{};
bool found = compilation_cache.lookup(ctx.context, state_ptr, mod);
halide_assert(user_context, found && mod != nullptr);
halide_abort_if_false(user_context, found && mod != nullptr);

debug(user_context) << "Got module " << mod << "\n";
CUfunction f;
Expand All @@ -1166,7 +1166,7 @@ WEAK int halide_cuda_run(void *user_context,
uint64_t *dev_handles = (uint64_t *)malloc(num_args * sizeof(uint64_t));
for (size_t i = 0; i <= num_args; i++) { // Get nullptr at end.
if (arg_is_buffer[i]) {
halide_assert(user_context, arg_sizes[i] == sizeof(uint64_t));
halide_abort_if_false(user_context, arg_sizes[i] == sizeof(uint64_t));
dev_handles[i] = ((halide_buffer_t *)args[i])->device;
translated_args[i] = &(dev_handles[i]);
debug(user_context) << " halide_cuda_run translated arg" << (int)i
Expand Down Expand Up @@ -1226,7 +1226,7 @@ WEAK int halide_cuda_device_and_host_free(void *user_context, struct halide_buff
}

WEAK int halide_cuda_wrap_device_ptr(void *user_context, struct halide_buffer_t *buf, uint64_t device_ptr) {
halide_assert(user_context, buf->device == 0);
halide_abort_if_false(user_context, buf->device == 0);
if (buf->device != 0) {
return -2;
}
Expand All @@ -1248,7 +1248,7 @@ WEAK int halide_cuda_detach_device_ptr(void *user_context, struct halide_buffer_
if (buf->device == 0) {
return 0;
}
halide_assert(user_context, buf->device_interface == &cuda_device_interface);
halide_abort_if_false(user_context, buf->device_interface == &cuda_device_interface);
buf->device_interface->impl->release_module();
buf->device = 0;
buf->device_interface = nullptr;
Expand All @@ -1259,7 +1259,7 @@ WEAK uintptr_t halide_cuda_get_device_ptr(void *user_context, struct halide_buff
if (buf->device == 0) {
return 0;
}
halide_assert(user_context, buf->device_interface == &cuda_device_interface);
halide_abort_if_false(user_context, buf->device_interface == &cuda_device_interface);
return (uintptr_t)buf->device;
}

Expand Down
Loading

0 comments on commit d6f1345

Please sign in to comment.