elliottt opened PR #6070 from trevor/restrict-isplit-iconcat
to main
:
All backends currently assume that
isplit
andiconcat
only work for splittingi128
values, or concatenating twoi64
values to make ani128
value. This PR changes the instruction definition to reflect that assumption.
<!--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.
-->
elliottt has marked PR #6070 as ready for review.
elliottt updated PR #6070 from trevor/restrict-isplit-iconcat
to main
.
elliottt updated PR #6070 from trevor/restrict-isplit-iconcat
to main
.
elliottt updated PR #6070 from trevor/restrict-isplit-iconcat
to main
.
elliottt updated PR #6070 from trevor/restrict-isplit-iconcat
to main
.
elliottt edited PR #6070 from trevor/restrict-isplit-iconcat
to main
:
All backends currently assume that
isplit
andiconcat
only work for splittingi128
values, or concatenating twoi64
values to make ani128
value. This PR changes the instruction definition to reflect that assumption.This PR makes the
HalfWidth
andDoubleWidth
constraints never constructed, but I think we should leave these two constructors marked#[allow(unused)]
rather than remove them, as we might want to relax the constraints onisplit
andiconcat
in the future.
<!--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.
-->
elliottt edited PR #6070 from trevor/restrict-isplit-iconcat
to main
:
All backends currently assume that
isplit
andiconcat
only work for splittingi128
values, or concatenating twoi64
values to make ani128
value. This PR changes the instruction definition to reflect that assumption.This PR makes the
HalfWidth
andDoubleWidth
constraints never constructed as the uses of those constraints in the instruction DSL now produces singleton sets. However, I think we should leave these two constructors marked#[allow(unused)]
rather than remove them, as we might want to relax the constraints onisplit
andiconcat
in the future.
<!--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.
-->
elliottt updated PR #6070 from trevor/restrict-isplit-iconcat
to main
.
elliottt updated PR #6070 from trevor/restrict-isplit-iconcat
to main
.
elliottt edited PR #6070 from trevor/restrict-isplit-iconcat
to main
:
All backends currently assume that
isplit
andiconcat
only work for splittingi128
values, or concatenating twoi64
values to make ani128
value. The type constraints for bot indicate that they can work over vectors as well as other bit-width integers. This PR resolves part of this discrepancy by removing vector support from bothisplit
andiconcat
.
<!--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.
-->
elliottt edited PR #6070 from trevor/restrict-isplit-iconcat
to main
:
All backends currently assume that
isplit
andiconcat
only work for splittingi128
values, or concatenating twoi64
values to make ani128
value. The type constraints for both indicate that they can work over vectors as well as other bit-width integers. This PR resolves part of this discrepancy by removing vector support from bothisplit
andiconcat
.
<!--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.
-->
jameysharp submitted PR review.
jameysharp created PR review comment:
The other place that
NarrowInt
is used is foriconst
. I recently asked Chris about whether we should be making use oficonst
's documented ability to construct a vector, and he was surprised that it was documented that way. Perhaps we should just removesimd_lanes
from the original definition ofNarrowInt
instead of shadowing it here?
elliottt submitted PR review.
elliottt created PR review comment:
Happy to make that change as well.
elliottt updated PR #6070 from trevor/restrict-isplit-iconcat
to main
.
elliottt updated PR #6070 from trevor/restrict-isplit-iconcat
to main
.
jameysharp submitted PR review.
jameysharp created PR review comment:
Apparently a couple of parser filetests use
iconst
with vector types, but since they're only testing the parser I don't think we should take that as a sign that this is actually used.In
rewrite.clif
I think you can just change it to aniconst.i32
. Or possibly delete the test in question entirely, because I don't think it's testing anything related to what the file-header comment claims to be testing.In
tiny.clif
I guess the test is only supposed to check that the parser can parse lane indexes forextractlane
andinsertlane
. Makingv0
be ani32x4
parameter instead of the result of aniconst
should do that just as well, I think.
elliottt updated PR #6070 from trevor/restrict-isplit-iconcat
to main
.
jameysharp submitted PR review.
elliottt merged PR #6070.
Last updated: Dec 23 2024 at 12:05 UTC