abrown opened PR #2813 from inst-format-2
to main
:
Because there are instructions that are present in more than one ISA feature set, we need to see if any of them match before emitting. This change includes the
VPABSQ
instruction as an example, which is present in bothAVX512F
andAVX512VL
.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
abrown requested cfallin for a review on PR #2813.
abrown requested cfallin and bnjbvr for a review on PR #2813.
abrown edited PR #2813 from inst-format-2
to main
:
Because there are instructions that are present in more than one ISA feature set, we need to see if any of the ISA requirements match before emitting. This change includes the
VPABSQ
instruction as an example, which is present in bothAVX512F
andAVX512VL
.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
abrown updated PR #2813 from inst-format-2
to main
.
cfallin submitted PR Review.
cfallin submitted PR Review.
cfallin created PR Review Comment:
Perhaps we could rename this to something like
present_in_isas()
? TheOR
semantics weren't immediately clear to me from the name until I read the code around the callsite above; otherwise I would have expectedAND
.
bnjbvr submitted PR Review.
bnjbvr created PR Review Comment:
uber nit: do you mind writing it in the final reduced form,
if !isa.is_empty() && !isa.iter().any(matches_isa_flags)
?
bnjbvr submitted PR Review.
bnjbvr created PR Review Comment:
Or how about
present_in_any_isa()
? (oravailable_in_any_isa()
?) The "any" in the name makes theOR
semantics crystal clear in my opinion.Also nit: considering a
SmallVec<[T; 1]>
will cause a heap allocation for the second element, and we only have the cases for 1 and 2 elements, could this returnSmallVec<[T; 2]>
(or 4; an initialVec
push will reserve capacity for 4 elements)?
abrown updated PR #2813 from inst-format-2
to main
.
abrown updated PR #2813 from inst-format-2
to main
.
abrown submitted PR Review.
abrown created PR Review Comment:
Went with
available_in_any_isa
, added some documentation to that function, and updated to useSmallVec<[T; 2]>
.
abrown merged PR #2813.
Last updated: Dec 23 2024 at 12:05 UTC