Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

[X86] SSE2/AVX2 regression, unfortunate instruction selection for vectorized i8 and i16 comparisons due to overzealous conversion to unsigned. #46417

Closed Quuxplusone closed 4 years ago

Quuxplusone commented 4 years ago
Bugzilla Link PR47448
Status RESOLVED FIXED
Importance P enhancement
Reported by Tom Hender (ToHe_EMA@gmx.de)
Reported on 2020-09-07 06:00:47 -0700
Last modified on 2020-09-07 08:20:54 -0700
Version trunk
Hardware PC All
CC craig.topper@gmail.com, htmldeveloper@gmail.com, llvm-bugs@lists.llvm.org, llvm-dev@redking.me.uk, spatel+llvm@rotateright.com
Fixed by commit(s) rG9de0a3da6a76
Attachments
Blocks
Blocked by
See also
While writing custom code to handle vectorized loop remainder I noticed that
the current LLVM trunk sometimes turns "signed greater than" SSE2-intrinsics
into "unsigned greater than or equal" which are not directly supported by SSE2.
The problem occurs with both i8 and i16 integer vectors and with SSE2 as well
as AVX2.

Here is some C code that illustrates the problem:

#include <emmintrin.h>
__m128i BadCompare(short value)
{
    return _mm_cmpgt_epi16(
        _mm_set1_epi16(value & 7),
        _mm_setr_epi16(0, 1, 2, 3, 4, 5, 6, 7));
}

This compiles to the following assembly (https://gcc.godbolt.org/z/nx5Pn4):

.LCPI0_0:
        .short  1
        .short  2
        .short  3
        .short  4
        .short  5
        .short  6
        .short  7
        .short  8
BadCompare(short):
        and     edi, 7
        movd    xmm0, edi
        pshuflw xmm0, xmm0, 0
        pshufd  xmm0, xmm0, 0
        movdqa  xmm1, xmmword ptr [rip + .LCPI0_0]
        psubusw xmm1, xmm0                            # !!!
        pxor    xmm0, xmm0                            # !!!
        pcmpeqw xmm0, xmm1                            # !!!
        ret

The generated code is suboptimal because "pcmpgt" should be used as requested
instead of incrementing all numbers and using "psubusw" and "pcmpeqw".

It seems to me that the regression is caused by some earlier pass in LLVM now
converting "icmp sgt" into "icmp ugt" which is indeed a valid transformation
because of the "& 7". For i32 and i64 integer vectors the x86 backend appears
to undo this conversion. However this step is either broken or missing in the
i8 and i16 case.
Quuxplusone commented 4 years ago
define <2 x i64> @_Z10BadCompares(i16 signext %0) {
  %2 = and i16 %0, 7
  %3 = insertelement <8 x i16> undef, i16 %2, i32 0
  %4 = shufflevector <8 x i16> %3, <8 x i16> undef, <8 x i32> zeroinitializer
  %5 = icmp ugt <8 x i16> %4, <i16 0, i16 1, i16 2, i16 3, i16 4, i16 5, i16 6, i16 7>
  %6 = sext <8 x i1> %5 to <8 x i16>
  %7 = bitcast <8 x i16> %6 to <2 x i64>
  ret <2 x i64> %7
}