Skip to content

[CANN] Adapt to dynamically loadable backends mechanism #9970

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 22, 2024
Merged

[CANN] Adapt to dynamically loadable backends mechanism #9970

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1657447
[CANN] Adapt to dynamically loadable backends mechanism
leo-pony Oct 21, 2024
4d4ae1c
Fix the Bug: inference running result is garbled in debug running mod…
leo-pony Oct 21, 2024
abf5be4
Handle the review comments of this pull request
leo-pony Oct 22, 2024
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
3 changes: 3 additions & 0 deletions ggml/include/ggml-cann.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ extern "C" {
* @brief Maximum number of CANN devices supported.
*/
#define GGML_CANN_MAX_DEVICES 16
#define GGML_CANN_NAME "CANN"
Copy link
Member

Choose a reason for hiding this comment

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

I suspect that this macro was taken from the implementation in the CUDA backend, but the reason it exists there is because the same code is used to build the ROCm and MUSA backends, so the name of the backend may change depending on the build flags, but I don't think this is necessary in the CANN backend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review, this macro is no longer exposed to llama. I have moved this macro to the cann implementation file.


GGML_API ggml_backend_reg_t ggml_backend_cann_reg(void);

/**
* @brief Initializes the CANN backend for a specified device.
Expand Down
10 changes: 9 additions & 1 deletion ggml/src/ggml-backend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,10 @@ void * ggml_backend_reg_get_proc_address(ggml_backend_reg_t reg, const char * na
# include "ggml-amx.h"
#endif

#ifdef GGML_USE_CANN
#include "ggml-cann.h"
#endif

struct ggml_backend_registry {
std::vector<ggml_backend_reg_t> backends;
std::vector<ggml_backend_dev_t> devices;
Expand All @@ -584,11 +588,15 @@ struct ggml_backend_registry {
#ifdef GGML_USE_RPC
register_backend(ggml_backend_rpc_reg());
#endif

#ifdef GGML_USE_AMX
register_backend(ggml_backend_amx_reg());
#endif

// TODO: kompute, cann
#ifdef GGML_USE_CANN
register_backend(ggml_backend_cann_reg());
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Do not add extra lines, use the same format as the rest of the backends.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review, I have removed the blank lines and the formatting is consistent with the rest of the backends. Function is normal after removing.

// TODO: kompute

register_backend(ggml_backend_cpu_reg());
}
Expand Down
Loading
Loading