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

No longer silently hide errors in Metal completion handlers (alternative approach) #8240

Merged

Conversation

shoaibkamil
Copy link
Contributor

As discussed in the dev meeting, here is an alternative approach for fixing #7780. Now, the runtime saves an error during the callback from a failed command buffer, and reports the error back at the next opportunity for reporting. Note that this means that a pipeline may get an error that should be attributed to the previous pipeline, but additional error information can be added (e.g. prepending the error message with the name of the kernel that caused it) if wanted.

@shoaibkamil shoaibkamil requested a review from slomp May 28, 2024 16:56
@shoaibkamil
Copy link
Contributor Author

Sorry @steven-johnson, I brain-farted and hadn't pushed the branch I intended to push when I opened this. Should be correct now.

@slomp
Copy link
Contributor

slomp commented May 28, 2024

@shoaibkamil I recommend pre-pending the error message with something that reminds the user that this error has happened unexpectedly during the execution of a previous pipeline.

@steven-johnson
Copy link
Contributor

C:\build_bot\worker\halide-testbranch-main-llvm19-x86-64-windows-cmake\halide-source\test\correctness\gpu_metal_completion_handler_error_check.cpp(2): fatal error C1083: Cannot open include file: 'unistd.h': No such file or directory

src/runtime/metal.cpp Show resolved Hide resolved
src/runtime/metal.cpp Outdated Show resolved Hide resolved
@shoaibkamil
Copy link
Contributor Author

@steven-johnson I've substantially revamped the API here to enable users to pass a user context. PTAL

@shoaibkamil shoaibkamil requested a review from zvookin June 10, 2024 18:20
@steven-johnson steven-johnson self-requested a review June 10, 2024 22:48
Copy link
Contributor

@steven-johnson steven-johnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending green buildbots

@shoaibkamil
Copy link
Contributor Author

Investigating crash, and will add tests to makefile.

@shoaibkamil
Copy link
Contributor Author

All green finally, modulor vulkan errors.

@shoaibkamil shoaibkamil merged commit f9ccd5c into main Jun 14, 2024
14 of 17 checks passed
@shoaibkamil shoaibkamil deleted the shoaibkamil/metal_completion_handler_error_alternative branch June 14, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants