Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotate all runtime functions with HALIDE_MUST_USE_RESULT #6388

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/lens_blur/process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t> left_im = load_image(argv[1]);
Buffer<uint8_t> right_im = load_image(argv[1]);
Expand Down
25 changes: 17 additions & 8 deletions src/CodeGen_LLVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 8 additions & 4 deletions src/runtime/HalideBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,15 +240,19 @@ 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()");
(void)result;
}
if (dev_ref_count) {
if (dev_ref_count->ownership == BufferDeviceOwnership::Cropped) {
Expand Down
208 changes: 112 additions & 96 deletions src/runtime/HalideRuntime.h

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion src/runtime/cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/device_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down
21 changes: 17 additions & 4 deletions src/runtime/halide_buffer_t.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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];
Expand All @@ -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()) {
Expand Down
10 changes: 8 additions & 2 deletions src/runtime/matlab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/runtime/msan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/tracing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
5 changes: 4 additions & 1 deletion src/runtime/write_debug_image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
6 changes: 5 additions & 1 deletion test/generator/buffer_copy_aottest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ int main(int argc, char **argv) {
Buffer<int> 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<int> in_crop = input.cropped(0, 32, 64).cropped(1, 32, 64);
out.for_each_value([&](int a, int b) {
Expand Down
6 changes: 5 additions & 1 deletion test/generator/gpu_object_lifetime_aottest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,11 @@ int main(int argc, char **argv) {
{
Buffer<int> 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
Expand Down
4 changes: 3 additions & 1 deletion test/generator/memory_profiler_mandelbrot_aottest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 5 additions & 2 deletions test/generator/user_context_insanity_aottest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion tools/RunGenMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down