Bug 80568

Created attachment 41285 [details]
bswap16.cc

gcc7 (or at least the gcc8 snapshot on https://godbolt.org/g/ZafCE0) is now splitting unaligned loads/stores even for AVX2 integer, where gcc6.3 didn't.

I think this is undesirable by default, because some projects probably build with -mavx2 but fail to use -mtune=haswell (or broadwell or skylake).  For now,
Intel CPUs that do well with 32B unaligned loads are probably the most common AVX2-supporting CPUs.

IDK what's optimal for Excavator or Zen.  Was this an intentional change to make those tune options work better for those CPUs?

I would suggest that -mavx2 should imply -mno-avx256-split-unaligned-load (and -store) for -mtune=generic.  Or if that's too ugly (having insn set selection affect tuning), then maybe just revert to the previous behaviour of having integer loads/store not be split the way FP loads/stores are.

 The conventional wisdom is that unaligned loads are just as fast as aligned when the data does happen to be aligned at run-time.  Splitting this way badly breaks that assumption.  It's inconvenient/impossible to portably communicate to the compiler that it should optimize for the case where the data is aligned, even if that's not guaranteed, so loadu / storeu are probably used in lots of code that normally runs on aligned data.

Also, gcc doesn't always figure out that a hand-written scalar prologue does leave the pointer aligned for a vector loop.  (And since programmers expect it not to matter, they may still use `_mm256_loadu_si256`).  I reduced some real existing code that a colleague wrote into a test-case for this: https://godbolt.org/g/ZafCE0, also attached.    If using potentially-overlapping first/last vectors instead of scalar loops, it might use loadu just to avoid duplicating a helper function.


----

For an example of affected code, consider an endian-swap function that uses this (inline) function in its inner loop.  The code inside the loop matches what we get for compiling it stand-alone, so I'll just show that:

#include <immintrin.h>
// static inline
void swap256(char* addr, __m256i mask) {
	__m256i vec = _mm256_loadu_si256((__m256i*)addr);
	vec = _mm256_shuffle_epi8(vec, mask);
	_mm256_storeu_si256((__m256i*)addr, vec);
}


gcc6.3 -O3 -mavx2:
        vmovdqu (%rdi), %ymm1
        vpshufb %ymm0, %ymm1, %ymm0
        vmovdqu %ymm0, (%rdi)

g++ (GCC-Explorer-Build) 8.0.0 20170429 (experimental)  -O3 -mavx2
        vmovdqu (%rdi), %xmm1
        vinserti128     $0x1, 16(%rdi), %ymm1, %ymm1
        vpshufb %ymm0, %ymm1, %ymm0
        vmovups %xmm0, (%rdi)
        vextracti128    $0x1, %ymm0, 16(%rdi)