-
Notifications
You must be signed in to change notification settings - Fork 534
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
Cache backend symbols in the unified backend #1770
Conversation
src/api/unified/symbol_manager.hpp
Outdated
typedef af_err(*af_func)(CalleeArgs...); | ||
af_func funcHandle; | ||
static std::unordered_map<const char*, std::array<af_func, NUM_BACKENDS>> funcHandles; | ||
af_func& funcHandle = funcHandles[symbolName][getActiveBackend()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it always return NULL
when it is not found ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am 96.3% sure it does.
src/api/unified/symbol_manager.hpp
Outdated
typedef af_err(*af_func)(CalleeArgs...); | ||
af_func funcHandle; | ||
static std::unordered_map<const char*, std::array<af_func, NUM_BACKENDS>> funcHandles; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the array outside ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/api/unified/symbol_manager.hpp
Outdated
static std::unordered_map<const char*, std::array<af_func, NUM_BACKENDS>> funcHandles; | ||
af_func& funcHandle = funcHandles[symbolName][getActiveBackend()]; | ||
static std::array<std::unordered_map<const char*, af_func>, NUM_BACKENDS> funcHandles; | ||
af_func& funcHandle = funcHandles[getActiveBackend()][symbolName]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is getActiveBackend
returning the enum or is it returning the index ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops. It's a flag. Fixing.
case AF_BACKEND_CPU: return 0; | ||
case AF_BACKEND_CUDA: return 1; | ||
case AF_BACKEND_OPENCL: return 2; | ||
default: return -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, we have something called AF_BACKEND_DEFAULT
. Please check if it works with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SymbolManager and setBackend function sets the activeBackend to one of those backends.
@umar456 Alternative suggestion: Instead of querying from an This reduces the lookup time from the map. You will need change the definition of |
I tried that first but the call function's template parameters are based on the type of arguments and not the function name itself. I could change the macro so that we only have an array instead of a map but I didn't want to break anything. I can change it if you think its worthwhile. |
We can still pass the arguments, but instead of calling the function, it will just return the function pointer. The call can be made externally. No reason to cache the symbol inside the |
build arrayfire windows ci |
81253ee
to
3730fb4
Compare
Test code: