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

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 5 09:05:05 PST 2019


spatel added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71064





More information about the llvm-commits mailing list