Skip to content

add optimized implementation for AVX512 targets#347

Merged
Cyan4973 merged 3 commits into
Cyan4973:devfrom
gzm55:work/avx512
Apr 7, 2020
Merged

add optimized implementation for AVX512 targets#347
Cyan4973 merged 3 commits into
Cyan4973:devfrom
gzm55:work/avx512

Conversation

@gzm55

@gzm55 gzm55 commented Apr 6, 2020

Copy link
Copy Markdown
Contributor

on avx512 targets, variant with default secrets will improve 45%. The following result is compiled with -Ofast -mavx512f, and there is also no need to add -mno-avx256-split-unaligned-load with gcc.

compiling option:

  • avx512 gcc-9/clang-9: -Ofast -mavx512f
  • avx2 gcc-9: -Ofast -mavx2 -mno-avx256-split-unaligned-load
  • avx2 clang-9: -Ofast -mavx2

compiled with Clang 9

./xxhsum 0.7.4 (64-bit x86_64 + AVX512 little endian), Clang 9.0.0 (tags/RELEASE_900/final), by Yann Collet
Sample of 100 KB...
XXH3_64b                      :     102400 ->   565524 it/s (55227.0 MB/s)
XXH3_64b unaligned            :     102400 ->   565409 it/s (55215.7 MB/s)
XXH3_64b w/seed               :     102400 ->   561795 it/s (54862.8 MB/s)
XXH3_64b w/seed unaligned     :     102400 ->   561556 it/s (54839.5 MB/s)
XXH3_64b w/secret             :     102400 ->   332811 it/s (32501.1 MB/s)
XXH3_64b w/secret unaligned   :     102400 ->   332829 it/s (32502.8 MB/s)
XXH128                        :     102400 ->   508060 it/s (49615.3 MB/s)
XXH128 unaligned              :     102400 ->   507962 it/s (49605.6 MB/s)
XXH128 w/seed                 :     102400 ->   505046 it/s (49320.9 MB/s)
XXH128 w/seed unaligned       :     102400 ->   504939 it/s (49310.5 MB/s)
XXH128 w/secret               :     102400 ->   345985 it/s (33787.6 MB/s)
XXH128 w/secret unaligned     :     102400 ->   345963 it/s (33785.4 MB/s)
./xxhsum 0.7.4 (64-bit x86_64 + AVX2 little endian), Clang 9.0.0 (tags/RELEASE_900/final), by Yann Collet
Sample of 100 KB...
XXH3_64b                      :     102400 ->   387469 it/s (37838.8 MB/s)
XXH3_64b unaligned            :     102400 ->   387931 it/s (37883.9 MB/s)
XXH3_64b w/seed               :     102400 ->   365748 it/s (35717.6 MB/s)
XXH3_64b w/seed unaligned     :     102400 ->   365647 it/s (35707.7 MB/s)
XXH3_64b w/secret             :     102400 ->   341313 it/s (33331.4 MB/s)
XXH3_64b w/secret unaligned   :     102400 ->   341394 it/s (33339.2 MB/s)
XXH128                        :     102400 ->   361423 it/s (35295.2 MB/s)
XXH128 unaligned              :     102400 ->   361373 it/s (35290.4 MB/s)
XXH128 w/seed                 :     102400 ->   331072 it/s (32331.3 MB/s)
XXH128 w/seed unaligned       :     102400 ->   331056 it/s (32329.7 MB/s)
XXH128 w/secret               :     102400 ->   314772 it/s (30739.4 MB/s)
XXH128 w/secret unaligned     :     102400 ->   314827 it/s (30744.8 MB/s)

compiled with GCC 9

./xxhsum 0.7.4 (64-bit x86_64 + AVX512 little endian), GCC 9.2.1 20191102, by Yann Collet
Sample of 100 KB...
XXH3_64b                      :     102400 ->   512896 it/s (50087.5 MB/s)
XXH3_64b unaligned            :     102400 ->   512808 it/s (50078.9 MB/s)
XXH3_64b w/seed               :     102400 ->   521637 it/s (50941.1 MB/s)
XXH3_64b w/seed unaligned     :     102400 ->   520925 it/s (50871.6 MB/s)
XXH3_64b w/secret             :     102400 ->   341146 it/s (33315.0 MB/s)
XXH3_64b w/secret unaligned   :     102400 ->   341093 it/s (33309.8 MB/s)
XXH128                        :     102400 ->   462556 it/s (45171.5 MB/s)
XXH128 unaligned              :     102400 ->   462441 it/s (45160.3 MB/s)
XXH128 w/seed                 :     102400 ->   446254 it/s (43579.5 MB/s)
XXH128 w/seed unaligned       :     102400 ->   446616 it/s (43614.8 MB/s)
XXH128 w/secret               :     102400 ->   404667 it/s (39518.3 MB/s)
XXH128 w/secret unaligned     :     102400 ->   404753 it/s (39526.6 MB/s)
./xxhsum 0.7.4 (64-bit x86_64 + AVX2 little endian), GCC 9.2.1 20191102, by Yann Collet
Sample of 100 KB...
XXH3_64b                      :     102400 ->   365130 it/s (35657.2 MB/s)
XXH3_64b unaligned            :     102400 ->   363159 it/s (35464.7 MB/s)
XXH3_64b w/seed               :     102400 ->   364409 it/s (35586.8 MB/s)
XXH3_64b w/seed unaligned     :     102400 ->   364189 it/s (35565.3 MB/s)
XXH3_64b w/secret             :     102400 ->   330790 it/s (32303.7 MB/s)
XXH3_64b w/secret unaligned   :     102400 ->   330939 it/s (32318.3 MB/s)
XXH128                        :     102400 ->   333179 it/s (32537.0 MB/s)
XXH128 unaligned              :     102400 ->   333267 it/s (32545.6 MB/s)
XXH128 w/seed                 :     102400 ->   331603 it/s (32383.1 MB/s)
XXH128 w/seed unaligned       :     102400 ->   331731 it/s (32395.6 MB/s)
XXH128 w/secret               :     102400 ->   303619 it/s (29650.3 MB/s)
XXH128 w/secret unaligned     :     102400 ->   303526 it/s (29641.2 MB/s)

@easyaspi314

easyaspi314 commented Apr 6, 2020

Copy link
Copy Markdown
Contributor

on avx512 targets, variant with default secrets will improve 45%. The following result is compiled with -Ofast -mavx512, and there is also no need to add -mno-avx256-split-unaligned-load with gcc.

Those benchmarks are made with Clang, which likes to impersonate gcc and cl.exe.

Notably, FreeBSD, macOS, the Android NDK, and Termux all link gcc to clang by default.

Clang is not affected by this, only GCC.

@gzm55

gzm55 commented Apr 6, 2020

Copy link
Copy Markdown
Contributor Author

the previous benchmarks are compiled via CC=clang-9 make on a Ubuntu 18.04 machine, the GCC compiled version is also tested with a similar result.

For the -mno-avx256-split-unaligned-load option, the real GCC is not affected any more since that is an avx2 only option.

@gzm55

gzm55 commented Apr 6, 2020

Copy link
Copy Markdown
Contributor Author

The benchmark results compiled by gcc is updated.

It is wired that custom secret will significantly affects performance, the clang-compiled avx512 XXH3_64b w/secret even slower than avx2.

@easyaspi314

easyaspi314 commented Apr 6, 2020

Copy link
Copy Markdown
Contributor

Clang doesn't appear to even try to unroll the withSecret variants, so there is a lot of branching overhead.

I wonder if we can get it to unroll and jam the loop.

@gzm55

gzm55 commented Apr 6, 2020

Copy link
Copy Markdown
Contributor Author

gcc also has the similar performance on the withSecret ones.

@gzm55

gzm55 commented Apr 6, 2020

Copy link
Copy Markdown
Contributor Author

@easyaspi314 the main factor seems to be the runtime secretSize which blocks unrolling in the internal loop. A switch table works around for 8 align size between [136,256]:

