[PATCH] D61400: [SelectionDAG] Utilize ARM shift behavior

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In D61400#1486885 <https://reviews.llvm.org/D61400#1486885>, @RKSimon wrote:

> Wouldn't it be better/safer to create ARMISD shift opcodes to handle this behaviour?


I think so too. Even if this could be something that other targets could benefit from as well. And we need something safer since as @nikic mentioned this transform seem to be wrong (as it leaves the undefined shift unguarded, and later transforms might find that the shift count is out-of-bounds and remove the shift).

About the idea in general:
Our downstream target also accepts shift counts that are "out-of-bounds", so this looks like something we could benefit from. Although our target also does not have separate instructions for left/right shift. So basically a negative shift count in an SHL will become a right shift if we just lower the SHL into a target shift instruction. So the solution here would not work right out-of-the-box for our target (we would need some special "shift behavior"). So it might be difficult to make this generic enough to safely be reused by other targets (the different shift behaviors must be specified in detail).

About introducing target specific opcodes: 
One tricky part is to decide when to introduce such target specific shifts. As soon as something is turned into a target specific ISD node you are "on you own" to handle further combines etc for that ISD node (including value tracking support etc). So usually you want to do it quite late. But then there is a risk that it is harder to detect the pattern (if other combines/legalization etc already has lowered/folded the SETCC etc into something else).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61400