From ed0619ce9a3a5b4f3b6c1acee67925572d0112f1 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Wed, 3 Nov 2021 14:10:23 -0700 Subject: [PATCH 1/3] Annotate all runtime functions with HALIDE_MUST_USE_RESULT All the halide runtime functions (and function pointers) that return interesting status results (int or bool) should be annotated with HALIDE_MUST_USE_RESULT (we have evidence that ignoring return codes has led to hard-to-diagnose errors for downstream users). Also update all the internal call sites to report and/or ignore errors as appropriate. --- apps/lens_blur/process.cpp | 2 +- src/CodeGen_LLVM.cpp | 25 ++- src/runtime/HalideBuffer.h | 11 +- src/runtime/HalideRuntime.h | 192 +++++++++--------- src/runtime/cache.cpp | 5 +- src/runtime/device_interface.cpp | 4 +- src/runtime/halide_buffer_t.cpp | 21 +- src/runtime/matlab.cpp | 10 +- src/runtime/msan.cpp | 8 +- src/runtime/tracing.cpp | 2 +- src/runtime/write_debug_image.cpp | 5 +- test/generator/buffer_copy_aottest.cpp | 6 +- .../generator/gpu_object_lifetime_aottest.cpp | 6 +- .../memory_profiler_mandelbrot_aottest.cpp | 4 +- .../user_context_insanity_aottest.cpp | 7 +- tools/RunGenMain.cpp | 5 +- 16 files changed, 183 insertions(+), 130 deletions(-) diff --git a/apps/lens_blur/process.cpp b/apps/lens_blur/process.cpp index 8002751705e6..d09a26ac0956 100644 --- a/apps/lens_blur/process.cpp +++ b/apps/lens_blur/process.cpp @@ -21,7 +21,7 @@ int main(int argc, char **argv) { // Let the Halide runtime hold onto GPU allocations for // intermediates and reuse them instead of eagerly freeing // them. cuMemAlloc/cuMemFree is slower than the algorithm! - halide_reuse_device_allocations(nullptr, true); + (void)halide_reuse_device_allocations(nullptr, true); // ignore errors Buffer left_im = load_image(argv[1]); Buffer right_im = load_image(argv[1]); diff --git a/src/CodeGen_LLVM.cpp b/src/CodeGen_LLVM.cpp index cf9e8e15bcf5..0b677a2ec5b0 100644 --- a/src/CodeGen_LLVM.cpp +++ b/src/CodeGen_LLVM.cpp @@ -1159,21 +1159,30 @@ void CodeGen_LLVM::optimize_module() { #endif pb.registerOptimizerLastEPCallback( [](ModulePassManager &mpm, OptimizationLevel level) { +#if LLVM_VERSION >= 140 + AddressSanitizerOptions asan_options; // default values are good... + asan_options.UseAfterScope = true; // ... except this one + mpm.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerPass(asan_options))); +#else constexpr bool compile_kernel = false; constexpr bool recover = false; constexpr bool use_after_scope = true; -#if LLVM_VERSION >= 140 - // TODO: this is the value that the default ctor uses. - // Not sure if it's ideal for Halide. - constexpr AsanDetectStackUseAfterReturnMode use_after_return = AsanDetectStackUseAfterReturnMode::Runtime; - mpm.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerPass( - AddressSanitizerOptions{compile_kernel, recover, use_after_scope, use_after_return}))); -#else mpm.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerPass( compile_kernel, recover, use_after_scope))); #endif }); -#if LLVM_VERSION >= 120 +#if LLVM_VERSION >= 140 + pb.registerPipelineStartEPCallback( + [](ModulePassManager &mpm, OptimizationLevel) { + AddressSanitizerOptions asan_options; // default values are good + constexpr bool use_global_gc = true; + constexpr bool use_odr_indicator = true; + constexpr auto destructor_kind = AsanDtorKind::Global; + mpm.addPass(ModuleAddressSanitizerPass( + asan_options, use_global_gc, + use_odr_indicator, destructor_kind)); + }); +#elif LLVM_VERSION >= 120 pb.registerPipelineStartEPCallback( [](ModulePassManager &mpm, OptimizationLevel) { constexpr bool compile_kernel = false; diff --git a/src/runtime/HalideBuffer.h b/src/runtime/HalideBuffer.h index 86fe6c05c0b4..aa51e5f2edf1 100644 --- a/src/runtime/HalideBuffer.h +++ b/src/runtime/HalideBuffer.h @@ -240,15 +240,18 @@ class Buffer { "Call device_free explicitly if you want to drop dirty device-side data. " "Call copy_to_host explicitly if you want the data copied to the host allocation " "before the device allocation is freed."); + int result = 0; if (dev_ref_count && dev_ref_count->ownership == BufferDeviceOwnership::WrappedNative) { - buf.device_interface->detach_native(nullptr, &buf); + result = buf.device_interface->detach_native(nullptr, &buf); } else if (dev_ref_count && dev_ref_count->ownership == BufferDeviceOwnership::AllocatedDeviceAndHost) { - buf.device_interface->device_and_host_free(nullptr, &buf); + result = buf.device_interface->device_and_host_free(nullptr, &buf); } else if (dev_ref_count && dev_ref_count->ownership == BufferDeviceOwnership::Cropped) { - buf.device_interface->device_release_crop(nullptr, &buf); + result = buf.device_interface->device_release_crop(nullptr, &buf); } else if (dev_ref_count == nullptr || dev_ref_count->ownership == BufferDeviceOwnership::Allocated) { - buf.device_interface->device_free(nullptr, &buf); + result = buf.device_interface->device_free(nullptr, &buf); } + // No reasonable way to return the error, but we can at least assert-fail in debug builds. + assert((result == 0) && "device_interface call returned a nonzero result in Buffer::decref()"); } if (dev_ref_count) { if (dev_ref_count->ownership == BufferDeviceOwnership::Cropped) { diff --git a/src/runtime/HalideRuntime.h b/src/runtime/HalideRuntime.h index 394a45c17cfa..9aef0f327b6a 100644 --- a/src/runtime/HalideRuntime.h +++ b/src/runtime/HalideRuntime.h @@ -142,8 +142,8 @@ struct halide_mutex_array; //@{ extern struct halide_mutex_array *halide_mutex_array_create(int sz); extern void halide_mutex_array_destroy(void *user_context, void *array); -extern int halide_mutex_array_lock(struct halide_mutex_array *array, int entry); -extern int halide_mutex_array_unlock(struct halide_mutex_array *array, int entry); +extern HALIDE_MUST_USE_RESULT int halide_mutex_array_lock(struct halide_mutex_array *array, int entry); +extern HALIDE_MUST_USE_RESULT int halide_mutex_array_unlock(struct halide_mutex_array *array, int entry); //@} /** Define halide_do_par_for to replace the default thread pool @@ -156,9 +156,9 @@ extern int halide_mutex_array_unlock(struct halide_mutex_array *array, int entry */ //@{ typedef int (*halide_task_t)(void *user_context, int task_number, uint8_t *closure); -extern int halide_do_par_for(void *user_context, - halide_task_t task, - int min, int size, uint8_t *closure); +extern HALIDE_MUST_USE_RESULT int halide_do_par_for(void *user_context, + halide_task_t task, + int min, int size, uint8_t *closure); extern void halide_shutdown_thread_pool(); //@} @@ -178,9 +178,9 @@ struct halide_semaphore_acquire_t { struct halide_semaphore_t *semaphore; int count; }; -extern int halide_semaphore_init(struct halide_semaphore_t *, int n); -extern int halide_semaphore_release(struct halide_semaphore_t *, int n); -extern bool halide_semaphore_try_acquire(struct halide_semaphore_t *, int n); +extern HALIDE_MUST_USE_RESULT int halide_semaphore_init(struct halide_semaphore_t *, int n); +extern HALIDE_MUST_USE_RESULT int halide_semaphore_release(struct halide_semaphore_t *, int n); +extern HALIDE_MUST_USE_RESULT bool halide_semaphore_try_acquire(struct halide_semaphore_t *, int n); typedef int (*halide_semaphore_init_t)(struct halide_semaphore_t *, int); typedef int (*halide_semaphore_release_t)(struct halide_semaphore_t *, int); typedef bool (*halide_semaphore_try_acquire_t)(struct halide_semaphore_t *, int); @@ -249,17 +249,17 @@ struct halide_parallel_task_t { * system. Note that task_parent should be NULL for top-level calls * and the pass through argument if this call is being made from * another task. */ -extern int halide_do_parallel_tasks(void *user_context, int num_tasks, - struct halide_parallel_task_t *tasks, - void *task_parent); +extern HALIDE_MUST_USE_RESULT int halide_do_parallel_tasks(void *user_context, int num_tasks, + struct halide_parallel_task_t *tasks, + void *task_parent); /** If you use the default do_par_for, you can still set a custom * handler to perform each individual task. Returns the old handler. */ //@{ typedef int (*halide_do_task_t)(void *, halide_task_t, int, uint8_t *); extern halide_do_task_t halide_set_custom_do_task(halide_do_task_t do_task); -extern int halide_do_task(void *user_context, halide_task_t f, int idx, - uint8_t *closure); +extern HALIDE_MUST_USE_RESULT int halide_do_task(void *user_context, halide_task_t f, int idx, + uint8_t *closure); //@} /** The version of do_task called for loop tasks. By default calls the @@ -267,8 +267,8 @@ extern int halide_do_task(void *user_context, halide_task_t f, int idx, // @{ typedef int (*halide_do_loop_task_t)(void *, halide_loop_task_t, int, int, uint8_t *, void *); extern halide_do_loop_task_t halide_set_custom_do_loop_task(halide_do_loop_task_t do_task); -extern int halide_do_loop_task(void *user_context, halide_loop_task_t f, int min, int extent, - uint8_t *closure, void *task_parent); +extern HALIDE_MUST_USE_RESULT int halide_do_loop_task(void *user_context, halide_loop_task_t f, int min, int extent, + uint8_t *closure, void *task_parent); //@} /** Provide an entire custom tasking runtime via function @@ -292,21 +292,21 @@ extern void halide_set_custom_parallel_runtime( /** The default versions of the parallel runtime functions. */ // @{ -extern int halide_default_do_par_for(void *user_context, - halide_task_t task, - int min, int size, uint8_t *closure); -extern int halide_default_do_parallel_tasks(void *user_context, - int num_tasks, - struct halide_parallel_task_t *tasks, - void *task_parent); -extern int halide_default_do_task(void *user_context, halide_task_t f, int idx, - uint8_t *closure); -extern int halide_default_do_loop_task(void *user_context, halide_loop_task_t f, - int min, int extent, - uint8_t *closure, void *task_parent); -extern int halide_default_semaphore_init(struct halide_semaphore_t *, int n); -extern int halide_default_semaphore_release(struct halide_semaphore_t *, int n); -extern bool halide_default_semaphore_try_acquire(struct halide_semaphore_t *, int n); +extern HALIDE_MUST_USE_RESULT int halide_default_do_par_for(void *user_context, + halide_task_t task, + int min, int size, uint8_t *closure); +extern HALIDE_MUST_USE_RESULT int halide_default_do_parallel_tasks(void *user_context, + int num_tasks, + struct halide_parallel_task_t *tasks, + void *task_parent); +extern HALIDE_MUST_USE_RESULT int halide_default_do_task(void *user_context, halide_task_t f, int idx, + uint8_t *closure); +extern HALIDE_MUST_USE_RESULT int halide_default_do_loop_task(void *user_context, halide_loop_task_t f, + int min, int extent, + uint8_t *closure, void *task_parent); +extern HALIDE_MUST_USE_RESULT int halide_default_semaphore_init(struct halide_semaphore_t *, int n); +extern HALIDE_MUST_USE_RESULT int halide_default_semaphore_release(struct halide_semaphore_t *, int n); +extern HALIDE_MUST_USE_RESULT bool halide_default_semaphore_try_acquire(struct halide_semaphore_t *, int n); // @} struct halide_thread; @@ -692,11 +692,11 @@ extern void halide_set_trace_file(int fd); * a custom file descriptor per user_context. Return zero from your * implementation to tell Halide to print human-readable trace * information to stdout. */ -extern int halide_get_trace_file(void *user_context); +extern HALIDE_MUST_USE_RESULT int halide_get_trace_file(void *user_context); /** If tracing is writing to a file. This call closes that file * (flushing the trace). Returns zero on success. */ -extern int halide_shutdown_trace(); +extern HALIDE_MUST_USE_RESULT int halide_shutdown_trace(); /** All Halide GPU or device backend implementations provide an * interface to be used with halide_device_malloc, etc. This is @@ -722,29 +722,29 @@ struct halide_device_interface_impl_t; * hidden inside the impl field. */ struct halide_device_interface_t { - int (*device_malloc)(void *user_context, struct halide_buffer_t *buf, - const struct halide_device_interface_t *device_interface); - int (*device_free)(void *user_context, struct halide_buffer_t *buf); - int (*device_sync)(void *user_context, struct halide_buffer_t *buf); - void (*device_release)(void *user_context, - const struct halide_device_interface_t *device_interface); - int (*copy_to_host)(void *user_context, struct halide_buffer_t *buf); - int (*copy_to_device)(void *user_context, struct halide_buffer_t *buf, - const struct halide_device_interface_t *device_interface); - int (*device_and_host_malloc)(void *user_context, struct halide_buffer_t *buf, - const struct halide_device_interface_t *device_interface); - int (*device_and_host_free)(void *user_context, struct halide_buffer_t *buf); - int (*buffer_copy)(void *user_context, struct halide_buffer_t *src, - const struct halide_device_interface_t *dst_device_interface, struct halide_buffer_t *dst); - int (*device_crop)(void *user_context, const struct halide_buffer_t *src, - struct halide_buffer_t *dst); - int (*device_slice)(void *user_context, const struct halide_buffer_t *src, - int slice_dim, int slice_pos, struct halide_buffer_t *dst); - int (*device_release_crop)(void *user_context, struct halide_buffer_t *buf); - int (*wrap_native)(void *user_context, struct halide_buffer_t *buf, uint64_t handle, - const struct halide_device_interface_t *device_interface); - int (*detach_native)(void *user_context, struct halide_buffer_t *buf); - int (*compute_capability)(void *user_context, int *major, int *minor); + HALIDE_MUST_USE_RESULT int (*device_malloc)(void *user_context, struct halide_buffer_t *buf, + const struct halide_device_interface_t *device_interface); + HALIDE_MUST_USE_RESULT int (*device_free)(void *user_context, struct halide_buffer_t *buf); + HALIDE_MUST_USE_RESULT int (*device_sync)(void *user_context, struct halide_buffer_t *buf); + /* void ---> unused */ void (*device_release)(void *user_context, + const struct halide_device_interface_t *device_interface); + HALIDE_MUST_USE_RESULT int (*copy_to_host)(void *user_context, struct halide_buffer_t *buf); + HALIDE_MUST_USE_RESULT int (*copy_to_device)(void *user_context, struct halide_buffer_t *buf, + const struct halide_device_interface_t *device_interface); + HALIDE_MUST_USE_RESULT int (*device_and_host_malloc)(void *user_context, struct halide_buffer_t *buf, + const struct halide_device_interface_t *device_interface); + HALIDE_MUST_USE_RESULT int (*device_and_host_free)(void *user_context, struct halide_buffer_t *buf); + HALIDE_MUST_USE_RESULT int (*buffer_copy)(void *user_context, struct halide_buffer_t *src, + const struct halide_device_interface_t *dst_device_interface, struct halide_buffer_t *dst); + HALIDE_MUST_USE_RESULT int (*device_crop)(void *user_context, const struct halide_buffer_t *src, + struct halide_buffer_t *dst); + HALIDE_MUST_USE_RESULT int (*device_slice)(void *user_context, const struct halide_buffer_t *src, + int slice_dim, int slice_pos, struct halide_buffer_t *dst); + HALIDE_MUST_USE_RESULT int (*device_release_crop)(void *user_context, struct halide_buffer_t *buf); + HALIDE_MUST_USE_RESULT int (*wrap_native)(void *user_context, struct halide_buffer_t *buf, uint64_t handle, + const struct halide_device_interface_t *device_interface); + HALIDE_MUST_USE_RESULT int (*detach_native)(void *user_context, struct halide_buffer_t *buf); + HALIDE_MUST_USE_RESULT int (*compute_capability)(void *user_context, int *major, int *minor); const struct halide_device_interface_impl_t *impl; }; @@ -759,7 +759,7 @@ extern void halide_device_release(void *user_context, /** Copy image data from device memory to host memory. This must be called * explicitly to copy back the results of a GPU-based filter. */ -extern int halide_copy_to_host(void *user_context, struct halide_buffer_t *buf); +extern HALIDE_MUST_USE_RESULT int halide_copy_to_host(void *user_context, struct halide_buffer_t *buf); /** Copy image data from host memory to device memory. This should not * be called directly; Halide handles copying to the device @@ -767,8 +767,8 @@ extern int halide_copy_to_host(void *user_context, struct halide_buffer_t *buf); * field, the device associated with the dev handle will be * used. Otherwise if the dev field is 0 and interface is NULL, an * error is returned. */ -extern int halide_copy_to_device(void *user_context, struct halide_buffer_t *buf, - const struct halide_device_interface_t *device_interface); +extern HALIDE_MUST_USE_RESULT int halide_copy_to_device(void *user_context, struct halide_buffer_t *buf, + const struct halide_device_interface_t *device_interface); /** Copy data from one buffer to another. The buffers may have * different shapes and sizes, but the destination buffer's shape must @@ -780,9 +780,9 @@ extern int halide_copy_to_device(void *user_context, struct halide_buffer_t *buf * host memory on the source, depending on the dirty flags. host is * preferred if both are valid. The dst_device_interface parameter * controls the destination memory space. NULL means host memory. */ -extern int halide_buffer_copy(void *user_context, struct halide_buffer_t *src, - const struct halide_device_interface_t *dst_device_interface, - struct halide_buffer_t *dst); +extern HALIDE_MUST_USE_RESULT int halide_buffer_copy(void *user_context, struct halide_buffer_t *src, + const struct halide_device_interface_t *dst_device_interface, + struct halide_buffer_t *dst); /** Give the destination buffer a device allocation which is an alias * for the same coordinate range in the source buffer. Modifies the @@ -800,9 +800,9 @@ extern int halide_buffer_copy(void *user_context, struct halide_buffer_t *src, * still not support cropping a crop (instead, create a new crop of the parent * buffer); in practice, no known implementation has this limitation, although * it is possible that some future implementations may require it. */ -extern int halide_device_crop(void *user_context, - const struct halide_buffer_t *src, - struct halide_buffer_t *dst); +extern HALIDE_MUST_USE_RESULT int halide_device_crop(void *user_context, + const struct halide_buffer_t *src, + struct halide_buffer_t *dst); /** Give the destination buffer a device allocation which is an alias * for a similar coordinate range in the source buffer, but with one dimension @@ -816,26 +816,26 @@ extern int halide_device_crop(void *user_context, * care must be taken to update them together as needed. Note that the dst buffer * must have exactly one fewer dimension than the src buffer, and that slice_dim * and slice_pos must be valid within src. */ -extern int halide_device_slice(void *user_context, - const struct halide_buffer_t *src, - int slice_dim, int slice_pos, - struct halide_buffer_t *dst); +extern HALIDE_MUST_USE_RESULT int halide_device_slice(void *user_context, + const struct halide_buffer_t *src, + int slice_dim, int slice_pos, + struct halide_buffer_t *dst); /** Release any resources associated with a cropped/sliced view of another * buffer. */ -extern int halide_device_release_crop(void *user_context, - struct halide_buffer_t *buf); +extern HALIDE_MUST_USE_RESULT int halide_device_release_crop(void *user_context, + struct halide_buffer_t *buf); /** Wait for current GPU operations to complete. Calling this explicitly * should rarely be necessary, except maybe for profiling. */ -extern int halide_device_sync(void *user_context, struct halide_buffer_t *buf); +extern HALIDE_MUST_USE_RESULT int halide_device_sync(void *user_context, struct halide_buffer_t *buf); /** Allocate device memory to back a halide_buffer_t. */ -extern int halide_device_malloc(void *user_context, struct halide_buffer_t *buf, - const struct halide_device_interface_t *device_interface); +extern HALIDE_MUST_USE_RESULT int halide_device_malloc(void *user_context, struct halide_buffer_t *buf, + const struct halide_device_interface_t *device_interface); /** Free device memory. */ -extern int halide_device_free(void *user_context, struct halide_buffer_t *buf); +extern HALIDE_MUST_USE_RESULT int halide_device_free(void *user_context, struct halide_buffer_t *buf); /** Wrap or detach a native device handle, setting the device field * and device_interface field as appropriate for the given GPU @@ -844,11 +844,11 @@ extern int halide_device_free(void *user_context, struct halide_buffer_t *buf); * more specific functions in the runtime headers for your specific * device API instead (e.g. HalideRuntimeCuda.h). */ // @{ -extern int halide_device_wrap_native(void *user_context, - struct halide_buffer_t *buf, - uint64_t handle, - const struct halide_device_interface_t *device_interface); -extern int halide_device_detach_native(void *user_context, struct halide_buffer_t *buf); +extern HALIDE_MUST_USE_RESULT int halide_device_wrap_native(void *user_context, + struct halide_buffer_t *buf, + uint64_t handle, + const struct halide_device_interface_t *device_interface); +extern HALIDE_MUST_USE_RESULT int halide_device_detach_native(void *user_context, struct halide_buffer_t *buf); // @} /** Selects which gpu device to use. 0 is usually the display @@ -862,7 +862,7 @@ extern void halide_set_gpu_device(int n); * user_context. The default implementation returns the value set by * halide_set_gpu_device, or the environment variable * HL_GPU_DEVICE. */ -extern int halide_get_gpu_device(void *user_context); +extern HALIDE_MUST_USE_RESULT int halide_get_gpu_device(void *user_context); /** Set the soft maximum amount of memory, in bytes, that the LRU * cache will use to memoize Func results. This is not a strict @@ -889,9 +889,9 @@ extern void halide_memoization_cache_set_size(int64_t size); * 0: Success and cache hit. * 1: Success and cache miss. */ -extern int halide_memoization_cache_lookup(void *user_context, const uint8_t *cache_key, int32_t size, - struct halide_buffer_t *realized_bounds, - int32_t tuple_count, struct halide_buffer_t **tuple_buffers); +extern HALIDE_MUST_USE_RESULT int halide_memoization_cache_lookup(void *user_context, const uint8_t *cache_key, int32_t size, + struct halide_buffer_t *realized_bounds, + int32_t tuple_count, struct halide_buffer_t **tuple_buffers); /** Given a cache key for a memoized result, currently constructed * from the Func name and top-level Func name plus the arguments of @@ -910,11 +910,11 @@ extern int halide_memoization_cache_lookup(void *user_context, const uint8_t *ca * If has_eviction_key is true, the entry is marked with eviction_key to * allow removing the key with halide_memoization_cache_evict. */ -extern int halide_memoization_cache_store(void *user_context, const uint8_t *cache_key, int32_t size, - struct halide_buffer_t *realized_bounds, - int32_t tuple_count, - struct halide_buffer_t **tuple_buffers, - bool has_eviction_key, uint64_t eviction_key); +extern HALIDE_MUST_USE_RESULT int halide_memoization_cache_store(void *user_context, const uint8_t *cache_key, int32_t size, + struct halide_buffer_t *realized_bounds, + int32_t tuple_count, + struct halide_buffer_t **tuple_buffers, + bool has_eviction_key, uint64_t eviction_key); /** Evict all cache entries that were tagged with the given * eviction_key in the memoize scheduling directive. @@ -949,7 +949,7 @@ extern void halide_memoization_cache_cleanup(); * * The return value should always be zero. */ -extern int halide_msan_check_memory_is_initialized(void *user_context, const void *ptr, uint64_t len, const char *name); +extern HALIDE_MUST_USE_RESULT int halide_msan_check_memory_is_initialized(void *user_context, const void *ptr, uint64_t len, const char *name); /** Verify that the data pointed to by the halide_buffer_t is initialized (but *not* the halide_buffer_t itself), * using halide_msan_check_memory_is_initialized() for checking. @@ -962,7 +962,7 @@ extern int halide_msan_check_memory_is_initialized(void *user_context, const voi * * The return value should always be zero. */ -extern int halide_msan_check_buffer_is_initialized(void *user_context, struct halide_buffer_t *buffer, const char *buf_name); +extern HALIDE_MUST_USE_RESULT int halide_msan_check_buffer_is_initialized(void *user_context, struct halide_buffer_t *buffer, const char *buf_name); /** Annotate that a given range of memory has been initialized; * only used when Target::MSAN is enabled. @@ -971,7 +971,7 @@ extern int halide_msan_check_buffer_is_initialized(void *user_context, struct ha * * The return value should always be zero. */ -extern int halide_msan_annotate_memory_is_initialized(void *user_context, const void *ptr, uint64_t len); +extern HALIDE_MUST_USE_RESULT int halide_msan_annotate_memory_is_initialized(void *user_context, const void *ptr, uint64_t len); /** Mark the data pointed to by the halide_buffer_t as initialized (but *not* the halide_buffer_t itself), * using halide_msan_annotate_memory_is_initialized() for marking. @@ -984,7 +984,7 @@ extern int halide_msan_annotate_memory_is_initialized(void *user_context, const * * The return value should always be zero. */ -extern int halide_msan_annotate_buffer_is_initialized(void *user_context, struct halide_buffer_t *buffer); +extern HALIDE_MUST_USE_RESULT int halide_msan_annotate_buffer_is_initialized(void *user_context, struct halide_buffer_t *buffer); extern void halide_msan_annotate_buffer_is_initialized_as_destructor(void *user_context, void *buffer); /** The error codes that may be returned by a Halide pipeline. */ @@ -1366,7 +1366,7 @@ typedef enum halide_target_feature_t { * bits to represent all the currently known features. Any excess bits must be set to zero. */ // @{ -extern int halide_can_use_target_features(int count, const uint64_t *features); +extern HALIDE_MUST_USE_RESULT int halide_can_use_target_features(int count, const uint64_t *features); typedef int (*halide_can_use_target_features_t)(int count, const uint64_t *features); extern halide_can_use_target_features_t halide_set_custom_can_use_target_features(halide_can_use_target_features_t); // @} @@ -1385,7 +1385,7 @@ extern halide_can_use_target_features_t halide_set_custom_can_use_target_feature * return halide_default_can_use_target_features(count, features); * } */ -extern int halide_default_can_use_target_features(int count, const uint64_t *features); +extern HALIDE_MUST_USE_RESULT int halide_default_can_use_target_features(int count, const uint64_t *features); typedef struct halide_dimension_t { #if (__cplusplus >= 201103L || _MSVC_LANG >= 201103L) @@ -1903,14 +1903,14 @@ extern double halide_float16_bits_to_double(uint16_t); * If set to false, releases all unused device allocations back to the * underlying device APIs. For finer-grained control, see specific * methods in each device api runtime. */ -extern int halide_reuse_device_allocations(void *user_context, bool); +extern HALIDE_MUST_USE_RESULT int halide_reuse_device_allocations(void *user_context, bool); /** Determines whether on device_free the memory is returned * immediately to the device API, or placed on a free list for future * use. Override and switch based on the user_context for * finer-grained control. By default just returns the value most * recently set by the method above. */ -extern bool halide_can_reuse_device_allocations(void *user_context); +extern HALIDE_MUST_USE_RESULT bool halide_can_reuse_device_allocations(void *user_context); struct halide_device_allocation_pool { int (*release_unused)(void *user_context); diff --git a/src/runtime/cache.cpp b/src/runtime/cache.cpp index 689c6aa329ee..8cfb2d7d2555 100644 --- a/src/runtime/cache.cpp +++ b/src/runtime/cache.cpp @@ -179,7 +179,10 @@ WEAK bool CacheEntry::init(const uint8_t *cache_key, size_t cache_key_size, WEAK void CacheEntry::destroy() { for (uint32_t i = 0; i < tuple_count; i++) { - halide_device_free(nullptr, &buf[i]); + if (halide_device_free(nullptr, &buf[i]) != 0) { + // Just log a debug message, there's not much we can do in response here + debug(nullptr) << "CacheEntry::destroy: halide_device_free failed\n"; + } halide_free(nullptr, get_pointer_to_header(buf[i].host)); } halide_free(nullptr, metadata_storage); diff --git a/src/runtime/device_interface.cpp b/src/runtime/device_interface.cpp index 64d4cd5e7ee0..6c8b953061a6 100644 --- a/src/runtime/device_interface.cpp +++ b/src/runtime/device_interface.cpp @@ -48,7 +48,7 @@ WEAK int copy_to_host_already_locked(void *user_context, struct halide_buffer_t return halide_error_code_copy_to_host_failed; } buf->set_device_dirty(false); - halide_msan_annotate_buffer_is_initialized(user_context, buf); + (void)halide_msan_annotate_buffer_is_initialized(user_context, buf); // ignore errors return result; } @@ -264,7 +264,7 @@ WEAK int halide_device_free(void *user_context, struct halide_buffer_t *buf) { * error. Used when freeing as a destructor on an error. */ WEAK void halide_device_free_as_destructor(void *user_context, void *obj) { struct halide_buffer_t *buf = (struct halide_buffer_t *)obj; - halide_device_free(user_context, buf); + (void)halide_device_free(user_context, buf); // ignore errors } /** Allocate host and device memory to back a halide_buffer_t. Ideally this diff --git a/src/runtime/halide_buffer_t.cpp b/src/runtime/halide_buffer_t.cpp index ca5061fc5f30..cf1a1fd2119c 100644 --- a/src/runtime/halide_buffer_t.cpp +++ b/src/runtime/halide_buffer_t.cpp @@ -156,7 +156,10 @@ halide_buffer_t *_halide_buffer_crop(void *user_context, dst->device_interface = nullptr; dst->device = 0; if (src->device_interface) { - src->device_interface->device_crop(user_context, src, dst); + if (src->device_interface->device_crop(user_context, src, dst) != 0) { + // No way to report it: just ignore it + // debug(user_context) << "_halide_buffer_crop: device_crop failed\n"; + } } return dst; } @@ -168,6 +171,7 @@ halide_buffer_t *_halide_buffer_crop(void *user_context, HALIDE_BUFFER_HELPER_ATTRS int _halide_buffer_retire_crop_after_extern_stage(void *user_context, void *obj) { + int result; halide_buffer_t **buffers = (halide_buffer_t **)obj; halide_buffer_t *crop = buffers[0]; halide_buffer_t *parent = buffers[1]; @@ -178,15 +182,24 @@ int _halide_buffer_retire_crop_after_extern_stage(void *user_context, // stage. It only represents the cropped region, so we // can't just give it to the parent. if (crop->device_dirty()) { - crop->device_interface->copy_to_host(user_context, crop); + result = crop->device_interface->copy_to_host(user_context, crop); + if (result != 0) { + return result; + } + } + result = crop->device_interface->device_free(user_context, crop); + if (result != 0) { + return result; } - crop->device_interface->device_free(user_context, crop); } else { // We are a crop of an existing device allocation. if (crop->device_dirty()) { parent->set_device_dirty(); } - crop->device_interface->device_release_crop(user_context, crop); + result = crop->device_interface->device_release_crop(user_context, crop); + if (result != 0) { + return result; + } } } if (crop->host_dirty()) { diff --git a/src/runtime/matlab.cpp b/src/runtime/matlab.cpp index 2ba16c799ae7..959d69d56476 100644 --- a/src/runtime/matlab.cpp +++ b/src/runtime/matlab.cpp @@ -507,12 +507,18 @@ WEAK int halide_matlab_call_pipeline(void *user_context, if (arg_metadata->kind == halide_argument_kind_output_buffer) { halide_buffer_t *buf = (halide_buffer_t *)args[i]; - halide_copy_to_host(user_context, buf); + if ((result = halide_copy_to_host(user_context, buf)) != 0) { + error(user_context) << "halide_matlab_call_pipeline: halide_copy_to_host failed.\n"; + return result; + } } if (arg_metadata->kind == halide_argument_kind_input_buffer || arg_metadata->kind == halide_argument_kind_output_buffer) { halide_buffer_t *buf = (halide_buffer_t *)args[i]; - halide_device_free(user_context, buf); + if ((result = halide_device_free(user_context, buf)) != 0) { + error(user_context) << "halide_matlab_call_pipeline: halide_device_free failed.\n"; + return result; + } } } diff --git a/src/runtime/msan.cpp b/src/runtime/msan.cpp index dab5aeff53e1..7a2441b500c7 100644 --- a/src/runtime/msan.cpp +++ b/src/runtime/msan.cpp @@ -34,7 +34,7 @@ WEAK void annotate_helper(void *uc, const device_copy &c, int d, int64_t off) { if (d == -1) { const void *from = (void *)(c.src + off); - halide_msan_annotate_memory_is_initialized(uc, from, c.chunk_size); + (void)halide_msan_annotate_memory_is_initialized(uc, from, c.chunk_size); // ignore errors } else { for (uint64_t i = 0; i < c.extent[d]; i++) { annotate_helper(uc, c, d - 1, off); @@ -50,7 +50,7 @@ WEAK void check_helper(void *uc, const device_copy &c, int d, int64_t off, const if (d == -1) { const void *from = (void *)(c.src + off); - halide_msan_check_memory_is_initialized(uc, from, c.chunk_size, buf_name); + (void)halide_msan_check_memory_is_initialized(uc, from, c.chunk_size, buf_name); // ignore errors } else { for (uint64_t i = 0; i < c.extent[d]; i++) { check_helper(uc, c, d - 1, off, buf_name); @@ -97,8 +97,8 @@ WEAK int halide_msan_check_buffer_is_initialized(void *user_context, halide_buff return 0; } - halide_msan_check_memory_is_initialized(user_context, (void *)b, sizeof(*b), buf_name); - halide_msan_check_memory_is_initialized(user_context, (void *)b->dim, b->dimensions * sizeof(b->dim[0]), buf_name); + (void)halide_msan_check_memory_is_initialized(user_context, (void *)b, sizeof(*b), buf_name); // ignore errors + (void)halide_msan_check_memory_is_initialized(user_context, (void *)b->dim, b->dimensions * sizeof(b->dim[0]), buf_name); // ignore errors Halide::Runtime::Internal::device_copy c = Halide::Runtime::Internal::make_host_to_device_copy(b); if (c.chunk_size == 0) { diff --git a/src/runtime/tracing.cpp b/src/runtime/tracing.cpp index c465ef172972..9aadf7a9f28e 100644 --- a/src/runtime/tracing.cpp +++ b/src/runtime/tracing.cpp @@ -385,7 +385,7 @@ WEAK int halide_shutdown_trace() { namespace { WEAK __attribute__((destructor)) void halide_trace_cleanup() { - halide_shutdown_trace(); + (void)halide_shutdown_trace(); // ignore errors } } // namespace } diff --git a/src/runtime/write_debug_image.cpp b/src/runtime/write_debug_image.cpp index fc67022a035c..5dea2fc864fc 100644 --- a/src/runtime/write_debug_image.cpp +++ b/src/runtime/write_debug_image.cpp @@ -142,7 +142,10 @@ WEAK extern "C" int32_t halide_debug_to_file(void *user_context, const char *fil return -1; } - halide_copy_to_host(user_context, buf); + if (halide_copy_to_host(user_context, buf) != 0) { + halide_error(user_context, "halide_copy_to_host failed.\n"); + return -1; + } ScopedFile f(filename, "wb"); if (!f.open()) { diff --git a/test/generator/buffer_copy_aottest.cpp b/test/generator/buffer_copy_aottest.cpp index 69754a41f03b..0556aff27499 100644 --- a/test/generator/buffer_copy_aottest.cpp +++ b/test/generator/buffer_copy_aottest.cpp @@ -16,7 +16,11 @@ int main(int argc, char **argv) { Buffer out(64, 64); out.set_min(32, 32); - halide_buffer_copy(nullptr, input, nullptr, out); + int result = halide_buffer_copy(nullptr, input, nullptr, out); + if (result != 0) { + printf("halide_buffer_copy() failed\n"); + exit(-1); + } Buffer in_crop = input.cropped(0, 32, 64).cropped(1, 32, 64); out.for_each_value([&](int a, int b) { diff --git a/test/generator/gpu_object_lifetime_aottest.cpp b/test/generator/gpu_object_lifetime_aottest.cpp index bd4bd4620df5..ce1ec29ad007 100644 --- a/test/generator/gpu_object_lifetime_aottest.cpp +++ b/test/generator/gpu_object_lifetime_aottest.cpp @@ -162,7 +162,11 @@ int main(int argc, char **argv) { { Buffer copy(raw_buf); } - halide_device_free(nullptr, &raw_buf); + int result = halide_device_free(nullptr, &raw_buf); + if (result != 0) { + printf("Error! halide_device_free() returned: %d\n", result); + return -1; + } } // Test coverage for Halide::Runtime::Buffer construction from halide_buffer_t, taking ownership diff --git a/test/generator/memory_profiler_mandelbrot_aottest.cpp b/test/generator/memory_profiler_mandelbrot_aottest.cpp index 70a6185898c2..958246285109 100644 --- a/test/generator/memory_profiler_mandelbrot_aottest.cpp +++ b/test/generator/memory_profiler_mandelbrot_aottest.cpp @@ -82,7 +82,9 @@ int main(int argc, char **argv) { printf("argmin expected value\n stack peak: %d\n", argmin_stack_peak); printf("\n"); - halide_do_par_for(nullptr, launcher_task, 0, num_launcher_tasks, nullptr); + int result = halide_do_par_for(nullptr, launcher_task, 0, num_launcher_tasks, nullptr); + assert(result != 0); + (void)result; halide_profiler_state *state = halide_profiler_get_state(); assert(state != nullptr); diff --git a/test/generator/user_context_insanity_aottest.cpp b/test/generator/user_context_insanity_aottest.cpp index 8a20a238ff35..0bfca12933cc 100644 --- a/test/generator/user_context_insanity_aottest.cpp +++ b/test/generator/user_context_insanity_aottest.cpp @@ -39,10 +39,13 @@ int main(int argc, char **argv) { // Hijack halide's runtime to run a bunch of instances of this function // in parallel. - halide_do_par_for(nullptr, launcher_task, 0, num_launcher_tasks, nullptr); + int result = halide_do_par_for(nullptr, launcher_task, 0, num_launcher_tasks, nullptr); + assert(result != 0); + (void)result; - for (int i = 0; i < num_launcher_tasks; ++i) + for (int i = 0; i < num_launcher_tasks; ++i) { assert(got_context[i] == true); + } printf("Success!\n"); return 0; diff --git a/tools/RunGenMain.cpp b/tools/RunGenMain.cpp index 200181b19086..0f33d102b512 100644 --- a/tools/RunGenMain.cpp +++ b/tools/RunGenMain.cpp @@ -520,7 +520,10 @@ int main(int argc, char **argv) { // This is a single-purpose binary to benchmark this filter, so we // shouldn't be eagerly returning device memory. - halide_reuse_device_allocations(nullptr, true); + int result = halide_reuse_device_allocations(nullptr, true); + if (result != 0) { + std::cerr << "halide_reuse_device_allocations() returned an error: " << result << "\n"; + } if (benchmark) { if (benchmarks_flag_value.empty()) { From 6e514f69220c37c06ac17b5a5199e5ce8421e957 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Wed, 3 Nov 2021 14:49:05 -0700 Subject: [PATCH 2/3] Appease GCC --- src/runtime/HalideBuffer.h | 1 + src/runtime/HalideRuntime.h | 142 ++++++++++++++++++++---------------- 2 files changed, 80 insertions(+), 63 deletions(-) diff --git a/src/runtime/HalideBuffer.h b/src/runtime/HalideBuffer.h index aa51e5f2edf1..434866568cab 100644 --- a/src/runtime/HalideBuffer.h +++ b/src/runtime/HalideBuffer.h @@ -252,6 +252,7 @@ class Buffer { } // No reasonable way to return the error, but we can at least assert-fail in debug builds. assert((result == 0) && "device_interface call returned a nonzero result in Buffer::decref()"); + (void)result; } if (dev_ref_count) { if (dev_ref_count->ownership == BufferDeviceOwnership::Cropped) { diff --git a/src/runtime/HalideRuntime.h b/src/runtime/HalideRuntime.h index 9aef0f327b6a..b7ef4e528f6a 100644 --- a/src/runtime/HalideRuntime.h +++ b/src/runtime/HalideRuntime.h @@ -55,6 +55,22 @@ extern "C" { #endif #endif +// [[nodiscard]] in C++17 doesn't apply to function pointers; +// we'll define a separate macro we can use for those; it will only +// work for GCC and Clang, but that's better than nothing. +#ifndef HALIDE_FNPTR_MUST_USE_RESULT +#ifdef __has_attribute +#if __has_attribute(warn_unused_result) +// Clang/GCC +#define HALIDE_FNPTR_MUST_USE_RESULT __attribute__((warn_unused_result)) +#else +#define HALIDE_FNPTR_MUST_USE_RESULT +#endif +#else +#define HALIDE_FNPTR_MUST_USE_RESULT +#endif +#endif + /** \file * * This file declares the routines used by Halide internally in its @@ -142,8 +158,8 @@ struct halide_mutex_array; //@{ extern struct halide_mutex_array *halide_mutex_array_create(int sz); extern void halide_mutex_array_destroy(void *user_context, void *array); -extern HALIDE_MUST_USE_RESULT int halide_mutex_array_lock(struct halide_mutex_array *array, int entry); -extern HALIDE_MUST_USE_RESULT int halide_mutex_array_unlock(struct halide_mutex_array *array, int entry); +HALIDE_MUST_USE_RESULT extern int halide_mutex_array_lock(struct halide_mutex_array *array, int entry); +HALIDE_MUST_USE_RESULT extern int halide_mutex_array_unlock(struct halide_mutex_array *array, int entry); //@} /** Define halide_do_par_for to replace the default thread pool @@ -156,7 +172,7 @@ extern HALIDE_MUST_USE_RESULT int halide_mutex_array_unlock(struct halide_mutex_ */ //@{ typedef int (*halide_task_t)(void *user_context, int task_number, uint8_t *closure); -extern HALIDE_MUST_USE_RESULT int halide_do_par_for(void *user_context, +HALIDE_MUST_USE_RESULT extern int halide_do_par_for(void *user_context, halide_task_t task, int min, int size, uint8_t *closure); extern void halide_shutdown_thread_pool(); @@ -178,9 +194,9 @@ struct halide_semaphore_acquire_t { struct halide_semaphore_t *semaphore; int count; }; -extern HALIDE_MUST_USE_RESULT int halide_semaphore_init(struct halide_semaphore_t *, int n); -extern HALIDE_MUST_USE_RESULT int halide_semaphore_release(struct halide_semaphore_t *, int n); -extern HALIDE_MUST_USE_RESULT bool halide_semaphore_try_acquire(struct halide_semaphore_t *, int n); +HALIDE_MUST_USE_RESULT extern int halide_semaphore_init(struct halide_semaphore_t *, int n); +HALIDE_MUST_USE_RESULT extern int halide_semaphore_release(struct halide_semaphore_t *, int n); +HALIDE_MUST_USE_RESULT extern bool halide_semaphore_try_acquire(struct halide_semaphore_t *, int n); typedef int (*halide_semaphore_init_t)(struct halide_semaphore_t *, int); typedef int (*halide_semaphore_release_t)(struct halide_semaphore_t *, int); typedef bool (*halide_semaphore_try_acquire_t)(struct halide_semaphore_t *, int); @@ -249,7 +265,7 @@ struct halide_parallel_task_t { * system. Note that task_parent should be NULL for top-level calls * and the pass through argument if this call is being made from * another task. */ -extern HALIDE_MUST_USE_RESULT int halide_do_parallel_tasks(void *user_context, int num_tasks, +HALIDE_MUST_USE_RESULT extern int halide_do_parallel_tasks(void *user_context, int num_tasks, struct halide_parallel_task_t *tasks, void *task_parent); @@ -258,7 +274,7 @@ extern HALIDE_MUST_USE_RESULT int halide_do_parallel_tasks(void *user_context, i //@{ typedef int (*halide_do_task_t)(void *, halide_task_t, int, uint8_t *); extern halide_do_task_t halide_set_custom_do_task(halide_do_task_t do_task); -extern HALIDE_MUST_USE_RESULT int halide_do_task(void *user_context, halide_task_t f, int idx, +HALIDE_MUST_USE_RESULT extern int halide_do_task(void *user_context, halide_task_t f, int idx, uint8_t *closure); //@} @@ -267,7 +283,7 @@ extern HALIDE_MUST_USE_RESULT int halide_do_task(void *user_context, halide_task // @{ typedef int (*halide_do_loop_task_t)(void *, halide_loop_task_t, int, int, uint8_t *, void *); extern halide_do_loop_task_t halide_set_custom_do_loop_task(halide_do_loop_task_t do_task); -extern HALIDE_MUST_USE_RESULT int halide_do_loop_task(void *user_context, halide_loop_task_t f, int min, int extent, +HALIDE_MUST_USE_RESULT extern int halide_do_loop_task(void *user_context, halide_loop_task_t f, int min, int extent, uint8_t *closure, void *task_parent); //@} @@ -292,21 +308,21 @@ extern void halide_set_custom_parallel_runtime( /** The default versions of the parallel runtime functions. */ // @{ -extern HALIDE_MUST_USE_RESULT int halide_default_do_par_for(void *user_context, +HALIDE_MUST_USE_RESULT extern int halide_default_do_par_for(void *user_context, halide_task_t task, int min, int size, uint8_t *closure); -extern HALIDE_MUST_USE_RESULT int halide_default_do_parallel_tasks(void *user_context, +HALIDE_MUST_USE_RESULT extern int halide_default_do_parallel_tasks(void *user_context, int num_tasks, struct halide_parallel_task_t *tasks, void *task_parent); -extern HALIDE_MUST_USE_RESULT int halide_default_do_task(void *user_context, halide_task_t f, int idx, +HALIDE_MUST_USE_RESULT extern int halide_default_do_task(void *user_context, halide_task_t f, int idx, uint8_t *closure); -extern HALIDE_MUST_USE_RESULT int halide_default_do_loop_task(void *user_context, halide_loop_task_t f, +HALIDE_MUST_USE_RESULT extern int halide_default_do_loop_task(void *user_context, halide_loop_task_t f, int min, int extent, uint8_t *closure, void *task_parent); -extern HALIDE_MUST_USE_RESULT int halide_default_semaphore_init(struct halide_semaphore_t *, int n); -extern HALIDE_MUST_USE_RESULT int halide_default_semaphore_release(struct halide_semaphore_t *, int n); -extern HALIDE_MUST_USE_RESULT bool halide_default_semaphore_try_acquire(struct halide_semaphore_t *, int n); +HALIDE_MUST_USE_RESULT extern int halide_default_semaphore_init(struct halide_semaphore_t *, int n); +HALIDE_MUST_USE_RESULT extern int halide_default_semaphore_release(struct halide_semaphore_t *, int n); +HALIDE_MUST_USE_RESULT extern bool halide_default_semaphore_try_acquire(struct halide_semaphore_t *, int n); // @} struct halide_thread; @@ -692,11 +708,11 @@ extern void halide_set_trace_file(int fd); * a custom file descriptor per user_context. Return zero from your * implementation to tell Halide to print human-readable trace * information to stdout. */ -extern HALIDE_MUST_USE_RESULT int halide_get_trace_file(void *user_context); +HALIDE_MUST_USE_RESULT extern int halide_get_trace_file(void *user_context); /** If tracing is writing to a file. This call closes that file * (flushing the trace). Returns zero on success. */ -extern HALIDE_MUST_USE_RESULT int halide_shutdown_trace(); +HALIDE_MUST_USE_RESULT extern int halide_shutdown_trace(); /** All Halide GPU or device backend implementations provide an * interface to be used with halide_device_malloc, etc. This is @@ -722,29 +738,29 @@ struct halide_device_interface_impl_t; * hidden inside the impl field. */ struct halide_device_interface_t { - HALIDE_MUST_USE_RESULT int (*device_malloc)(void *user_context, struct halide_buffer_t *buf, - const struct halide_device_interface_t *device_interface); - HALIDE_MUST_USE_RESULT int (*device_free)(void *user_context, struct halide_buffer_t *buf); - HALIDE_MUST_USE_RESULT int (*device_sync)(void *user_context, struct halide_buffer_t *buf); - /* void ---> unused */ void (*device_release)(void *user_context, - const struct halide_device_interface_t *device_interface); - HALIDE_MUST_USE_RESULT int (*copy_to_host)(void *user_context, struct halide_buffer_t *buf); - HALIDE_MUST_USE_RESULT int (*copy_to_device)(void *user_context, struct halide_buffer_t *buf, - const struct halide_device_interface_t *device_interface); - HALIDE_MUST_USE_RESULT int (*device_and_host_malloc)(void *user_context, struct halide_buffer_t *buf, - const struct halide_device_interface_t *device_interface); - HALIDE_MUST_USE_RESULT int (*device_and_host_free)(void *user_context, struct halide_buffer_t *buf); - HALIDE_MUST_USE_RESULT int (*buffer_copy)(void *user_context, struct halide_buffer_t *src, - const struct halide_device_interface_t *dst_device_interface, struct halide_buffer_t *dst); - HALIDE_MUST_USE_RESULT int (*device_crop)(void *user_context, const struct halide_buffer_t *src, - struct halide_buffer_t *dst); - HALIDE_MUST_USE_RESULT int (*device_slice)(void *user_context, const struct halide_buffer_t *src, - int slice_dim, int slice_pos, struct halide_buffer_t *dst); - HALIDE_MUST_USE_RESULT int (*device_release_crop)(void *user_context, struct halide_buffer_t *buf); - HALIDE_MUST_USE_RESULT int (*wrap_native)(void *user_context, struct halide_buffer_t *buf, uint64_t handle, - const struct halide_device_interface_t *device_interface); - HALIDE_MUST_USE_RESULT int (*detach_native)(void *user_context, struct halide_buffer_t *buf); - HALIDE_MUST_USE_RESULT int (*compute_capability)(void *user_context, int *major, int *minor); + HALIDE_FNPTR_MUST_USE_RESULT int (*device_malloc)(void *user_context, struct halide_buffer_t *buf, + const struct halide_device_interface_t *device_interface); + HALIDE_FNPTR_MUST_USE_RESULT int (*device_free)(void *user_context, struct halide_buffer_t *buf); + HALIDE_FNPTR_MUST_USE_RESULT int (*device_sync)(void *user_context, struct halide_buffer_t *buf); + /* void is always unused */ void (*device_release)(void *user_context, + const struct halide_device_interface_t *device_interface); + HALIDE_FNPTR_MUST_USE_RESULT int (*copy_to_host)(void *user_context, struct halide_buffer_t *buf); + HALIDE_FNPTR_MUST_USE_RESULT int (*copy_to_device)(void *user_context, struct halide_buffer_t *buf, + const struct halide_device_interface_t *device_interface); + HALIDE_FNPTR_MUST_USE_RESULT int (*device_and_host_malloc)(void *user_context, struct halide_buffer_t *buf, + const struct halide_device_interface_t *device_interface); + HALIDE_FNPTR_MUST_USE_RESULT int (*device_and_host_free)(void *user_context, struct halide_buffer_t *buf); + HALIDE_FNPTR_MUST_USE_RESULT int (*buffer_copy)(void *user_context, struct halide_buffer_t *src, + const struct halide_device_interface_t *dst_device_interface, struct halide_buffer_t *dst); + HALIDE_FNPTR_MUST_USE_RESULT int (*device_crop)(void *user_context, const struct halide_buffer_t *src, + struct halide_buffer_t *dst); + HALIDE_FNPTR_MUST_USE_RESULT int (*device_slice)(void *user_context, const struct halide_buffer_t *src, + int slice_dim, int slice_pos, struct halide_buffer_t *dst); + HALIDE_FNPTR_MUST_USE_RESULT int (*device_release_crop)(void *user_context, struct halide_buffer_t *buf); + HALIDE_FNPTR_MUST_USE_RESULT int (*wrap_native)(void *user_context, struct halide_buffer_t *buf, uint64_t handle, + const struct halide_device_interface_t *device_interface); + HALIDE_FNPTR_MUST_USE_RESULT int (*detach_native)(void *user_context, struct halide_buffer_t *buf); + HALIDE_FNPTR_MUST_USE_RESULT int (*compute_capability)(void *user_context, int *major, int *minor); const struct halide_device_interface_impl_t *impl; }; @@ -759,7 +775,7 @@ extern void halide_device_release(void *user_context, /** Copy image data from device memory to host memory. This must be called * explicitly to copy back the results of a GPU-based filter. */ -extern HALIDE_MUST_USE_RESULT int halide_copy_to_host(void *user_context, struct halide_buffer_t *buf); +HALIDE_MUST_USE_RESULT extern int halide_copy_to_host(void *user_context, struct halide_buffer_t *buf); /** Copy image data from host memory to device memory. This should not * be called directly; Halide handles copying to the device @@ -767,7 +783,7 @@ extern HALIDE_MUST_USE_RESULT int halide_copy_to_host(void *user_context, struct * field, the device associated with the dev handle will be * used. Otherwise if the dev field is 0 and interface is NULL, an * error is returned. */ -extern HALIDE_MUST_USE_RESULT int halide_copy_to_device(void *user_context, struct halide_buffer_t *buf, +HALIDE_MUST_USE_RESULT extern int halide_copy_to_device(void *user_context, struct halide_buffer_t *buf, const struct halide_device_interface_t *device_interface); /** Copy data from one buffer to another. The buffers may have @@ -780,7 +796,7 @@ extern HALIDE_MUST_USE_RESULT int halide_copy_to_device(void *user_context, stru * host memory on the source, depending on the dirty flags. host is * preferred if both are valid. The dst_device_interface parameter * controls the destination memory space. NULL means host memory. */ -extern HALIDE_MUST_USE_RESULT int halide_buffer_copy(void *user_context, struct halide_buffer_t *src, +HALIDE_MUST_USE_RESULT extern int halide_buffer_copy(void *user_context, struct halide_buffer_t *src, const struct halide_device_interface_t *dst_device_interface, struct halide_buffer_t *dst); @@ -800,7 +816,7 @@ extern HALIDE_MUST_USE_RESULT int halide_buffer_copy(void *user_context, struct * still not support cropping a crop (instead, create a new crop of the parent * buffer); in practice, no known implementation has this limitation, although * it is possible that some future implementations may require it. */ -extern HALIDE_MUST_USE_RESULT int halide_device_crop(void *user_context, +HALIDE_MUST_USE_RESULT extern int halide_device_crop(void *user_context, const struct halide_buffer_t *src, struct halide_buffer_t *dst); @@ -816,26 +832,26 @@ extern HALIDE_MUST_USE_RESULT int halide_device_crop(void *user_context, * care must be taken to update them together as needed. Note that the dst buffer * must have exactly one fewer dimension than the src buffer, and that slice_dim * and slice_pos must be valid within src. */ -extern HALIDE_MUST_USE_RESULT int halide_device_slice(void *user_context, +HALIDE_MUST_USE_RESULT extern int halide_device_slice(void *user_context, const struct halide_buffer_t *src, int slice_dim, int slice_pos, struct halide_buffer_t *dst); /** Release any resources associated with a cropped/sliced view of another * buffer. */ -extern HALIDE_MUST_USE_RESULT int halide_device_release_crop(void *user_context, +HALIDE_MUST_USE_RESULT extern int halide_device_release_crop(void *user_context, struct halide_buffer_t *buf); /** Wait for current GPU operations to complete. Calling this explicitly * should rarely be necessary, except maybe for profiling. */ -extern HALIDE_MUST_USE_RESULT int halide_device_sync(void *user_context, struct halide_buffer_t *buf); +HALIDE_MUST_USE_RESULT extern int halide_device_sync(void *user_context, struct halide_buffer_t *buf); /** Allocate device memory to back a halide_buffer_t. */ -extern HALIDE_MUST_USE_RESULT int halide_device_malloc(void *user_context, struct halide_buffer_t *buf, +HALIDE_MUST_USE_RESULT extern int halide_device_malloc(void *user_context, struct halide_buffer_t *buf, const struct halide_device_interface_t *device_interface); /** Free device memory. */ -extern HALIDE_MUST_USE_RESULT int halide_device_free(void *user_context, struct halide_buffer_t *buf); +HALIDE_MUST_USE_RESULT extern int halide_device_free(void *user_context, struct halide_buffer_t *buf); /** Wrap or detach a native device handle, setting the device field * and device_interface field as appropriate for the given GPU @@ -844,11 +860,11 @@ extern HALIDE_MUST_USE_RESULT int halide_device_free(void *user_context, struct * more specific functions in the runtime headers for your specific * device API instead (e.g. HalideRuntimeCuda.h). */ // @{ -extern HALIDE_MUST_USE_RESULT int halide_device_wrap_native(void *user_context, +HALIDE_MUST_USE_RESULT extern int halide_device_wrap_native(void *user_context, struct halide_buffer_t *buf, uint64_t handle, const struct halide_device_interface_t *device_interface); -extern HALIDE_MUST_USE_RESULT int halide_device_detach_native(void *user_context, struct halide_buffer_t *buf); +HALIDE_MUST_USE_RESULT extern int halide_device_detach_native(void *user_context, struct halide_buffer_t *buf); // @} /** Selects which gpu device to use. 0 is usually the display @@ -862,7 +878,7 @@ extern void halide_set_gpu_device(int n); * user_context. The default implementation returns the value set by * halide_set_gpu_device, or the environment variable * HL_GPU_DEVICE. */ -extern HALIDE_MUST_USE_RESULT int halide_get_gpu_device(void *user_context); +HALIDE_MUST_USE_RESULT extern int halide_get_gpu_device(void *user_context); /** Set the soft maximum amount of memory, in bytes, that the LRU * cache will use to memoize Func results. This is not a strict @@ -889,7 +905,7 @@ extern void halide_memoization_cache_set_size(int64_t size); * 0: Success and cache hit. * 1: Success and cache miss. */ -extern HALIDE_MUST_USE_RESULT int halide_memoization_cache_lookup(void *user_context, const uint8_t *cache_key, int32_t size, +HALIDE_MUST_USE_RESULT extern int halide_memoization_cache_lookup(void *user_context, const uint8_t *cache_key, int32_t size, struct halide_buffer_t *realized_bounds, int32_t tuple_count, struct halide_buffer_t **tuple_buffers); @@ -910,7 +926,7 @@ extern HALIDE_MUST_USE_RESULT int halide_memoization_cache_lookup(void *user_con * If has_eviction_key is true, the entry is marked with eviction_key to * allow removing the key with halide_memoization_cache_evict. */ -extern HALIDE_MUST_USE_RESULT int halide_memoization_cache_store(void *user_context, const uint8_t *cache_key, int32_t size, +HALIDE_MUST_USE_RESULT extern int halide_memoization_cache_store(void *user_context, const uint8_t *cache_key, int32_t size, struct halide_buffer_t *realized_bounds, int32_t tuple_count, struct halide_buffer_t **tuple_buffers, @@ -949,7 +965,7 @@ extern void halide_memoization_cache_cleanup(); * * The return value should always be zero. */ -extern HALIDE_MUST_USE_RESULT int halide_msan_check_memory_is_initialized(void *user_context, const void *ptr, uint64_t len, const char *name); +HALIDE_MUST_USE_RESULT extern int halide_msan_check_memory_is_initialized(void *user_context, const void *ptr, uint64_t len, const char *name); /** Verify that the data pointed to by the halide_buffer_t is initialized (but *not* the halide_buffer_t itself), * using halide_msan_check_memory_is_initialized() for checking. @@ -962,7 +978,7 @@ extern HALIDE_MUST_USE_RESULT int halide_msan_check_memory_is_initialized(void * * * The return value should always be zero. */ -extern HALIDE_MUST_USE_RESULT int halide_msan_check_buffer_is_initialized(void *user_context, struct halide_buffer_t *buffer, const char *buf_name); +HALIDE_MUST_USE_RESULT extern int halide_msan_check_buffer_is_initialized(void *user_context, struct halide_buffer_t *buffer, const char *buf_name); /** Annotate that a given range of memory has been initialized; * only used when Target::MSAN is enabled. @@ -971,7 +987,7 @@ extern HALIDE_MUST_USE_RESULT int halide_msan_check_buffer_is_initialized(void * * * The return value should always be zero. */ -extern HALIDE_MUST_USE_RESULT int halide_msan_annotate_memory_is_initialized(void *user_context, const void *ptr, uint64_t len); +HALIDE_MUST_USE_RESULT extern int halide_msan_annotate_memory_is_initialized(void *user_context, const void *ptr, uint64_t len); /** Mark the data pointed to by the halide_buffer_t as initialized (but *not* the halide_buffer_t itself), * using halide_msan_annotate_memory_is_initialized() for marking. @@ -984,7 +1000,7 @@ extern HALIDE_MUST_USE_RESULT int halide_msan_annotate_memory_is_initialized(voi * * The return value should always be zero. */ -extern HALIDE_MUST_USE_RESULT int halide_msan_annotate_buffer_is_initialized(void *user_context, struct halide_buffer_t *buffer); +HALIDE_MUST_USE_RESULT extern int halide_msan_annotate_buffer_is_initialized(void *user_context, struct halide_buffer_t *buffer); extern void halide_msan_annotate_buffer_is_initialized_as_destructor(void *user_context, void *buffer); /** The error codes that may be returned by a Halide pipeline. */ @@ -1366,7 +1382,7 @@ typedef enum halide_target_feature_t { * bits to represent all the currently known features. Any excess bits must be set to zero. */ // @{ -extern HALIDE_MUST_USE_RESULT int halide_can_use_target_features(int count, const uint64_t *features); +HALIDE_MUST_USE_RESULT extern int halide_can_use_target_features(int count, const uint64_t *features); typedef int (*halide_can_use_target_features_t)(int count, const uint64_t *features); extern halide_can_use_target_features_t halide_set_custom_can_use_target_features(halide_can_use_target_features_t); // @} @@ -1385,7 +1401,7 @@ extern halide_can_use_target_features_t halide_set_custom_can_use_target_feature * return halide_default_can_use_target_features(count, features); * } */ -extern HALIDE_MUST_USE_RESULT int halide_default_can_use_target_features(int count, const uint64_t *features); +HALIDE_MUST_USE_RESULT extern int halide_default_can_use_target_features(int count, const uint64_t *features); typedef struct halide_dimension_t { #if (__cplusplus >= 201103L || _MSVC_LANG >= 201103L) @@ -1903,14 +1919,14 @@ extern double halide_float16_bits_to_double(uint16_t); * If set to false, releases all unused device allocations back to the * underlying device APIs. For finer-grained control, see specific * methods in each device api runtime. */ -extern HALIDE_MUST_USE_RESULT int halide_reuse_device_allocations(void *user_context, bool); +HALIDE_MUST_USE_RESULT extern int halide_reuse_device_allocations(void *user_context, bool); /** Determines whether on device_free the memory is returned * immediately to the device API, or placed on a free list for future * use. Override and switch based on the user_context for * finer-grained control. By default just returns the value most * recently set by the method above. */ -extern HALIDE_MUST_USE_RESULT bool halide_can_reuse_device_allocations(void *user_context); +HALIDE_MUST_USE_RESULT extern bool halide_can_reuse_device_allocations(void *user_context); struct halide_device_allocation_pool { int (*release_unused)(void *user_context); From 12de9313f1085fc828b7a8b5b089b63b37abc110 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Wed, 3 Nov 2021 15:13:29 -0700 Subject: [PATCH 3/3] Oops --- test/generator/memory_profiler_mandelbrot_aottest.cpp | 2 +- test/generator/user_context_insanity_aottest.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/generator/memory_profiler_mandelbrot_aottest.cpp b/test/generator/memory_profiler_mandelbrot_aottest.cpp index 958246285109..7f4294f68965 100644 --- a/test/generator/memory_profiler_mandelbrot_aottest.cpp +++ b/test/generator/memory_profiler_mandelbrot_aottest.cpp @@ -83,7 +83,7 @@ int main(int argc, char **argv) { printf("\n"); int result = halide_do_par_for(nullptr, launcher_task, 0, num_launcher_tasks, nullptr); - assert(result != 0); + assert(result == 0); (void)result; halide_profiler_state *state = halide_profiler_get_state(); diff --git a/test/generator/user_context_insanity_aottest.cpp b/test/generator/user_context_insanity_aottest.cpp index 0bfca12933cc..50fead7b1cc5 100644 --- a/test/generator/user_context_insanity_aottest.cpp +++ b/test/generator/user_context_insanity_aottest.cpp @@ -40,7 +40,7 @@ int main(int argc, char **argv) { // Hijack halide's runtime to run a bunch of instances of this function // in parallel. int result = halide_do_par_for(nullptr, launcher_task, 0, num_launcher_tasks, nullptr); - assert(result != 0); + assert(result == 0); (void)result; for (int i = 0; i < num_launcher_tasks; ++i) {