[PATCH] D152600: [InstCombine] Add target option to bail scalarization

Tue Jun 13 05:39:09 PDT 2023

nikic added a comment.

In D152600#4414070 <https://reviews.llvm.org/D152600#4414070>, @hgreving wrote:

> In D152600#4410907 <https://reviews.llvm.org/D152600#4410907>, @nikic wrote:
>
>> InstCombine is a target-independent canonicalization pass. The use of TTI hooks is forbidden as a matter of policy.
>>
>> If there is no subset of scalarization that is universally profitable, then the transform should be moved into VectorCombine, which is cost-model driven.
>
> I expected this kind of reply, I felt though that TTI is already used in some cases.

TTI is only used for handling of target intrinsics.

> If it's strictly target independent, I don't understand why we would scalarize in instcombine in the first place, since this is very unlikely to be target agnostic.

Probably just an old transform that used to be universally profitable at the time.

> Would you be ok with a switch (probably not)? The only problem with moving it to vector-combine is that besides scalarize phi, it seems entangled with extract_elt transforms that _are_ target independent, WDYT?

The patch description has no information on what problem you're actually trying to solve, so it's hard to give meaningful advice here. Is any kind of scalarization problematic for your target, or is it something more specific, e.g. an expensive variable extract from a binop being converted into two expensive variable extracts?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152600