[PATCH] D55780: [X86] Create PSUBUS from (add (umax X, C), -C)

Andrea Di Biagio via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 18 07:26:43 PST 2018


andreadb added a comment.

In D55780#1334383 <https://reviews.llvm.org/D55780#1334383>, @spatel wrote:

> This change handles psubus cases with no undefs, so LGTM. 
>  Depending on how the patches land, we could add the undef capability and tests for it directly here or make that an enhancement.
>  But as I mentioned, I don't think this patch alone is enough to close PR40053 because that's at least 2 independent bugs in 1 report.


Don't get me wrong: I am not suggesting that we shouldn't commit this patch.

I just wanted to point out that it is not true that all the psubus cases are fixed as you wrote. To fully fix psubus we need to make sure that we match UMAX even in the presence of undefs.
As I wrote in my previous comment, if you slightly change the example from the bugzilla, then we no longer get a UMAX in the DAG, and we lose this optimization (see below):

  unsigned long long test_sub_2(__m128i x) {
      __m128i c = _mm_set1_epi8(70);
      return _mm_subs_epu8(x, c)[0];
  }


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

https://reviews.llvm.org/D55780





More information about the llvm-commits mailing list