-
Notifications
You must be signed in to change notification settings - Fork 1k
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
cpu: aarch64: Expand ARM SVE support in jit_uni_softmax #1786
cpu: aarch64: Expand ARM SVE support in jit_uni_softmax #1786
Conversation
@@ -665,6 +665,283 @@ struct jit_softmax_t<sve_512> : public jit_softmax_base_t<sve_512> { | |||
} | |||
}; // namespace aarch64 | |||
|
|||
template <> | |||
struct jit_softmax_t<sve_256> : public jit_softmax_base_t<sve_256> { |
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 there a reason for repeating the code for SVE256 - which is the same as SVE512 from Lines 389? Can there be just one jit_softmax_t and then there be entries for SVE512, 256 etc? (See #1445)
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.
@cfRod, there is no specific reason for repeating the code for SVE256; it was done for faster development.
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.
Ok fair enough
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.
Thanks @cfRod
@cfRod, do you have any comments from your side? |
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.
Suggested for a refactor but happy to approve either way.
@densamoilov , Hello the changes are approved from @cfRod |
Thank you for the contribution! |
Thank you @densamoilov @dzarukin for your quick response and merging the PR |
Description
Enhancement: Expand ARM SVE support in jit_uni_softmax
This commit enhances the existing ARM SVE support in jit_uni_softmax to include additional vector length compatibility. The changes made are for implementation of different ARM SVE vector length.
Major Code changes:
• Introduce a new struct to accommodate the extended ARM SVE vector length, similar to the sve_512 struct.
• Modify the get_horizontal_op function accordingly for enhanced vector length.
Checklist
General
make test
andmake test_benchdnn_*
) pass locally for each commit? YesTest output is same with and without this commit.
make test output:
make test_benchdnn_* output:
make test_benchdnn_softmax_ci_cpu/fast output:
cc: @kawakami-k , @davsva01