[PATCH] D71064: [InstCombine] Invert `add A, sext(B) --> sub A, zext(B)` canonicalization (to `sub A, zext B -> add A, sext B`)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D71064#1771030 <https://reviews.llvm.org/D71064#1771030>, @spatel wrote:

> In D71064#1770969 <https://reviews.llvm.org/D71064#1770969>, @lebedev.ri wrote:
>
> > In D71064#1770937 <https://reviews.llvm.org/D71064#1770937>, @spatel wrote:
> >
> > > Did you confirm that codegen is equal or better for these cases?
> > >  I think we have DAGCombiner reversals for this transform, but some targets that seem like they would benefit have not enabled the TLI hook.
> >
> >
> > I didn't yet. So, as far as i can tell, these are all the interesting cases:
> >  (i'm looking at `@t0_new_canon` vs `@t1_old_canon` since that is the only change in this patch)
> >  https://godbolt.org/z/vbb25Q - no regression for x86
> >  https://godbolt.org/z/Htg7m5 - aarch64 also looks ok?
> >  https://godbolt.org/z/xmP6JV - arm good?
> >  https://godbolt.org/z/vNrNJ8 - thumb good?
> >
> > So i'd say everything is already covered by backend undo folds?
> >  Let me know if i'm missing the point here?
>
>
> Scalar looks same all-around. Vector shows some potential diffs:
>  https://godbolt.org/z/y3E-mb
>
> If I'm seeing it correctly, we always do better on the typical case where the bool vector is produced by a compare, but we might do worse if we don't have that cmp and don't have AssertSext knowledge.


So the comment is that the undo fold needs to be adjusted first, to fire for non-cmp i1 vectors on aarch64 and powerpc64le?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71064