[PATCH] D102687: [RISCV] Ensure shuffle splat operands are type-legal

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1853
   // encounter a bitcasted BUILD_VECTOR with low/high i32 values.
-  if (SDValue SplatValue = DAG.getSplatValue(V1)) {
+  SDValue SplatValue = DAG.getSplatValue(V1);
+  if (SplatValue && (isa<ConstantSDNode>(SplatValue) ||
----------------
craig.topper wrote:
> This still creates the illegal type. The node just ends up unused with this patch and gets deleted before anyone notices?
> 
> Could we call getSplatSourceVector instead and create an XLenVT extract_vector_elt ourselves? Or teach getSplatValue to obey NewNodesMustHaveLegalTypes.
Yeah true, I wasn't too happy with this solution to be honest.

I gave a go at fixing it in `getSplatValue`. Unfortunately I found that X86 relies on illegal types being returned. What's worse is the X86 backend gets into a loop with this test case when using the type-legal `getSplatValue`. So for now I added an X86-specific version (the original) to X86ISelLowering.

```
; RUN: llc < %s -mtriple=i686-unknown -mattr=+sse2

define void @shift1a(<2 x i64> %val, <2 x i64>* %dst, <2 x i64> %sh) nounwind {
entry:
  %shamt = shufflevector <2 x i64> %sh, <2 x i64> undef, <2 x i32> <i32 0, i32 0>
  %shl = shl <2 x i64> %val, %shamt
  store <2 x i64> %shl, <2 x i64>* %dst
  ret void
}
```

Perhaps this isn't the way forward but I thought I'd throw it up in case people had comments.

The alternative for RISC-V is, as you say, calling `getSplatSourceVector` ourselves. But it does feel like `getSplatValue` should obey `NewNoesMustHaveLegalTypes`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102687