[PATCH] D85965: [GlobalISel] Add a combine for ashr(shl x, c), c --> sext_inreg x, c'

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a comment.

In D85965#2218363 <https://reviews.llvm.org/D85965#2218363>, @arsenm wrote:

> In D85965#2218352 <https://reviews.llvm.org/D85965#2218352>, @aemerson wrote:
>
>> In D85965#2218104 <https://reviews.llvm.org/D85965#2218104>, @arsenm wrote:
>>
>>> Seems like it's missing a legality check, although I'm unclear on what the overall strategy for those is supposed to be
>>
>> This is intended to run prelegalizer so I don't think we need it.
>
> Combines should probably try to support both. There's fragments of half implemented support for this

Every combine should check legality for each instruction it wants to generate? That seems like way overkill to me. The majority of the combines we do will be pre-legalize with post being used to clean up legalizer related things or do some lowering for isel. I think it should be done on as needed basis by whichever target needs a specific combine to be done post-legalize. Unless we have a better mechanism to do this that doesn't involve manually calling into LI every time,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85965