-
Notifications
You must be signed in to change notification settings - Fork 66
Add intrinsics for the FEAT_SVE2p2/FEAT_SME2p2 features introduced by the 2024 dpISA #412
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
base: main
Are you sure you want to change the base?
Conversation
b421a82 to
2dd7a0d
Compare
|
Corrected offline that these intrinsics are all already present, and will just be simplified by the new instructions. |
2dd7a0d to
137ff03
Compare
main/acle.md
Outdated
| [`<arm_sve.h>`](#arm_sve.h) or [`<arm_sme.h>`](#arm_sme.h) | ||
| when `__ARM_FEATURE_SVE2p2` or `__ARM_FEATURE_SME2p2` is defined, respectively. | ||
|
|
||
| These intrinsics can only be called from non-streaming code if |
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.
So this is not correct. These instruction can be called in non-streaming mode if we have sme2p2 and sve as well.
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.
For FIRSTP and LASTP you can also run them in streaming mode if you have sve2p2 and sme.
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've updated the conditions. Please can you have another look?
137ff03 to
7de9269
Compare
Lukacma
left a comment
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.
LGTM
main/acle.md
Outdated
| `__ARM_FEATURE_SME2p2` feature macro is defined. | ||
|
|
||
| They can be called from non-streaming code if the `__ARM_FEATURE_SVE2p2` feature | ||
| macro is defined or both the `__ARM_FEATURE_SVE` and `__ARM_FEATURE_SME2p2` |
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 not sure this is correct:
__ARM_FEATURE_SVE and __ARM_FEATURE_SME2p2
should it be:
__ARM_FEATURE_SME and __ARM_FEATURE_SME2p2
Why are you writing that it needs SVE, have you seeing this in the pseudo code from the instruction?
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.
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 size that is now available also in sme2p2 and sve is Word and doubleword.
You are adding new ones that are only available for sve2p2 or sme2p2
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.
Removed those conditions because it is simply repeating information already available in the ISA docs.
main/acle.md
Outdated
|
|
||
| Scalar index of first/last true predicate element (predicated). | ||
|
|
||
| These intrinsics can be called from streaming mode if either of the feature |
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 believe this is incorrect. Can you remove these sentences?
They need sve2p2 or sme2p2, there is nothing saying about sve or sme.
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.
Similar to COMPACT, this is also something I understood from the CheckSVEEnabled pseudocode used by FIRSTP
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.
That is when the instructions is defined:
if !IsFeatureImplemented(FEAT_SVE2p2) && !IsFeatureImplemented(FEAT_SME2p2) then
CheckSVEEnabled is how it will run, in streaming mode or sve.
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.
Removed those conditions because it is simply repeating information already available in the ISA docs.
main/acle.md
Outdated
|
|
||
| Copy active vector elements to/from lower-numbered elements. | ||
|
|
||
| These intrinsics can be called from streaming code only if the |
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 believe this sentence are applied to all sve2p2 and sme2p2 instruction you can move it up. After line 13601
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.
Removed those conditions because it is simply repeating information already available in the ISA docs.
ff8fbf9 to
777686d
Compare
|
|
||
| ``` c | ||
| // Variants are also available for: | ||
| // [_single_f32_x2] |
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 think we should split this into x2 and x4 variants. I think this might confuse users as to what type to use for arguments.
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've split them up. Does this look okay?
These instructions are available under features FEAT_SVE2p2 or FEAT_SME2p2. COMPACT: Copy Active vector elements to lower-numbered elements (Byte/Halfword variants) EXPAND: Copy lower-numbered vector elements to Active elements (Byte/Halfword/Word/Doubleword variants) FIRSTP: Scalar index of first true predicate element (predicated) (Byte/Halfword/Word/Doubleword variants) LASTP: Scalar index of last true predicate element (predicated) (Byte/Halfword/Word/Doubleword variants) FMUL (multiple and single vector): Multi-vector floating-point multiply by vector FMUL (multiple vectors): Multi-vector floating-point multiply
1. Change firstp/lastp parameter names to match the register names: op -> pn 2. split the x2 and x4 variants of the FMUL intrinsics
5f3e66f to
29f8554
Compare
| extended in the future. | ||
|
|
||
| The intrinsics in this section are defined by the header file | ||
| [`<arm_sme.h>`](#arm_sme.h) when `__ARM_FEATURE_SME2p2` is defined. |
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.
Since these instructions do not operate on ZA array, I think they should be in arm_sve.h.
name: Pull request
about: Technical issues, document format problems, bugs in scripts or feature proposal.
Thank you for submitting a pull request!
If this PR is about a bugfix:
Please use the bugfix label and make sure to go through the checklist below.
If this PR is about a proposal:
We are looking forward to evaluate your proposal, and if possible to
make it part of the Arm C Language Extension (ACLE) specifications.
We would like to encourage you reading through the contribution
guidelines, in particular the section on submitting
a proposal.
Please use the proposal label.
As for any pull request, please make sure to go through the below
checklist.
Checklist: (mark with
Xthose which apply)PR (do not bother creating the issue if all you want to do is
fixing the bug yourself).
SPDX-FileCopyrightTextlines on topof any file I have edited. Format is
SPDX-FileCopyrightText: Copyright {year} {entity or name} <{contact informations}>(Please update existing copyright lines if applicable. You can
specify year ranges with hyphen , as in
2017-2019, and usecommas to separate gaps, as in
2018-2020, 2022).Copyrightsection of the sources of thespecification I have edited (this will show up in the text
rendered in the PDF and other output format supported). The
format is the same described in the previous item.
tricky to set up on non-*nix machines). The sequence can be
found in the contribution
guidelines. Don't
worry if you cannot run these scripts on your machine, your
patch will be automatically checked in the Actions of the pull
request.
introduced in this PR in the section Changes for next
release of the section Change Control/Document history
of the document. Create Changes for next release if it does
not exist. Notice that changes that are not modifying the
content and rendering of the specifications (both HTML and PDF)
do not need to be listed.
correctness of the result in the PDF output (please refer to the
instructions on how to build the PDFs
locally).
draftversionis set totruein the YAML headerof the sources of the specifications I have modified.
in the README page of the project.