XXH_NO_INLINE XXH64_hash_t
XXH3_hashLong_64b_withSecret(const xxh_u8* XXH_RESTRICT input, size_t len,
                             const xxh_u8* XXH_RESTRICT secret, size_t secretSize)
{
    switch (secretSize) {
    case XXH3_SECRET_SIZE_MIN + 0 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN + 0 * 8);
    case XXH3_SECRET_SIZE_MIN + 1 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN + 1 * 8);
    case XXH3_SECRET_SIZE_MIN + 2 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN + 2 * 8);
    case XXH3_SECRET_SIZE_MIN + 3 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN + 3 * 8);
    case XXH3_SECRET_SIZE_MIN + 4 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN + 4 * 8);
    case XXH3_SECRET_SIZE_MIN + 5 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN + 5 * 8);
    case XXH3_SECRET_SIZE_MIN + 6 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN + 6 * 8);
    case XXH3_SECRET_SIZE_MIN + 7 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN + 7 * 8);
    case XXH3_SECRET_SIZE_MIN + 8 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN + 8 * 8);
    case XXH3_SECRET_SIZE_MIN + 9 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN + 9 * 8);
    case XXH3_SECRET_SIZE_MIN +10 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN +10 * 8);
    case XXH3_SECRET_SIZE_MIN +11 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN +11 * 8);
    case XXH3_SECRET_SIZE_MIN +12 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN +12 * 8);
    case XXH3_SECRET_SIZE_MIN +13 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN +13 * 8);
    case XXH3_SECRET_SIZE_MIN +14 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN +14 * 8);
    case XXH3_SECRET_SIZE_MIN +15 * 8: return XXH3_hashLong_64b_internal(input, len, secret, XXH3_SECRET_SIZE_MIN +15 * 8);
    }
    return XXH3_hashLong_64b_internal(input, len, secret, secretSize);
}

This improves scalar, sse2, avx2 and avx512. The result for avx512:

./xxhsum 0.7.4 (64-bit x86_64 + AVX512 little endian), Clang 9.0.0 (tags/RELEASE_900/final), by Yann Collet
Sample of 100 KB...
XXH3_64b                      :     102400 ->   565861 it/s (55259.8 MB/s)
XXH3_64b unaligned            :     102400 ->   565829 it/s (55256.8 MB/s)
XXH3_64b w/seed               :     102400 ->   561965 it/s (54879.4 MB/s)
XXH3_64b w/seed unaligned     :     102400 ->   561832 it/s (54866.4 MB/s)
XXH3_64b w/secret             :     102400 ->   502027 it/s (49026.0 MB/s)
XXH3_64b w/secret unaligned   :     102400 ->   502387 it/s (49061.3 MB/s)
XXH128                        :     102400 ->   507926 it/s (49602.2 MB/s)
XXH128 unaligned              :     102400 ->   507856 it/s (49595.3 MB/s)
XXH128 w/seed                 :     102400 ->   505029 it/s (49319.2 MB/s)
XXH128 w/seed unaligned       :     102400 ->   504918 it/s (49308.4 MB/s)
XXH128 w/secret               :     102400 ->   452006 it/s (44141.2 MB/s)
XXH128 w/secret unaligned     :     102400 ->   451734 it/s (44114.7 MB/s)

@easyaspi314

easyaspi314 commented Apr 6, 2020

Copy link
Copy Markdown
Contributor

What about this? I don't have access to a CPU with AVX512, so I can't test.

