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

[Caroline via Phabricator via llvm-commits]

CarolineConcatto added a comment.

Hello @ABataev,
Thank you for the patch and for adding me as a reviewer.
I did a comment on your patch, hope you don't mind.
Carol



================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:748
+        return TargetTransformInfo::SK_Reverse;
+      if (ShuffleVectorInst::isZeroEltSplatMask(Mask))
+        return TargetTransformInfo::SK_Broadcast;
----------------

Why are you changing this to broadcast I don't see that happening in SLPVectorizer.cpp.
Before your implementation

```
Kind == TTI::SK_PermuteSingleSrc
``` 
would call:


```
return getPermuteShuffleOverhead(cast<FixedVectorType>(Tp));
```


But now it calls:


```
return getBroadcastShuffleOverhead(cast<FixedVectorType>(Tp));
```

Is that correct?

In my view, some tests would fail because now it has another cost model being called.

Oddly, there is not test failure, despite, in my view and with my knowledge, it would make some tests fail because you are changing the code path for the cost model.

Do you mind adding some tests for that as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100865