[PATCH] D100865: [COST] Improve shuffle kind detection if shuffle mask is provided.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:753
+    case TTI::SK_PermuteTwoSrc:
+      if (ShuffleVectorInst::isSelectMask(Mask))
+        return TTI::SK_Select;
----------------
ABataev wrote:
> dmgreen wrote:
> > ABataev wrote:
> > > dmgreen wrote:
> > > > Some of these Mask functions do not handle all sizes of mask. Do we need to be careful about shuffles that changesLength?
> > > Here we don't have info about length changes, looks like all these functions use the size of the Mask array as vector factor.
> > I may have the wrong impression of what is going on exactly, but I think it's about the elements being out of range for the mask size.
> > (Hopefully) Try this example:
> > https://godbolt.org/z/WWrPbPxcW
> This function does not use 2 different length, it has just one, deduced out of Mask size. It is just impossible to have a `change of size` situation here, I assume. We do not work with actual shuffles, functions are static.
> Also, could you provide more details about your example? What shall I expect as a result!
> Also, could you provide more details about your example? What shall I expect as a result!

It should not crash with this patch :-)

It says for me:
```
SLP: Adding cost 0 for bundle that starts with   %add.i33 = fadd fast half %21, %22.
SLP: Current total cost = 0
SLP: Adding cost 6 for bundle that starts with   %21 = extractelement <8 x half> %20, i32 0.
SLP: Current total cost = 6
clang: /work/llvm-project/llvm/lib/IR/Instructions.cpp:2122: bool isSingleSourceMaskImpl(llvm::ArrayRef<int>, int): Assertion `I
>= 0 && I < (NumOpElts * 2) && "Out-of-bounds shuffle mask element"' failed.
...
0.      Program arguments: bin/clang -target arm-arm-none-eabi -mcpu=cortex-m55 -Ofast tmp.c -S -o - -mllvm -debug -mllvm -print-
after-all
...
 #9 0x0000558afa5bc6de llvm::ShuffleVectorInst::isSingleSourceMask(llvm::ArrayRef<int>) (bin/clang+0x728e6de)
#10 0x0000558afa5bc911 llvm::ShuffleVectorInst::isSelectMask(llvm::ArrayRef<int>) (bin/clang+0x728e911)
#11 0x0000558af90f9c40 llvm::ARMTTIImpl::getShuffleCost(llvm::TargetTransformInfo::ShuffleKind, llvm::VectorType*, llvm::ArrayRef
<int>, int, llvm::VectorType*) ARMTargetTransformInfo.cpp:0:0
#12 0x0000558af9fcac6a llvm::TargetTransformInfo::getShuffleCost(llvm::TargetTransformInfo::ShuffleKind, llvm::VectorType*, llvm:
:ArrayRef<int>, int, llvm::VectorType*) const (bin/clang+0x6c9cc6a)
#13 0x0000558afb0d4f50 llvm::slpvectorizer::BoUpSLP::getEntryCost(llvm::slpvectorizer::BoUpSLP::TreeEntry*) (bin/clang+0x7da6f50)
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100865