[PATCH] D148647: [RISCV] Fix interleave crash on unary interleaves

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 19 23:59:59 PDT 2023


craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:3294
 
+  // If both elements of the even and odd vector are smaller than the size of
+  // the VT, then it's an unary interleave like:  
----------------
luke wrote:
> craig.topper wrote:
> > I wonder if the fix should be
> > 
> > if (EvenSrc % (NumElts / 2) != 0 || OddSrc % (NumElts /2) != 0)
> That works for the first two cases mentioned but then it seems to miss the interleave in this case:
> 
> ```
> ; This interleaves the first 2 elements of a vector in opposite order. With
> ; undefs for the remaining elements. We use to miscompile this.
> define <4 x i8> @unary_interleave_10uu_v4i8(<4 x i8> %x) {
>   %a = shufflevector <4 x i8> %x, <4 x i8> poison, <4 x i32> <i32 1, i32 0, i32 undef, i32 undef>
>   ret <4 x i8> %a
> }
> ```
> 
> I noticed we also probably want to support non-multiple-of-numelts offsets like this:
> ```
> shufflevector <4 x i32> %x, <4 x i32> %y, <4 x i32> <i32 0, i32 5, i32 1, i32 6>
> ```
> 
> I've updated the patch to check the range given we know the start indices and size of each vector being interleaved, but there's probably a more elegant way of expressing this that I'm missing
Does that case in `unary_interleave_10uu_v4i8` violate this rule for the extract_subvector?

```
  /// EXTRACT_SUBVECTOR(VECTOR, IDX) - Returns a subvector from VECTOR.          
  /// Let the result type be T, then IDX represents the starting element number  
  /// from which a subvector of type T is extracted. IDX must be a constant      
  /// multiple of T's known minimum vector length.
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148647/new/

https://reviews.llvm.org/D148647



More information about the llvm-commits mailing list