[HVX] Fix state_var issue #6894
Conversation
|
@pranavb-ca might be able to weigh in |
src/runtime/hexagon_host.cpp
Outdated
| @@ -755,6 +755,13 @@ WEAK uint64_t halide_hexagon_get_device_size(void *user_context, struct halide_b | |||
| return handle->size; | |||
| } | |||
|
|
|||
| WEAK void *halide_hexagon_get_module_state(void **host) { | |||
| if (host == nullptr) { | |||
| return nullptr; | |||
There was a problem hiding this comment.
Is nullptr a value that the caller can deal with intelligently? (i.e., is this an error condition?)
There was a problem hiding this comment.
I'm not quite sure. This check doesn't exist in the current implementation, so perhaps I should remove it. I wanted to avoid null pointer dereferencing.
There was a problem hiding this comment.
Always a good idea. Might be worth throwing an abort() or similar in the null path to see?
There was a problem hiding this comment.
Sure, just tried it out in 2df1b2c
|
LGTM. @aankit-ca can help you test this if you need. (sorry, i am on vacation with limited access to my work machine). |
|
Thanks for taking a look @pranavb-ca - sorry for bothering you on your vacation! |
|
So how confident are we in this change? Do we have a way to verify that the null-check code is actually being exercised? |
|
I don't think we have a way to verify that it is, however this change is actually safer - the current version in main will just load without the nullptr check. |
|
@aankit-ca Any chance you could look at this? #6884 is stalled until this PR goes through |
|
Monday review ping to @aankit-ca |
|
bump |
|
If @aankit-ca isn't available for this review, we should either find someone else or just land it as-is. |
* fix HVX state_var issue * abort if host is nullptr
The issue in #6893 is the result of an attempt to dereference a pointer to the module_state pointer. This PR simply adds a runtime method to dereference the pointer, in order to fix that issue. I'm not necessarily sure if this is the best approach, would love feedback from anyone who knows the HVX runtime better than I do.