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

Thu Oct 5 07:58:00 PDT 2023

ABataev added a comment.

In D158449#4653154 <https://reviews.llvm.org/D158449#4653154>, @reames wrote:

> 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?

1. It is supposed to be used by other clients too. Currently it mostly affects SLP.
2. Backend relies on the different functions, which already include check for size change. This change actually make the API for middle-end more strictly follow the backend requirements and allows better cost estimation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158449