Skip to content
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

[RISCV] Add a combine to form masked.load from unit strided load #65674

Merged
merged 2 commits into from
Sep 11, 2023

Conversation

preames
Copy link
Collaborator

@preames preames commented Sep 7, 2023

Add a DAG combine to form a masked.load from a masked_strided_load intrinsic with stride equal to element size. This covers a couple of extra test cases, and allows us to simplify and common some existing code on the concat_vector(load, ...) to strided load transform.

This is the first in a mini-patch series to try and generalize our strided load and gather matching to handle more cases, and common up different approaches to the same problems in different places.

Add a DAG combine to form a masked.load from a masked_strided_load intrinsic with stride equal to element size.  This covers a couple of extra test cases, and allows us to simplify and common some existing code on the concat_vector(load, ...) to strided load transform.
@preames preames requested a review from a team as a code owner September 7, 2023 21:04

// If the stride is equal to the element size in bytes, we can use
// a masked.load.
const unsigned ElementSizeInBits = VT.getScalarType().getSizeInBits();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getScalarStoreSize()? Which will return the size in bytes.

@preames
Copy link
Collaborator Author

preames commented Sep 11, 2023

ping

Copy link
Contributor

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Should we be canonicalising this for Intrinsic::riscv_vlse and Intrinsic::riscv_vlse_mask too?

@preames
Copy link
Collaborator Author

preames commented Sep 11, 2023

LGTM. Should we be canonicalising this for Intrinsic::riscv_vlse and Intrinsic::riscv_vlse_mask too?

Yes. However, I'm focused on generic IR codegen and don't care too much about intrinsic usage.

(Edit: For context, the strided load intrinsic is mostly a detail of our gather lowering, the others are user written intrinsics.)

@preames preames merged commit 5352c79 into llvm:main Sep 11, 2023
2 checks passed
AntonRydahl pushed a commit to AntonRydahl/llvm-project that referenced this pull request Sep 11, 2023
…m#65674)

Add a DAG combine to form a masked.load from a masked_strided_load
intrinsic with stride equal to element size. This covers a couple of
extra test cases, and allows us to simplify and common some existing
code on the concat_vector(load, ...) to strided load transform.

This is the first in a mini-patch series to try and generalize our
strided load and gather matching to handle more cases, and common up
different approaches to the same problems in different places.
preames added a commit to preames/llvm-project that referenced this pull request Sep 18, 2023
Add a DAG combine to form a masked.store from a masked_strided_store intrinsic
with stride equal to element size. This is the store analogy to PR llvm#65674.

As seen in the tests, this does pickup a few cases that we'd previously missed
due to selection ordering.  We match strided stores early without going through
the recently added generic mscatter combines, and thus weren't recognizing the
unit strided store.
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
…m#65674)

Add a DAG combine to form a masked.load from a masked_strided_load
intrinsic with stride equal to element size. This covers a couple of
extra test cases, and allows us to simplify and common some existing
code on the concat_vector(load, ...) to strided load transform.

This is the first in a mini-patch series to try and generalize our
strided load and gather matching to handle more cases, and common up
different approaches to the same problems in different places.
lukel97 added a commit to lukel97/llvm-project that referenced this pull request Sep 19, 2023
This is the VP equivalent of llvm#65674. We already combine MGATHER loads with unit
stride to MLOAD, so this extends it for EXPERIMENTAL_VP_STRIDED_LOAD.
preames added a commit that referenced this pull request Sep 19, 2023
Add a DAG combine to form a masked.store from a masked_strided_store intrinsic
with stride equal to element size. This is the store analogy to PR #65674.

As seen in the tests, this does pickup a few cases that we'd previously missed
due to selection ordering.  We match strided stores early without going through
the recently added generic mscatter combines, and thus weren't recognizing the
unit strided store.
lukel97 added a commit that referenced this pull request Sep 19, 2023
…6766)

This is the VP equivalent of #65674. We already combine MGATHER loads
with unit stride to MLOAD, so this extends it for
EXPERIMENTAL_VP_STRIDED_LOAD.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants