[PATCH] D101297: [SLP]Allow masked gathers only if allowed by target.

[Mikael Holmén via Phabricator via llvm-commits]

uabelho added a comment.

In D101297#2735950 <https://reviews.llvm.org/D101297#2735950>, @ABataev wrote:

> In D101297#2735948 <https://reviews.llvm.org/D101297#2735948>, @uabelho wrote:
>
>> In D101297#2735939 <https://reviews.llvm.org/D101297#2735939>, @ABataev wrote:
>>
>>> In D101297#2735772 <https://reviews.llvm.org/D101297#2735772>, @uabelho wrote:
>>>
>>>> In D101297#2735721 <https://reviews.llvm.org/D101297#2735721>, @uabelho wrote:
>>>>
>>>>> I'm seeing a strange case with this patch where I now get calls to
>>>>>
>>>>>   llvm.masked.gather.v2i16.v2p0i16
>>>>>
>>>>> after SLP vectorizer and before we didn't?
>>>>> Masked gather is not legal on my target. Something seems to be backwards?
>>>>
>>>> In my case the cost as computed by TTI->getGatherScatterOpCost (so the cost prior to this patch) is 6 but calculated in the new way (since masked gather is not legal) I get the cost 0. getGatherCost(VL) returns 2 and then getEntryCost(It->get()) 2 as well so VecLdCost now ends up as 0.
>>>
>>> If the masked gather is not legal, it should be converted to a regular gather by `scalarizeMaskedGather` function (called by `ScalarizeMaskedMemIntrinLegacyPass::runOnFunction`), that's why we calculate it as a regular gather
>>
>> So you say it is expected that with this patch, for a target where masked gather is not legal, the SLP Vectorizer should now suddenly introduce @llvm.masked.gather where it didn't before?
>
> Well, generally speaking, yes. Just if it is not legal, it should be scalarized and turned into a regular gather later. And the patch calculates the cost of the gather, if it is not legal

Ok. A little surprising to me given the patch headline:

  [SLP]Allow masked gathers only if allowed by target.

because the opposite is what is actually happening.

I saw a regression for my out-of-tree target with this patch, that's why it caught my eye.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101297