#if XXH_VECTOR == XXH_AVX512 && !XXH_REROLL
    if (XXH_likely(nbStripes % 4 == 0)) {
        for (n = 0; n < nbStripes; n+=4 ) {
            const xxh_u8* const in = input + n*STRIPE_LEN;
            if (accWidth == XXH3_acc_64bits) XXH_PREFETCH(in + XXH_PREFETCH_DIST_AVX512_64);
            else                             XXH_PREFETCH(in + XXH_PREFETCH_DIST_AVX512_128);
            XXH3_accumulate_512(acc,
                                in,
                                secret + n*XXH_SECRET_CONSUME_RATE,
                                accWidth);
            XXH3_accumulate_512(acc,
                                in + 1 * STRIPE_LEN,
                                secret + (n+1)*XXH_SECRET_CONSUME_RATE,
                                accWidth);
            XXH3_accumulate_512(acc,
                                in + 2 * STRIPE_LEN,
                                secret + (n+2)*XXH_SECRET_CONSUME_RATE,
                                accWidth);
            XXH3_accumulate_512(acc,
                                in + 3 * STRIPE_LEN,
                                secret + (n+3)*XXH_SECRET_CONSUME_RATE,
                                accWidth);
        }
    } else
#endif
    for (n = 0; n < nbStripes; n++) {
        ...

Comment thread xxh3.h Outdated
@Cyan4973

Cyan4973 commented Apr 6, 2020

Copy link
Copy Markdown
Owner

Thanks for this great submission @gzm55 !

I understand it noticeably improves speed when using the default secret for CPU with AVX512 instruction set. +40% definitely matches the bar in order to introduce a new target-specific code path in the library.

I also understand that optimizing the _withSecret() variant is more complex.
That being said, as a way to keep this PR clean and simple, I'm happy to merge it if you wish, and accept a follow-up PR with improvements for the _withSecret variant.

Before merge, I merely have a small question on the new prefetch distance.

@Cyan4973

Cyan4973 commented Apr 6, 2020

Copy link
Copy Markdown
Owner

Clang doesn't appear to even try to unroll the withSecret variants, so there is a lot of branching overhead.

Was it tested with XXH_INLINE_ALL ?
I naively assumed that, if the secret is known at compile time, then the compiler would find a way to use the secret's length to correctly optimize the inner loop, even if it's not the default one.

@gzm55

gzm55 commented Apr 6, 2020

Copy link
Copy Markdown
Contributor Author

@Cyan4973 agree to separate the _withSecret discussion and optimization to another PR.

the distance of prefetching is got from profiling each one byte for 100K length input (xxhsum default). The benchmark machine is a ECS server, not a desktop machine. Do you have some experience or environments to settle down a better distance?

@Cyan4973

Cyan4973 commented Apr 6, 2020

Copy link
Copy Markdown
Owner

Do you have some experience or environments to settle down a better distance?

There will never be a "one size fits all" best prefetch distance for all systems.
The "better" distance is necessarily system dependent.

Some generic rules of thumb to establish a distance :

  • Currently, this is a "better than nothing" approach; we want a "generic" value which is good enough to improve performance across the board. This is different from looking for the best value for each system.
  • In the future, we might have to offer some programmability on this value. Compile time, and maybe runtime. We might have to produce a tool to automate distance determination for a given system. We don't have to do that now, this is ground for some future patch.
  • Be wary of synthetic benchmark on this topic.
    A benchmark will basically assume that it has the whole system for itself. This will lead to over-use of memory bandwidth and cache size, on the ground that they measurably improve the synthetic benchmark's results.
    But in a real application, bandwidth will be shared with other parts of the application, and over-using it with over-demanding prefetch requests will lead to system-wide performance degradation. When selecting a prefetching distance, one wants to lean towards smaller distances, and basically stop early, as soon as benefits from larger prefetching distances start to become "small". That's because it preserves a general resource (memory bandwidth) for the benefit of the larger system, be it other parts of the application or other concurrent applications.

@gzm55 gzm55 changed the title add optimized implementation for AVX512 targets WIP: add optimized implementation for AVX512 targets Apr 7, 2020
@gzm55 gzm55 changed the title WIP: add optimized implementation for AVX512 targets add optimized implementation for AVX512 targets Apr 7, 2020
@gzm55

gzm55 commented Apr 7, 2020

Copy link
Copy Markdown
Contributor Author

@Cyan4973 the prefetch distances are updated to 64 aligned for clang and gcc. When do benchmark, start offset % 64 == 16. The results:

Clang

./xxhsum 0.7.4 (64-bit x86_64 + AVX512 little endian), Clang 9.0.0 (tags/RELEASE_900/final), by Yann Collet
Sample of 100 KB...
XXH3_64b                      :     102400 ->   564617 it/s (55138.4 MB/s)
XXH3_64b unaligned            :     102400 ->   564709 it/s (55147.3 MB/s)
XXH3_64b w/seed               :     102400 ->   561776 it/s (54860.9 MB/s)
XXH3_64b w/seed unaligned     :     102400 ->   561770 it/s (54860.4 MB/s)
XXH3_64b w/secret             :     102400 ->   332788 it/s (32498.8 MB/s)
XXH3_64b w/secret unaligned   :     102400 ->   332700 it/s (32490.2 MB/s)
XXH128                        :     102400 ->   508311 it/s (49639.7 MB/s)
XXH128 unaligned              :     102400 ->   508230 it/s (49631.9 MB/s)
XXH128 w/seed                 :     102400 ->   505657 it/s (49380.5 MB/s)
XXH128 w/seed unaligned       :     102400 ->   505754 it/s (49390.1 MB/s)
XXH128 w/secret               :     102400 ->   346498 it/s (33837.7 MB/s)
XXH128 w/secret unaligned     :     102400 ->   346553 it/s (33843.1 MB/s)

Gcc

./xxhsum 0.7.4 (64-bit x86_64 + AVX512 little endian), GCC 9.2.1 20191102, by Yann Collet
Sample of 100 KB...
XXH3_64b                      :     102400 ->   528095 it/s (51571.8 MB/s)
XXH3_64b unaligned            :     102400 ->   528163 it/s (51578.4 MB/s)
XXH3_64b w/seed               :     102400 ->   528725 it/s (51633.3 MB/s)
XXH3_64b w/seed unaligned     :     102400 ->   528844 it/s (51645.0 MB/s)
XXH3_64b w/secret             :     102400 ->   341283 it/s (33328.4 MB/s)
XXH3_64b w/secret unaligned   :     102400 ->   341335 it/s (33333.5 MB/s)
XXH128                        :     102400 ->   472524 it/s (46144.9 MB/s)
XXH128 unaligned              :     102400 ->   472535 it/s (46146.0 MB/s)
XXH128 w/seed                 :     102400 ->   465276 it/s (45437.1 MB/s)
XXH128 w/seed unaligned       :     102400 ->   465038 it/s (45413.8 MB/s)
XXH128 w/secret               :     102400 ->   404590 it/s (39510.7 MB/s)
XXH128 w/secret unaligned     :     102400 ->   404518 it/s (39503.7 MB/s)

@easyaspi314

easyaspi314 commented Apr 7, 2020

Copy link
Copy Markdown
Contributor

After some testing, I don't think prefetching on aarch64 is worth it.

It ends up being slower.

It also crashes the (albeit experimental) AVR and MSP430 Clang backends.

Perhaps we might want to only enable it on x86.

Comment thread xxh3.h
@Cyan4973

Cyan4973 commented Apr 7, 2020

Copy link
Copy Markdown
Owner

After some testing, I don't think prefetching on aarch64 is worth it.
It ends up being slower.

This is a surprising outcome, though one certainly worth investigating.

I don't know how aarch64 prefetching works, but I guess one mechanism probably exists, and specificities are supposed to be abstracted behind the gcc intrinsics.
Maybe they are not...

Anyway, selecting different prefetching policy depending on target hardware is certainly within the scope of this project.

@Cyan4973 Cyan4973 merged commit 294c403 into Cyan4973:dev Apr 7, 2020
@gzm55

gzm55 commented Apr 8, 2020

Copy link
Copy Markdown
Contributor Author

Anyway, selecting different prefetching policy depending on target hardware is certainly within the scope of this project.

While searching the prefetch dist for avx512, I think we should define a matrix over (target, compiler, accWidth) for the dist values or disable prefetch.

@gzm55 gzm55 deleted the work/avx512 branch April 8, 2020 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants