[PATCH] D55639: GlobalISel: Allow shift amount to be a different type

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D55639#1350196 <https://reviews.llvm.org/D55639#1350196>, @aemerson wrote:

> In D55639#1349093 <https://reviews.llvm.org/D55639#1349093>, @arsenm wrote:
>
> > In D55639#1348912 <https://reviews.llvm.org/D55639#1348912>, @aemerson wrote:
> >
> > > Thanks, this definitely needed to be done. Looks mostly fine, but I also can't be confident on the X86 changes. @igorb @aivchenk can you take a look at this change?
> >
> >
> > Do you have an opinion how how to handle selecting the shift amount type in the combiner?
>
>
> For AArch64 we want shift immediates to be 64 bit in order to select using the imported patterns. I can see 3 options:
>  a) we ask the target via a hook what the preferred shift immediate type is
>  b) we iteratively check from i64->i32(maybe -> i16?) using the `isInstUnsupported()` call to see which type combination is legal and pick the first
>  c) we don't worry about the type and get the legalizer to re-legalize it. I don't think however that the legalizer artifact combiner currently tries to re-legalize the combined instructions. @aditya_nandakumar do you have an opinion?
>
> For now, my slight preference would be option b) as its simpler and doesn't try to change the way the legalizer/combiner currently works.


This does assume a power of 2 legality


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

https://reviews.llvm.org/D55639