-
Notifications
You must be signed in to change notification settings - Fork 12.2k
[CANN]Support operator SIN COS ARGMAX #12709
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
Conversation
Signed-off-by: noemotiovon <[email protected]>
ggml/src/ggml-cann/aclnn_ops.cpp
Outdated
ACL_CHECK(aclnnArgMax(workspaceAddr, workspaceSize, executor, ctx.stream())); | ||
|
||
size_t cpy_size = ggml_nbytes(dst); | ||
ACL_CHECK(aclrtMemcpyAsync(dst->data, cpy_size, buffer, cpy_size, |
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 extra copy here is necessary because the shape computed by the aclnn operator differs from dst, so a buffer is used to hold the data before copying it back to dst.
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.
No need to alloc a extra buffer. use dst->data instead.
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.
Thank you for your suggestion! I forgot to directly modify dst->data's shape, haha.
ggml/src/ggml-cann/aclnn_ops.cpp
Outdated
|
||
aclTensor* acl_src = ggml_cann_create_tensor(src0); | ||
aclTensor* acl_dst = ggml_cann_create_tensor(dst); | ||
aclnn_argmax(ctx, acl_src, acl_dst, dst); |
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.
It seems aclnn_argmax need dst, and acl_dst can be created in aclnn_argmax, and do not need pass as a parameter.
Besides, If aclnn_argmax is not used by other internal functions, these two function should combine together.
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.
Okay, I will inline this method.
Signed-off-by: noemotiovon <[email protected]>
Signed-off-by: noemotiovon <[email protected]>
Why is this PR needed?
Optimize the
sin
,cos
,argmax
operator in the CANN backend with the aclnn acceleration library.Test