[PATCH] D116362: [TTI] Support ScalableVectorType in getShuffleCost with SK_Broadcast kind

[JunMa via Phabricator via llvm-commits]

junparser added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:881
     case TTI::SK_Broadcast:
-      return getBroadcastShuffleOverhead(cast<FixedVectorType>(Tp));
+      return getBroadcastShuffleOverhead(Tp);
     case TTI::SK_Select:
----------------
paulwalker-arm wrote:
> Based on the above personally I think `getShuffleCost` is wholly unsafe for scalable vector shuffles and I'd sooner see the "you probably didn't want to get here" code here.  For example:
> ```
> case TTI::SK_Broadcast:
>   if (!isa<FixedVectorType>(Tp))
>     return InstructionCost::getInvalid();
>   return getBroadcastShuffleOverhead(cast<FixedVectorType>(Tp));
> ```
> This for me makes it clearer that if you'd rather not return an invalid cost then you'll need to fix the target specific implementation of this function.
> 
> Presumably the other shuffle types below need the same treatment?
> Based on the above personally I think `getShuffleCost` is wholly unsafe for scalable vector shuffles and I'd sooner see the "you probably didn't want to get here" code here.  For example:
> ```
> case TTI::SK_Broadcast:
>   if (!isa<FixedVectorType>(Tp))
>     return InstructionCost::getInvalid();
>   return getBroadcastShuffleOverhead(cast<FixedVectorType>(Tp));
> ```
> This for me makes it clearer that if you'd rather not return an invalid cost then you'll need to fix the target specific implementation of this function.
> 
make sense to me.

> Presumably the other shuffle types below need the same treatment?
Yes,  we also need handle intrinsic like vector.reverse with scalable vector.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116362