[RISCV] Use a DAG combine to prune pointless vrgather.vi #135392
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the vrgather.vi is preceeded by a vmv.v.x which writes a superset of the lanes writen by the vrgather, and the vrgather has no passthru, then the vrgather has no semantic effect. This is the start of a mini-series of patches around rewriting vrgather.vi/vx preceeded by vmv.v.x, vfmf.v.f, vmv.s.x, etc... Starting with the simplest, but also lowest impact. One point I'd like a second oppinion on is the out of bounds semenatic change. As far as I can tell, all the indices are in bounds by construction. The doc change is as much as I couldn't figure out how to test the alternative as anything else.
|
@llvm/pr-subscribers-backend-risc-v Author: Philip Reames (preames) ChangesIf the vrgather.vi is preceeded by a vmv.v.x which writes a superset of the lanes writen by the vrgather, and the vrgather has no passthru, then the vrgather has no semantic effect. This is the start of a mini-series of patches around rewriting vrgather.vi/vx preceeded by vmv.v.x, vfmf.v.f, vmv.s.x, etc... Starting with the simplest, but also lowest impact. One point I'd like a second oppinion on is the out of bounds semenatic change. As far as I can tell, all the indices are in bounds by construction. The doc change is as much as I couldn't figure out how to test the alternative as anything else. Full diff: https://github.com/llvm/llvm-project/pull/135392.diff 3 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index fef9084bd0e73..e5843477e04e5 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -19709,6 +19709,19 @@ SDValue RISCVTargetLowering::PerformDAGCombine(SDNode *N,
if (SDValue V = combineToVCPOP(N, DAG, Subtarget))
return V;
break;
+ case RISCVISD::VRGATHER_VX_VL: {
+ // Drop a redundant vrgather_vx.
+ // Note this assumes that out of bounds indices produce poison
+ // and can thus be replaced without having to prove them inbounds..
+ SDValue Src = N->getOperand(0);
+ SDValue Passthru = N->getOperand(2);
+ SDValue VL = N->getOperand(4);
+ // TODO: Handle fmv.v.f?
+ if (Src.getOpcode() == RISCVISD::VMV_V_X_VL && Passthru.isUndef() &&
+ VL == Src.getOperand(2))
+ return Src;
+ break;
+ }
}
return SDValue();
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.h b/llvm/lib/Target/RISCV/RISCVISelLowering.h
index 5ebdbbd51f2b1..d624ee33d6a63 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.h
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.h
@@ -388,7 +388,8 @@ enum NodeType : unsigned {
VMSET_VL,
// Matches the semantics of vrgather.vx and vrgather.vv with extra operands
- // for passthru and VL. Operands are (src, index, mask, passthru, vl).
+ // for passthru and VL, except that out of bound indices result in a poison
+ // result not zero. Operands are (src, index, mask, passthru, vl).
VRGATHER_VX_VL,
VRGATHER_VV_VL,
VRGATHEREI16_VV_VL,
diff --git a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-int.ll b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-int.ll
index e6375e276d37f..185b8b6d03134 100644
--- a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-int.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-int.ll
@@ -1346,10 +1346,9 @@ define <4 x i16> @vmerge_2(<4 x i16> %x) {
define <4 x i16> @vmerge_3(<4 x i16> %x) {
; CHECK-LABEL: vmerge_3:
; CHECK: # %bb.0:
-; CHECK-NEXT: vsetivli zero, 4, e16, mf2, ta, mu
+; CHECK-NEXT: vsetivli zero, 4, e16, mf2, ta, ma
; CHECK-NEXT: vmv.v.i v0, 6
-; CHECK-NEXT: vmv.v.i v9, 5
-; CHECK-NEXT: vrgather.vi v8, v9, 1, v0.t
+; CHECK-NEXT: vmerge.vim v8, v8, 5, v0
; CHECK-NEXT: ret
%s = shufflevector <4 x i16> %x, <4 x i16> <i16 poison, i16 5, i16 poison, i16 poison>, <4 x i32> <i32 0, i32 5, i32 5, i32 3>
ret <4 x i16> %s
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/139/builds/13436 Here is the relevant piece of the build log for the reference |
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
If the vrgather.vi is preceeded by a vmv.v.x which writes a superset of the lanes writen by the vrgather, and the vrgather has no passthru, then the vrgather has no semantic effect. This is the start of a mini-series of patches around rewriting vrgather.vi/vx preceeded by vmv.v.x, vfmf.v.f, vmv.s.x, etc... Starting with the simplest, but also lowest impact. One point I'd like a second oppinion on is the out of bounds semenatic change. As far as I can tell, all the indices are in bounds by construction. The doc change is as much as I couldn't figure out how to test the alternative as anything else.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the vrgather.vi is preceeded by a vmv.v.x which writes a superset of the lanes writen by the vrgather, and the vrgather has no passthru, then the vrgather has no semantic effect.
This is the start of a mini-series of patches around rewriting vrgather.vi/vx preceeded by vmv.v.x, vfmf.v.f, vmv.s.x, etc... Starting with the simplest, but also lowest impact.
One point I'd like a second oppinion on is the out of bounds semenatic change. As far as I can tell, all the indices are in bounds by construction. The doc change is as much as I couldn't figure out how to test the alternative as anything else.