[Bug tree-optimization/82356] New: auto-vectorizing pack of 16->8 has a redundant AND after a shift

[peter at cordes dot ca]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82356

            Bug ID: 82356
           Summary: auto-vectorizing pack of 16->8 has a redundant AND
                    after a shift
           Product: gcc
           Version: 8.0
            Status: UNCONFIRMED
          Keywords: missed-optimization, ssemmx
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: peter at cordes dot ca
  Target Milestone: ---
            Target: x86_64-*-*, i?86-*-*

#include <stdint.h>
void pack_high8_baseline(uint8_t *__restrict__ dst, const uint16_t
*__restrict__ src, size_t bytes) {
  uint8_t *end_dst = dst + bytes;
  do{
     *dst++ = *src++ >> 8;
  } while(dst < end_dst);
}

https://godbolt.org/g/yoJZ3C
gcc -O3  auto-vectorizes to this inner loop:

.L5:
        movdqa  (%rdx,%rax,2), %xmm0
        movdqa  16(%rdx,%rax,2), %xmm1
        psrlw   $8, %xmm0
        pand    %xmm2, %xmm0             # Redundant with the shift
        psrlw   $8, %xmm1
        pand    %xmm2, %xmm1             # Redundant with the shift
        packuswb        %xmm1, %xmm0
        movups  %xmm0, (%rcx,%rax)
        addq    $16, %rax
        cmpq    %rsi, %rax
        jne     .L5

This is mostly good, but the PAND instructions are redundant, because psrlw by
8 already leaves the high byte of each 16-bit element zeroed.

The same extra AND is present when auto-vectorizing for AVX2 (but not AVX512,
where it uses a different strategy.)  Other than that, the AVX2 vectorization
strategy looks very good (packus ymm, then vpermq to fix the result).

If the input is 32B-aligned (or 64B aligned for AVX2), one of the load+shifts
can be *replaced* with an AND + unaligned load offset by -1.  This avoids a
bottleneck on shift throughput (at least with unrolling it does; without
unrolling we bottleneck on the front-end except on Ryzen).  It's even better
with AVX, because load+AND can fold into one instruction.

See https://stackoverflow.com/a/46477080/224132 for more details.

This C source produces the inner loop that I think should be very good across
K10, Bulldozer, Ryzen,  Nehalem, Sandybridge, HSW/SKL, Jaguar, Atom, and
Silvermont.  (With SSE2 or AVX.)  i.e. this should be great for tune=generic
after reaching a 32B boundary.

Not great on Core2 or K8 where non-cache-line-crossing movdqu costs more.

// take both args as uint8_t* so we can offset by 1 byte to replace a shift
with an AND
// if src is 32B-aligned, we never have cache-line splits
void pack_high8_alignhack(uint8_t *restrict dst, const uint8_t *restrict src,
size_t bytes) {
  uint8_t *end_dst = dst + bytes;
  do{
     __m128i v0 = _mm_loadu_si128((__m128i*)src);  // this load should be
aligned
     __m128i v1_offset = _mm_loadu_si128(1+(__m128i*)(src-1));
     v0 = _mm_srli_epi16(v0, 8);
     __m128i v1 = _mm_and_si128(v1_offset, _mm_set1_epi16(0x00FF));
     __m128i pack = _mm_packus_epi16(v0, v1);
     _mm_storeu_si128((__m128i*)dst, pack);
     dst += 16;
     src += 32;  // 32 bytes
  } while(dst < end_dst);
}