Missed SLP vectorization with umin/umax

[Quuxplusone]

Bugzilla Link	PR46968
Status	NEW
Importance	P enhancement
Reported by	David Bolvansky (david.bolvansky@gmail.com)
Reported on	2020-08-03 07:48:28 -0700
Last modified on	2020-12-13 07:35:59 -0800
Version	trunk
Hardware	PC Linux
CC	craig.topper@gmail.com, florian_hahn@apple.com, hideki.saito@intel.com, llvm-bugs@lists.llvm.org, llvm-dev@redking.me.uk, spatel+llvm@rotateright.com
Fixed by commit(s)
Attachments
Blocks
Blocked by	PR46983
See also	PR46915

Hot code - LightPixel - from Firefox (rasterflood-svg benchmark could be
faster):

#include<stdint.h>
#include<stddef.h>

const ptrdiff_t B8G8R8A8_COMPONENT_BYTEOFFSET_B = 0;
const ptrdiff_t B8G8R8A8_COMPONENT_BYTEOFFSET_G = 1;
const ptrdiff_t B8G8R8A8_COMPONENT_BYTEOFFSET_R = 2;
const ptrdiff_t B8G8R8A8_COMPONENT_BYTEOFFSET_A = 3;

static const int sInputIntPrecisionBits = 15;
static const int sOutputIntPrecisionBits = 15;
static const int sCacheIndexPrecisionBits = 7;

static inline unsigned umax(unsigned a, unsigned b) {
  return a > b ? a : b;
}

static inline unsigned umin(unsigned a, unsigned b) {
  return a > b ? b : a;
}

void foo(uint8_t components[4], uint32_t specularNHi, uint32_t aColor) {

  components[B8G8R8A8_COMPONENT_BYTEOFFSET_B] =
      umin((specularNHi * components[B8G8R8A8_COMPONENT_BYTEOFFSET_B]) >>
               sOutputIntPrecisionBits,
           255U);
  components[B8G8R8A8_COMPONENT_BYTEOFFSET_G] =
      umin((specularNHi *components[B8G8R8A8_COMPONENT_BYTEOFFSET_G]) >>
               sOutputIntPrecisionBits,
           255U);
  components[B8G8R8A8_COMPONENT_BYTEOFFSET_R] =
      umin((specularNHi * components[B8G8R8A8_COMPONENT_BYTEOFFSET_R]) >>
               sOutputIntPrecisionBits,
           255U);

  components[B8G8R8A8_COMPONENT_BYTEOFFSET_A] =
      umax(components[B8G8R8A8_COMPONENT_BYTEOFFSET_B],
           umax(components[B8G8R8A8_COMPONENT_BYTEOFFSET_G],
                components[B8G8R8A8_COMPONENT_BYTEOFFSET_R]));
}

-O3 -mavx2 - We got: List vectorization was possible but not beneficial with
cost 0 >= 0

foo(unsigned char*, unsigned int, unsigned int):                             #
@foo(unsigned char*, unsigned int, unsigned int)
        movzx   eax, byte ptr [rdi]
        imul    eax, esi
        shr     eax, 15
        cmp     eax, 255
        mov     r8d, 255
        cmovae  eax, r8d
        mov     byte ptr [rdi], al
        movzx   edx, byte ptr [rdi + 1]
        imul    edx, esi
        shr     edx, 15
        cmp     edx, 255
        cmovae  edx, r8d
        mov     byte ptr [rdi + 1], dl
        movzx   ecx, byte ptr [rdi + 2]
        imul    ecx, esi
        shr     ecx, 15
        cmp     ecx, 255
        cmovae  ecx, r8d
        mov     byte ptr [rdi + 2], cl
        cmp     edx, ecx
        cmova   ecx, edx
        cmp     eax, ecx
        cmova   ecx, eax
        mov     byte ptr [rdi + 3], cl
        ret

-O3 -mavx512f - Clang partially vectorizes it:
.LCPI0_0:
        .long   255                             # 0xff
foo(unsigned char*, unsigned int, unsigned int):                             #
@foo(unsigned char*, unsigned int, unsigned int)
        movzx   eax, byte ptr [rdi]
        imul    eax, esi
        shr     eax, 15
        cmp     eax, 255
        mov     ecx, 255
        cmovb   ecx, eax
        mov     byte ptr [rdi], cl
        movzx   eax, word ptr [rdi + 1]
        vmovd   xmm0, eax
        vpmovzxbd       xmm0, xmm0              # xmm0 = xmm0[0],zero,zero,zero,xmm0[1],zero,zero,zero,xmm0[2],zero,zero,zero,xmm0[3],zero,zero,zero
        vmovd   xmm1, esi
        vpbroadcastd    xmm1, xmm1
        vpmulld xmm0, xmm1, xmm0
        vpbroadcastd    xmm1, dword ptr [rip + .LCPI0_0] # xmm1 = [255,255,255,255]
        vpsrld  xmm0, xmm0, 15
        vpminud xmm0, xmm0, xmm1
        vmovd   eax, xmm0
        mov     byte ptr [rdi + 1], al
        vpextrd edx, xmm0, 1
        mov     byte ptr [rdi + 2], dl
        cmp     eax, edx
        cmova   edx, eax
        cmp     ecx, edx
        cmova   edx, ecx
        mov     byte ptr [rdi + 3], dl
        ret

Godbolt:  -Rpass-missed=vec*

[Quuxplusone]

I'll take a look at this

[Quuxplusone]

This snippet requires a uint3 vectorization of the RGB components, followed by
a umax reduction of the uint3 result for the A component.

Even before getting that to work, the more basic uint4 pattern fails to
vectorize:

  components[B8G8R8A8_COMPONENT_BYTEOFFSET_*] =
      umin((specularNHi * components[B8G8R8A8_COMPONENT_BYTEOFFSET_*]) >>
               sOutputIntPrecisionBits,
           255U);

[Quuxplusone]

Similar code:

#include<stdint.h>
#include<stddef.h>

const ptrdiff_t B8G8R8A8_COMPONENT_BYTEOFFSET_B = 0;
const ptrdiff_t B8G8R8A8_COMPONENT_BYTEOFFSET_G = 1;
const ptrdiff_t B8G8R8A8_COMPONENT_BYTEOFFSET_R = 2;
const ptrdiff_t B8G8R8A8_COMPONENT_BYTEOFFSET_A = 3;

struct Color
{
  float r, g, b, a;
};

 uint32_t ColorToBGRA(const Color& aColor)
{
  union {
    uint32_t color;
    uint8_t components[4];
  };
  components[B8G8R8A8_COMPONENT_BYTEOFFSET_R] = uint8_t(aColor.r * aColor.a * 255.0f);
  components[B8G8R8A8_COMPONENT_BYTEOFFSET_G] = uint8_t(aColor.g * aColor.a * 255.0f);
  components[B8G8R8A8_COMPONENT_BYTEOFFSET_B] = uint8_t(aColor.b * aColor.a * 255.0f);
  components[B8G8R8A8_COMPONENT_BYTEOFFSET_A] = uint8_t(aColor.a * 255.0f);
  return color;
}

Not vectorized with generic x86 target.

https://godbolt.org/z/b1nz3K

[Quuxplusone]

“the more basic uint4 pattern fails to vectorize”

Fixed by Anton in trunk.