This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Improve isInterleaveShuffle to handle interleaving the high half and low half of the same source.
ClosedPublic

Authored by craig.topper on Feb 15 2023, 3:22 PM.

Details

Summary

This is needed to support the new interleave intrinsics from D141924 for
fixed vectors.

I've reworked the core loop to operate in terms of half of a source. Making 4
possible half sources. The first element of the half is used to indicate which
source using the same numbering as the shuffle where the second source elements
are numbered after the first source.

I've added restrictions to only match the first half of two vectors or the
first and second half of a single vector. This was done to prevent regressions
on the cases we have coverage for. I saw cases where generic DAG combine split
a single interleave into 2 smaller interleaves a concat. We can revisit in the
future.

Diff Detail

Event Timeline

craig.topper created this revision.Feb 15 2023, 3:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 15 2023, 3:22 PM
craig.topper requested review of this revision.Feb 15 2023, 3:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 15 2023, 3:22 PM
luke added a comment.Feb 16 2023, 7:25 AM

I applied the patch onto https://reviews.llvm.org/D144092 and can confirm it fixes the fixed length vector test cases

reames added inline comments.Feb 17 2023, 8:17 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
3362

I don't understand this if condition. It looks like you're leaving EvenV unset for the low element of the vector. That probably can't be right (as your tests cover that case), but there's something I'm missing here.

Ah, I think I see what's going on here. You're trying to handle undef vectors (i.e. we never matched the other polarity). I think this is dead code in the current patch, and these should be probably be asserts.

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fp-interleave.ll
422

From the test name, I'd expect these to be float not half. Did you forget to update the test?

449

Same here.

479

Same here

craig.topper added inline comments.Feb 17 2023, 8:24 AM
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fp-interleave.ll
422

Yes I did forget. Probably got tripped up by needing to replace both half->float and f16->f32 for floating point. For integer I only needed i16->i32 to update the test name and the content.

Address review comments

reames accepted this revision.Feb 17 2023, 9:11 AM

LGTM

This revision is now accepted and ready to land.Feb 17 2023, 9:11 AM
This revision was landed with ongoing or failed builds.Feb 17 2023, 10:01 AM
This revision was automatically updated to reflect the committed changes.
luke added inline comments.Feb 20 2023, 4:09 AM
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fp-interleave.ll
12

@craig.topper Are these VLs double than what's necessary?

v10: v2f32 = vmaddu.vv v8: v2f16, v9: v2f16 
SEW = e16
v10 EEW = e32
VL needed = 2

v10: v2f32 = vwmaccu.vx v10: v2f32, a0, v9: v2f16 
SEW = e16
v10 EEW = e32
VL needed = 2