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

d3d12 runtime: replacing spinlocks by mutex objects #7489

Merged
merged 2 commits into from
Apr 11, 2023
Merged

Conversation

slomp
Copy link
Contributor

@slomp slomp commented Apr 7, 2023

No description provided.

@slomp slomp closed this Apr 8, 2023
@slomp slomp reopened this Apr 8, 2023
@slomp
Copy link
Contributor Author

slomp commented Apr 10, 2023

I reverted those two WEAK -> static changes. I'll have to revisit the runtime in another PR to fix all the weak linkage hints.

@slomp
Copy link
Contributor Author

slomp commented Apr 10, 2023

In fact, expect a lot of PRs with changes and improvements coming to the d3d12 back-end.

@steven-johnson
Copy link
Contributor

In fact, expect a lot of PRs with changes and improvements coming to the d3d12 back-end.

If you are going to spend time here, it could use a some attention to error handling. I'll post details if you like.

@slomp
Copy link
Contributor Author

slomp commented Apr 10, 2023

In fact, expect a lot of PRs with changes and improvements coming to the d3d12 back-end.

If you are going to spend time here, it could use a some attention to error handling. I'll post details if you like.

Yup, better error handling (and reporting) is also one of the goals.
The poor d3d12 back-end has been neglected for far too long :-)

Feel free to voice your concerns and advice.
(Maybe on a new issue, instead of on this PR?)

@steven-johnson
Copy link
Contributor

Feel free to voice your concerns and advice. (Maybe on a new issue, instead of on this PR?)

The existing issue for Runtime error handling is #7350, but it's more about coming up with a design that is less mistake-prone, rather than defining the status quo.

TL;DR for best practices for the status quo:

  • ("Public-facing functions" means a Halide Runtime function that is either defined in HalideRuntime.h, or called by Halide-generated code)
  • Public-facing functions should always return a value that is a valid halide_error_code_t (rather than some random integer)
  • Public-facing functions should always do both of these when an error occurs:
    • call halide_error() or error()
    • return a valid, nonzero halide_error_code_t as the function result
  • Don't ever use halide_abort_if_false():
    • Runtime code should only call abort() if it detects a situation in which it is unsafe for the current process to continue executing -- e.g., memory corruption. This should be infrequent enough that we don't need a macro for it, we can just call it explicitly.
    • Validating things like non-null argument should be mostly handled by halide_debug_assert() (which mainly defines an expected invariant, but only enforces in Debug runtimes); validation that need to happen in non-Debug runtimes should be done by explicit C++ of the form if (bad-thing) { error() << "Bad thing Detected"; return halide_error_code_bad_thing; }

(I'm copying this to #7350 for future reference)

@slomp
Copy link
Contributor Author

slomp commented Apr 11, 2023

All good to merge this branch?

@slomp slomp merged commit 93e3d39 into main Apr 11, 2023
@slomp slomp deleted the slomp/d3d12-mutex branch April 11, 2023 16:34
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
* replacing spinlock by mutex

* adding back weak linkage hint

---------

Co-authored-by: Marcos Slomp <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants