[PATCH] D40984: [InstCombine] canonicalize shifty abs(): ashr+add+xor --> cmp+neg+sel

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D40984#956809, @spatel wrote:

> 1. DAGCombiner has a generic transform for all targets to convert the **scalar** cmp+sel variant of abs into the shift variant. This is the opposite of this IR canonicalization.
> 2. DAGCombiner has a generic transform for all targets to convert the **vector** cmp+sel variant of abs into either an ABS node or the shift variant. This is again the opposite of this IR canonicalization.
> 3. DAGCombiner has a generic transform for all targets to convert the exact scalar shift variant produced by #1 into an ISD::ABS node. Note: It would be an efficiency improvement if we had #1 go directly to an ABS node when that's legal/custom.
> 4. The pattern matching above is incomplete, so it is possible to escape the intended/optimal codegen in a variety of ways.
>   1. For #2, the vector path is missing the case for setlt with a '1' constant.
>   2. For #3, we are missing a match for commuted versions of the scalar shift variant.
>   3. There is no vector equivalent at all for #3.


Re-reading the code, and I got that wrong. The transform from shift to ABS does work for vectors (it uses isConstOrConstSplat() for the shift amount). So both scalars and vectors have the same pattern matching hole - they won't convert to ABS for commuted variants of that shift pattern.


https://reviews.llvm.org/D40984