Skip to content

[RISCV] Remove last use of @llvm.experimental.vp.splat in RISCVCodeGenPrepare. NFCI #170543

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+4 −2
Open

[RISCV] Remove last use of @llvm.experimental.vp.splat in RISCVCodeGenPrepare. NFCI #170543

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions llvm/lib/Target/RISCV/RISCVCodeGenPrepare.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,10 @@ bool RISCVCodeGenPrepare::expandVPStrideLoad(IntrinsicInst &II) {
IRBuilder<> Builder(&II);
Type *STy = VTy->getElementType();
Value *Val = Builder.CreateLoad(STy, BasePtr);
Value *Res = Builder.CreateIntrinsic(Intrinsic::experimental_vp_splat, {VTy},
{Val, II.getOperand(2), VL});
Value *Res = Builder.CreateIntrinsic(
Intrinsic::vp_merge, VTy,
Copy link
Collaborator

@topperc topperc Dec 3, 2025

Choose a reason for hiding this comment

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

Would it be legal to delete this vp_merge since the false value is poison? Are we creating something fragile here that a future optimization may break?

Copy link
Contributor Author

@lukel97 lukel97 Dec 4, 2025

Choose a reason for hiding this comment

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

I guess it's also legal to combine the vp_splat to just a regular splat, although that's a combine not a simplification.

My main motivation for this is that I think at the time experimental.vp.splat was added, we were still going in the direction of emitting VP intrinsics for most things in the loop vectorizer etc. But now that we don't plan on emitting them anymore in the loop vectorizer, having an entire intrinsic for this one expansion seems overkill.

We could avoid the vp_merge and emit a vmv.v.x directly by moving this expansion into RISCVISelLowering like in #97798. But we might not have enough information about whether or not the AVL is non-zero there.

Copy link
Contributor Author

@lukel97 lukel97 Dec 4, 2025

Choose a reason for hiding this comment

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

Now that I think about it, the original motivation for vp.splat was to avoid VL toggles if this expansion occurred in a loop: #98140 (comment)

But I think that should be a solved problem now with RISCVVLOptimizer. So if a vp_merge gets folded to poison then it should still be fine in practice because the VL will get reduced anyway.

The vp_merge here is really only needed to preserve the test cases which return the vector instead of storing it with an EVL.

{II.getOperand(2), Builder.CreateVectorSplat(VTy->getElementCount(), Val),
PoisonValue::get(VTy), VL});

II.replaceAllUsesWith(Res);
II.eraseFromParent();
Expand Down
Loading