[PATCH] D158449: [IR]Add NumSrcElts param to is..Mask static function in ShuffleVectorInst.

Thu Oct 5 07:52:29 PDT 2023

reames added a comment.

In D158449#4653006 <https://reviews.llvm.org/D158449#4653006>, @ABataev wrote:

> In D158449#4653004 <https://reviews.llvm.org/D158449#4653004>, @reames wrote:
>
>> Coming into this quite late, but I'm confused by the need for this API change.
>>
>> Doesn't the existing ArrayRef::take_front API fulfill the same need?  If you need to consider a prefix of the mask, why not just take a reference to that prefix?
>
> Sure, it may work this way, but better to have this in the interface functions. It helped to find so many bugs in the cost model. Plus, this extract thing must be copied over almost all target-specific TTI implementations.

On the first point, I disagree.  Having a simpler API and shifting the mask manipulation to the client (SLP) seems strictly better.  On the second, I don't see (in your patch) need for truncation *within* backend modeling, I only see it from the client (SLP).  Am I missing something?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158449