[PATCH] D95598: [AArch64][SVE]Add cost model for broadcast shuffle

[Caroline via Phabricator via llvm-commits]

CarolineConcatto marked 3 inline comments as done.
CarolineConcatto added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1194
                                    int Index, VectorType *SubTp) {
-  if (Kind == TTI::SK_Broadcast || Kind == TTI::SK_Transpose ||
-      Kind == TTI::SK_Select || Kind == TTI::SK_PermuteSingleSrc) {
----------------
sdesmalen wrote:
> Why did you remove this condition?
I don't see any reason why we need this if here.
IMO it is a redundancy that I think should be removed if we have more then one cost table.
But as we are merging the cost for scalable vectors with fixed vectors I am leaving this if as it was.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1236
+
+    static const CostTblEntry ScalableShuffleTbl[] = {
+        {TTI::SK_Broadcast, MVT::nxv16i8, 1},
----------------
sdesmalen wrote:
> Can't these be moved into the table above? (`ShuffleTbl`)
We can, but  I thought that having one table cost for fixed vector and  one for scalable makes the code clear.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95598