Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This is the Fletcher4 algorithm implemented in NEON for Aarch64 / ARMv8 64 bits #5248

Merged
merged 1 commit into from
Oct 21, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions include/zfs_fletcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ typedef struct zfs_fletcher_avx512 {
uint64_t v[8] __attribute__((aligned(64)));
} zfs_fletcher_avx512_t;

typedef struct zfs_fletcher_aarch64_neon {
uint64_t v[2] __attribute__((aligned(16)));
} zfs_fletcher_aarch64_neon_t;


typedef union fletcher_4_ctx {
zio_cksum_t scalar;
Expand All @@ -90,6 +94,9 @@ typedef union fletcher_4_ctx {
#if defined(__x86_64) && defined(HAVE_AVX512F)
zfs_fletcher_avx512_t avx512[4];
#endif
#if defined(__aarch64__)
zfs_fletcher_aarch64_neon_t aarch64_neon[4];
#endif
} fletcher_4_ctx_t;

/*
Expand Down Expand Up @@ -128,6 +135,10 @@ extern const fletcher_4_ops_t fletcher_4_avx2_ops;
extern const fletcher_4_ops_t fletcher_4_avx512f_ops;
#endif

#if defined(__aarch64__)
extern const fletcher_4_ops_t fletcher_4_aarch64_neon_ops;
#endif

#ifdef __cplusplus
}
#endif
Expand Down
1 change: 1 addition & 0 deletions lib/libzpool/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ KERNEL_C = \
zfs_fletcher_intel.c \
zfs_fletcher_sse.c \
zfs_fletcher_avx512.c \
zfs_fletcher_aarch64_neon.c \
zfs_namecheck.c \
zfs_prop.c \
zfs_uio.c \
Expand Down
2 changes: 1 addition & 1 deletion man/man5/zfs-module-parameters.5
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,7 @@ Default value: \fB67,108,864\fR.
Select a fletcher 4 implementation.
.sp
Supported selectors are: \fBfastest\fR, \fBscalar\fR, \fBsse2\fR, \fBssse3\fR,
\fBavx2\fR, and \fBavx512f\fR.
\fBavx2\fR, \fBavx512f\fR, and \fBaarch64_neon\fR.
All of the selectors except \fBfastest\fR and \fBscalar\fR require instruction
set extensions to be available and will only appear if ZFS detects that they are
present at runtime. If multiple implementations of fletcher 4 are available,
Expand Down
1 change: 1 addition & 0 deletions module/zcommon/Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ $(MODULE)-objs += zpool_prop.o
$(MODULE)-$(CONFIG_X86) += zfs_fletcher_intel.o
$(MODULE)-$(CONFIG_X86) += zfs_fletcher_sse.o
$(MODULE)-$(CONFIG_X86) += zfs_fletcher_avx512.o
$(MODULE)-$(CONFIG_ARM64) += zfs_fletcher_aarch64_neon.o
3 changes: 3 additions & 0 deletions module/zcommon/zfs_fletcher.c
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ static const fletcher_4_ops_t *fletcher_4_impls[] = {
#if defined(__x86_64) && defined(HAVE_AVX512F)
&fletcher_4_avx512f_ops,
#endif
#if defined(__aarch64__)
&fletcher_4_aarch64_neon_ops,
#endif
};

/* Hold all supported implementations */
Expand Down
215 changes: 215 additions & 0 deletions module/zcommon/zfs_fletcher_aarch64_neon.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
/*
* Implement fast Fletcher4 with NEON instructions. (aarch64)
*
* Use the 128-bit NEON SIMD instructions and registers to compute
* Fletcher4 in four incremental 64-bit parallel accumulator streams,
* and then combine the streams to form the final four checksum words.
* This implementation is a derivative of the AVX SIMD implementation by
* James Guilford and Jinshan Xiong from Intel (see zfs_fletcher_intel.c).
*
* Copyright (C) 2016 Romain Dolbeau.
*
* Authors:
* Romain Dolbeau <[email protected]>
*
* This software is available to you under a choice of one of two
* licenses. You may choose to be licensed under the terms of the GNU
* General Public License (GPL) Version 2, available from the file
* COPYING in the main directory of this source tree, or the
* OpenIB.org BSD license below:
*
* Redistribution and use in source and binary forms, with or
* without modification, are permitted provided that the following
* conditions are met:
*
* - Redistributions of source code must retain the above
* copyright notice, this list of conditions and the following
* disclaimer.
*
* - Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials
* provided with the distribution.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
* BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
* ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/

#if defined(__aarch64__)

#include <linux/simd_aarch64.h>
#include <sys/spa_checksum.h>
#include <zfs_fletcher.h>
#include <strings.h>

static void
fletcher_4_aarch64_neon_init(fletcher_4_ctx_t *ctx)
{
bzero(ctx->aarch64_neon, 4 * sizeof (zfs_fletcher_aarch64_neon_t));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not picking up sysmacros.h in the aarch64 build for some reason resulting in bzero() being undefined. You could just use memset() here which is what bzero() gets defined too, or adding the missing header.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added "sys/sysmacros.h"

Copy link
Contributor Author

@rdolbeau rdolbeau Oct 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't help. I've switched to <strings.h> (which _sse.c includes, and it uses bzero as well), and that seems OK with --enable-debug.

}

static void
fletcher_4_aarch64_neon_fini(fletcher_4_ctx_t *ctx, zio_cksum_t *zcp)
{
uint64_t A, B, C, D;
A = ctx->aarch64_neon[0].v[0] + ctx->aarch64_neon[0].v[1];
B = 2 * ctx->aarch64_neon[1].v[0] + 2 * ctx->aarch64_neon[1].v[1] -
ctx->aarch64_neon[0].v[1];
C = 4 * ctx->aarch64_neon[2].v[0] - ctx->aarch64_neon[1].v[0] +
4 * ctx->aarch64_neon[2].v[1] - 3 * ctx->aarch64_neon[1].v[1];
D = 8 * ctx->aarch64_neon[3].v[0] - 4 * ctx->aarch64_neon[2].v[0] +
8 * ctx->aarch64_neon[3].v[1] - 8 * ctx->aarch64_neon[2].v[1] +
ctx->aarch64_neon[1].v[1];
ZIO_SET_CHECKSUM(zcp, A, B, C, D);
}

#define NEON_INIT_LOOP() \
asm("eor %[ZERO].16b,%[ZERO].16b,%[ZERO].16b\n" \
"ld1 { %[ACC0].4s }, %[CTX0]\n" \
"ld1 { %[ACC1].4s }, %[CTX1]\n" \
"ld1 { %[ACC2].4s }, %[CTX2]\n" \
"ld1 { %[ACC3].4s }, %[CTX3]\n" \
: [ZERO] "=w" (ZERO), \
[ACC0] "=w" (ACC0), [ACC1] "=w" (ACC1), \
[ACC2] "=w" (ACC2), [ACC3] "=w" (ACC3) \
: [CTX0] "Q" (ctx->aarch64_neon[0]), \
[CTX1] "Q" (ctx->aarch64_neon[1]), \
[CTX2] "Q" (ctx->aarch64_neon[2]), \
[CTX3] "Q" (ctx->aarch64_neon[3]))

#define NEON_DO_REVERSE "rev32 %[SRC].16b, %[SRC].16b\n"

#define NEON_DONT_REVERSE ""

#define NEON_MAIN_LOOP(REVERSE) \
asm("ld1 { %[SRC].4s }, %[IP]\n" \
REVERSE \
"zip1 %[TMP1].4s, %[SRC].4s, %[ZERO].4s\n" \
"zip2 %[TMP2].4s, %[SRC].4s, %[ZERO].4s\n" \
"add %[ACC0].2d, %[ACC0].2d, %[TMP1].2d\n" \
"add %[ACC1].2d, %[ACC1].2d, %[ACC0].2d\n" \
"add %[ACC2].2d, %[ACC2].2d, %[ACC1].2d\n" \
"add %[ACC3].2d, %[ACC3].2d, %[ACC2].2d\n" \
"add %[ACC0].2d, %[ACC0].2d, %[TMP2].2d\n" \
"add %[ACC1].2d, %[ACC1].2d, %[ACC0].2d\n" \
"add %[ACC2].2d, %[ACC2].2d, %[ACC1].2d\n" \
"add %[ACC3].2d, %[ACC3].2d, %[ACC2].2d\n" \
: [SRC] "=&w" (SRC), \
[TMP1] "=&w" (TMP1), [TMP2] "=&w" (TMP2), \
[ACC0] "+w" (ACC0), [ACC1] "+w" (ACC1), \
[ACC2] "+w" (ACC2), [ACC3] "+w" (ACC3) \
: [ZERO] "w" (ZERO), [IP] "Q" (*ip))

#define NEON_FINI_LOOP() \
asm("st1 { %[ACC0].4s },%[DST0]\n" \
"st1 { %[ACC1].4s },%[DST1]\n" \
"st1 { %[ACC2].4s },%[DST2]\n" \
"st1 { %[ACC3].4s },%[DST3]\n" \
: [DST0] "=Q" (ctx->aarch64_neon[0]), \
[DST1] "=Q" (ctx->aarch64_neon[1]), \
[DST2] "=Q" (ctx->aarch64_neon[2]), \
[DST3] "=Q" (ctx->aarch64_neon[3]) \
: [ACC0] "w" (ACC0), [ACC1] "w" (ACC1), \
[ACC2] "w" (ACC2), [ACC3] "w" (ACC3))

static void
fletcher_4_aarch64_neon_native(fletcher_4_ctx_t *ctx,
const void *buf, uint64_t size)
{
const uint64_t *ip = buf;
const uint64_t *ipend = (uint64_t *)((uint8_t *)ip + size);
#if defined(_KERNEL)
register unsigned char ZERO asm("v0") __attribute__((vector_size(16)));
register unsigned char ACC0 asm("v1") __attribute__((vector_size(16)));
register unsigned char ACC1 asm("v2") __attribute__((vector_size(16)));
register unsigned char ACC2 asm("v3") __attribute__((vector_size(16)));
register unsigned char ACC3 asm("v4") __attribute__((vector_size(16)));
register unsigned char TMP1 asm("v5") __attribute__((vector_size(16)));
register unsigned char TMP2 asm("v6") __attribute__((vector_size(16)));
register unsigned char SRC asm("v7") __attribute__((vector_size(16)));
#else
unsigned char ZERO __attribute__((vector_size(16)));
unsigned char ACC0 __attribute__((vector_size(16)));
unsigned char ACC1 __attribute__((vector_size(16)));
unsigned char ACC2 __attribute__((vector_size(16)));
unsigned char ACC3 __attribute__((vector_size(16)));
unsigned char TMP1 __attribute__((vector_size(16)));
unsigned char TMP2 __attribute__((vector_size(16)));
unsigned char SRC __attribute__((vector_size(16)));
#endif

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kfpu_begin() / kfpu_end() this asm

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ; disappeared when updating to 482cd9e - I wasn't paying attention it seems :-/

kfpu_begin();

NEON_INIT_LOOP();

for (; ip < ipend; ip += 2) {
NEON_MAIN_LOOP(NEON_DONT_REVERSE);
}

NEON_FINI_LOOP();

kfpu_end();
}

static void
fletcher_4_aarch64_neon_byteswap(fletcher_4_ctx_t *ctx,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comments apply as for fletcher_4_aarch64_neon_native()

const void *buf, uint64_t size)
{
const uint64_t *ip = buf;
const uint64_t *ipend = (uint64_t *)((uint8_t *)ip + size);
#if defined(_KERNEL)
register unsigned char ZERO asm("v0") __attribute__((vector_size(16)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a simple method that does not call into stuff. Could you just put used simd regs in the clobber list instead of these declarations? Would save a ton of space...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might reduce (source) code size, but it makes the code less readable IMHO (explicit names are easier to read than register numbers, in particular for data flow between blocks). It would also make the code more dependent on compiler behavior, since data are passed between blocks. The only block-local values are TMP1 and TMP2.

Experience has taught me to write ASM blocks with belt, suspenders, and some extra glue just in case :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention was to reduce difference between kernel and user-space code. I'm fine with this as is...

register unsigned char ACC0 asm("v1") __attribute__((vector_size(16)));
register unsigned char ACC1 asm("v2") __attribute__((vector_size(16)));
register unsigned char ACC2 asm("v3") __attribute__((vector_size(16)));
register unsigned char ACC3 asm("v4") __attribute__((vector_size(16)));
register unsigned char TMP1 asm("v5") __attribute__((vector_size(16)));
register unsigned char TMP2 asm("v6") __attribute__((vector_size(16)));
register unsigned char SRC asm("v7") __attribute__((vector_size(16)));
#else
unsigned char ZERO __attribute__((vector_size(16)));
unsigned char ACC0 __attribute__((vector_size(16)));
unsigned char ACC1 __attribute__((vector_size(16)));
unsigned char ACC2 __attribute__((vector_size(16)));
unsigned char ACC3 __attribute__((vector_size(16)));
unsigned char TMP1 __attribute__((vector_size(16)));
unsigned char TMP2 __attribute__((vector_size(16)));
unsigned char SRC __attribute__((vector_size(16)));
#endif

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kfpu_begin() / kfpu_end()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot that one, sorry. And thanks everyone for all the reviews.

kfpu_begin();

NEON_INIT_LOOP();

for (; ip < ipend; ip += 2) {
NEON_MAIN_LOOP(NEON_DO_REVERSE);
}

NEON_FINI_LOOP();

kfpu_end();
}

static boolean_t fletcher_4_aarch64_neon_valid(void)
{
return (B_TRUE);
}

const fletcher_4_ops_t fletcher_4_aarch64_neon_ops = {
.init_native = fletcher_4_aarch64_neon_init,
.compute_native = fletcher_4_aarch64_neon_native,
.fini_native = fletcher_4_aarch64_neon_fini,
.init_byteswap = fletcher_4_aarch64_neon_init,
.compute_byteswap = fletcher_4_aarch64_neon_byteswap,
.fini_byteswap = fletcher_4_aarch64_neon_fini,
.valid = fletcher_4_aarch64_neon_valid,
.name = "aarch64_neon"
};

#endif /* defined(__aarch64__) */