[PATCH] D74322: GlobalISel: Extend narrowing to G_ASHR

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

In D74322#1871279 <https://reviews.llvm.org/D74322#1871279>, @arsenm wrote:

> In D74322#1867403 <https://reviews.llvm.org/D74322#1867403>, @foad wrote:
>
> > Is it really worth splitting ashr into three separate cases? Can't CSE and constant folding do it all for you?
>
>
> I feel weird relying on such things, especially when I can see the cases. If I just delete the special case, it does not manage to take care of it


I guess it's up to you, but given that we already have a constant folding MIR builder it seems silly to write (and test and review) extra code to handle cases that it could handle for you. I was a bit surprised that we're not already using it. I had to tweak Combiner::combineMachineInstrs to create a ConstantFoldingMIRBuilder instead of plain MachineIRBuilder, but then your new tests still pass even if I delete the "NarrowShiftAmt == 0" special cases for all of shl, lshr and ashr.


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

https://reviews.llvm.org/D74322