Skip to content

[CG][RISCV]Fix shuffling of odd number of input vectors #125693

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

Merged
merged 5 commits into from
Feb 5, 2025
Merged

[CG][RISCV]Fix shuffling of odd number of input vectors #125693

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dc89ff2
[𝘀𝗽𝗿] initial version
alexey-bataev Feb 4, 2025
9339ca5
Fix formatting
alexey-bataev Feb 4, 2025
6d607bd
Remove extra checks and simplify code
alexey-bataev Feb 4, 2025
ca2bd3f
Rebase
alexey-bataev Feb 5, 2025
466ac09
Rebase
alexey-bataev Feb 5, 2025
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
26 changes: 20 additions & 6 deletions llvm/lib/Target/RISCV/RISCVISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5213,13 +5213,27 @@ static SDValue lowerShuffleViaVRegSplitting(ShuffleVectorSDNode *SVN,
for (unsigned I : seq<unsigned>(Data.size())) {
const auto &[Idx1, Idx2, _] = Data[I];
if (Values.contains(Idx1)) {
assert(Idx2 != UINT_MAX && Values.contains(Idx2) &&
"Expected both indices to be extracted already.");
break;
// If the shuffle contains permutation of odd number of elements,
// Idx1 might be used already in the first iteration.
//
// Idx1 = shuffle Idx1, Idx2
// Idx1 = shuffle Idx1, Idx3
if (const auto It = Values.find(Idx1);
Copy link
Collaborator

@preames preames Feb 4, 2025

Choose a reason for hiding this comment

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

The It variable is only used in an assert, not the condition. Needs to either be folded into the assert, or #ifndef NDEBUG guards added to prevent non-assertions enabled build break.

Hm, looking at the check this is used for, it's entirely redundant with the contains check above. I think you can just remove this,

I > 0 && std::get<0>(Data[I - 1]) == Idx1) {
assert(Idx2 != UINT_MAX && It != Values.end() &&
!Values.contains(Idx2) &&
"Expected only first index to be extracted already.");
;
Copy link
Collaborator

@preames preames Feb 4, 2025

Choose a reason for hiding this comment

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

Stray semi?

} else {
Copy link
Collaborator

@preames preames Feb 4, 2025

Choose a reason for hiding this comment

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

Can you reverse the if-clause so this becomes an early break for readability? (And reduces nesting.)

assert(Idx2 != UINT_MAX && Values.contains(Idx2) &&
"Expected both indices to be extracted already.");
break;
}
} else {
SDValue V = ExtractValue(Idx1 >= NumOfSrcRegs ? V2 : V1,
(Idx1 % NumOfSrcRegs) * NumOpElts);
Values[Idx1] = V;
}
SDValue V = ExtractValue(Idx1 >= NumOfSrcRegs ? V2 : V1,
(Idx1 % NumOfSrcRegs) * NumOpElts);
Values[Idx1] = V;
if (Idx2 != UINT_MAX)
Values[Idx2] = ExtractValue(Idx2 >= NumOfSrcRegs ? V2 : V1,
(Idx2 % NumOfSrcRegs) * NumOpElts);
Expand Down
28 changes: 28 additions & 0 deletions llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-exact-vlen.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,3 +431,31 @@ define void @shuffle_i256_ldst(ptr %p) vscale_range(2,2) {
store <4 x i256> %res, ptr %p
ret void
}

define void @shuffle_3_input_vectors() vscale_range(4,4) {
; CHECK-LABEL: shuffle_3_input_vectors:
; CHECK: # %bb.0:
; CHECK-NEXT: vsetvli a0, zero, e64, m8, ta, ma
; CHECK-NEXT: vmv.v.i v8, 1
; CHECK-NEXT: vsetivli zero, 1, e8, mf8, ta, ma
; CHECK-NEXT: vmv.v.i v0, 6
; CHECK-NEXT: vsetvli a0, zero, e64, m8, ta, ma
; CHECK-NEXT: vmv.v.i v16, 0
; CHECK-NEXT: vsetivli zero, 4, e64, m1, ta, mu
; CHECK-NEXT: vslidedown.vi v20, v8, 1, v0.t
; CHECK-NEXT: vslideup.vi v20, v9, 3
; CHECK-NEXT: vslidedown.vi v21, v9, 1
; CHECK-NEXT: vmv1r.v v22, v8
; CHECK-NEXT: vsetvli a0, zero, e64, m8, ta, ma
; CHECK-NEXT: vmsgt.vi v8, v16, 0
; CHECK-NEXT: vsetvli zero, zero, e32, m4, ta, ma
; CHECK-NEXT: vmv.x.s a0, v8
; CHECK-NEXT: sb a0, 0(zero)
; CHECK-NEXT: ret
%1 = shufflevector <32 x i64> zeroinitializer, <32 x i64> splat (i64 1), <32 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15, i32 16, i32 34, i32 35, i32 36, i32 37, i32 38, i32 39, i32 poison, i32 poison, i32 33, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
%2 = icmp slt <32 x i64> zeroinitializer, %1
%3 = bitcast <32 x i1> %2 to i32
%4 = trunc i32 %3 to i8
store i8 %4, ptr null, align 1
ret void
}