From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3C5A7A0350; Mon, 11 May 2020 00:49:20 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B88851D15F; Mon, 11 May 2020 00:49:19 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by dpdk.org (Postfix) with ESMTP id CE6131D15E for ; Mon, 11 May 2020 00:49:17 +0200 (CEST) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 04AMkFvK032405; Sun, 10 May 2020 15:49:14 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : references : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pfpt0818; bh=Wd8dGSc6Em20tIMYLph/eAXBxGGOCiRrl7GLB/TdPI8=; b=oUOeqqf5+SFB+MDZi7Yy2N6NgczCPlz5YT1ZDTH/Vkiy9IhhyCO7qv89tSUp70fg04k1 OhhVH6FpVjM7E0aHT0DAECmnaNWZJjQk+HqPho8/cAXTTTG+BAEWOuTGE/gpFeMDA8Fv Th+niw0HTZlRjvoMFp6aRdQm28/iv1vrez5Ec3gmpk2Ak/Cta2dDY1fL8RyqX3ijD7Lw oBiXpcyEFGLvxWirMnZTwwl7d84IhoTg+V/SfvpHzzTO13GnVLmLCGq+sDs95evRc1oK sZKqV/dVKFY2MCQmZC+rPnDpD4OZGyXr8kMSCNNjoAwUS1gvu0GRBboQvZ/JPrSkLWej QQ== Received: from sc-exch04.marvell.com ([199.233.58.184]) by mx0b-0016f401.pphosted.com with ESMTP id 30wv1n4eh7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Sun, 10 May 2020 15:49:14 -0700 Received: from SC-EXCH04.marvell.com (10.93.176.84) by SC-EXCH04.marvell.com (10.93.176.84) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Sun, 10 May 2020 15:49:12 -0700 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (104.47.55.176) by SC-EXCH04.marvell.com (10.93.176.84) with Microsoft SMTP Server (TLS) id 15.0.1497.2 via Frontend Transport; Sun, 10 May 2020 15:49:11 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=F8HrxF3WzRdLMJTqFY0Y8hFglzN+dn+rc5rx/AjdyvsDWFOhNjEggXr/do0q9dqTSSrU/dVUAszl04la24tRTMUY4hAkKn2KVHsvBhZOIcEl2fH2ZMKF6PnYJHciAjt7+nQmr6KG8buDaMEom61bitKi2gump+Xysw8jFyXzAojrx6/nd/XwRBFR/8vnB+S7e4FTueiTEMWTOef+vhn6YIvSuAN90LWWUlYnMdYr5m/deQyct2Ak4LY43NeV7TAm/E0ldcDfbPo79FOzkPgQdJeKnuBI6pU9nWDNI9FOGVMo39yQnv+6yvmdOJFZkQsmqOl5gvwsYC9VN9nANvwB2A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Wd8dGSc6Em20tIMYLph/eAXBxGGOCiRrl7GLB/TdPI8=; b=PS9tT8YqG+izPBZ3UBhStdHUWgaIoFb1bDk8dRLWaNZ69Wom5JogQx95goIMchV/cmmm4oXhFo3q3S+0qank5vsE8Gz76DViT6NaVHQddKvKD3WtmehrU4J6XRnqlq2uny9I/yGCAKxYpzPJGTBxQd6az8lzZ1On0e7VdfgUdfJVYLIV6FsHMfHIvrgj/e+D2FR7ngClzQIqMn3ldRgzzrn0+L5WW5whqREu1+8axL3yfk30JLqgeeJdlLnodjOhhcBHc8Z78+el43wG6/+Cs6HpzvX1g+KlvjaXgXIIW9lgpkWApp7VZVPqlQKBNKgyymhYeoLFtl1ysgQeWu8d3g== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=marvell.com; dmarc=pass action=none header.from=marvell.com; dkim=pass header.d=marvell.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.onmicrosoft.com; s=selector1-marvell-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Wd8dGSc6Em20tIMYLph/eAXBxGGOCiRrl7GLB/TdPI8=; b=iQ0F+nIrEclETncEoefKUAdo7FNxD9fVGKr5X+UNdLqitiuI7CHbuXcRKG1fDC5DtjECoHVK6jnXBGHZzKUilf/t1BhhNPkknWD6z9B3MuFUPP/SeC0EzgvlS/JtWEGpr+kXNXlVwXJDZNC5jh2mP44FCKt2S4mzD71PC8HGdzc= Received: from BYAPR18MB2518.namprd18.prod.outlook.com (2603:10b6:a03:13b::18) by BYAPR18MB3077.namprd18.prod.outlook.com (2603:10b6:a03:10a::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2979.34; Sun, 10 May 2020 22:49:10 +0000 Received: from BYAPR18MB2518.namprd18.prod.outlook.com ([fe80::fca4:5e00:46d9:a289]) by BYAPR18MB2518.namprd18.prod.outlook.com ([fe80::fca4:5e00:46d9:a289%3]) with mapi id 15.20.2979.033; Sun, 10 May 2020 22:49:10 +0000 From: Pavan Nikhilesh Bhagavatula To: "Wang, Yipeng1" , "Van Haaren, Harry" , Jerin Jacob Kollanukkaran , "thomas@monjalon.net" , "Gobriel, Sameh" , "Richardson, Bruce" , Ruifeng Wang CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [RFC] hash: unify crc32 API header for x86 and ARM Thread-Index: AQHWHlDKpsPX0y+mQU+xHed40hV836ibp5IAgAYwQqA= Date: Sun, 10 May 2020 22:49:09 +0000 Message-ID: References: <20200429180515.5704-1-pbhagavatula@marvell.com> In-Reply-To: Accept-Language: en-IN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=marvell.com; x-originating-ip: [223.226.86.58] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 3d92b66b-f630-4995-550a-08d7f5345a6a x-ms-traffictypediagnostic: BYAPR18MB3077: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 039975700A x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: ej07HYvF5nVoVPdAGTtsvnn1MNjRisirfS49QxmBHvAS4ETbAdSSeyZ+vMopB4t3RbvlRQmgjOqTwMkMbZX/iC7AeIAOZyD3w5KPlp2jcLTugx4uGmo7p42bpbpLilAvefHaSBp9ROH2SmG3hEaZU8d632j4OsR2PWMteMxYdEOZrQYIB722S6QrpXJ9SA17TzwCZgH5bd7PzlCwWNHbdXr1kYTpOj8fIUwBc/M1WdNgPHeslEss67gNCSLtwpwc0yf7962SATian7tZU8ClDLbHXS6a+4m+vVwVAXkTsVWhoIaZmm+4Ogr/hRR83Sx4sw91SKjoM1ieV1Po6YW2pNhl4QKtAruryuggwwV0APihZlOb7jlrqZfTNKcwXiQy9DzNnKaor9miuAN3RaI5HZLCMemzz3rBWxH6kF4FfGxAmbKTt82+l4nybQaK5eV58BWxzzmupFAHoNO2eNB0oOUJrd9pTQem0SdwZomZbTkZ5sRyAmq1JHgBf/10ncEkDdPSo7qdRLbegS713tlQJVu1Sqytb0/smHm1rG/V8JY= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR18MB2518.namprd18.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(346002)(396003)(39850400004)(366004)(376002)(136003)(33430700001)(478600001)(26005)(33656002)(316002)(9686003)(4326008)(8676002)(110136005)(55016002)(5660300002)(8936002)(71200400001)(55236004)(64756008)(66556008)(6506007)(186003)(33440700001)(66476007)(76116006)(30864003)(66446008)(2906002)(66946007)(86362001)(52536014)(7696005)(921003)(579004); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: eOAkNQ9moOSAKE7XUvUaCXN7nzBy7Zh5gjI2itdd7zGANIl2VldzTHQ07tuQ1WOOBw68hcKOnuqbDxfX72u1vk8j3GAwB3a8E6J6wwkkA1CmXVYexAKWv9tlwkp+JZrFA0n3OtVy12pt/7A28253BsOWUZlBH5013Tq1974zCqUGszIXZIKVE90fTC8AMt1HuiwUyIzzFrLqaeffF4MsBAGVq4GwSADFrmXO7VUAkD7SdwEozl+GkZzmWUcP3YtvJXy2GULGet4kbiuy1OzfQ9iEf1o+oANRuIC6oF+pJ6vdK7MzGzgyasVetqK4apjItzr/za92wifC1KAMdgNOsf0wFr/9tO7JtW86vRJuagcL56iW0uzRzWxam5fChHlTCFtLTjuwZUAG3w3LcHLZgf94XxyXfZVpR7i7m0ISWvlK23n7VMhHRv/+TU5wsaxRGMh+/vOFpUb7gGPMQo0NMqOfcvFhX94yX0zwuKDsuyo= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 3d92b66b-f630-4995-550a-08d7f5345a6a X-MS-Exchange-CrossTenant-originalarrivaltime: 10 May 2020 22:49:09.8098 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: n6W7faOuOp5a1ndg/ahdVG3XMp8RUR61TXxDkMronYZg3D+sDxQBtNMoLX1NA2rFJaksVse7SJsBiksBjDEH8YJYrxLwF8X3Rod4g5elLBo= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR18MB3077 X-OriginatorOrg: marvell.com X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.216, 18.0.676 definitions=2020-05-10_11:2020-05-08, 2020-05-10 signatures=0 Subject: Re: [dpdk-dev] [RFC] hash: unify crc32 API header for x86 and ARM X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" >> Subject: [dpdk-dev] [RFC] hash: unify crc32 API header for x86 and >ARM >> >> From: Pavan Nikhilesh >> >> Merge crc32 hash calculation public API headers for x86 and ARM, split >> implementations of x86 and ARM into their respective private >headers. >> This reduces the ifdef code clutter while keeping current ABI intact. >> >> Although we install `rte_crc_arm64.h` it is not used in any of the lib o= r >drivers >> layers. All the libs and drivers use `rte_hash_crc.h` which falls back t= o >SW >> crc32 calculation for ARM platform. >> >> Signed-off-by: Pavan Nikhilesh >> --- >> >> Currently, if application incorrectly sets CRC32_ARM64 as crc32 >algorithm >> through `rte_hash_crc_set_alg()` on x86 or vice-versa we fallback to >> algorithm set previously via `rte_hash_crc_set_alg()` instead of >setting the >> best available. >> This behaviour should probably change to setting the best available >> algorithm and is up for discussion. > >[Wang, Yipeng] Should it be an illegal flag if I set ARM64 >On x86? I am thinking we should generate a warning message into logs >if this happens. The current behavior (with and without this patch) is to fallback to softwa= re CRC. Warning log sounds good. Maybe we can fallback to best available CRC mode o= n=20 the platform too. >> >> app/test/test_hash.c | 6 + >> lib/librte_hash/Makefile | 5 - >> lib/librte_hash/crc_arm64.h | 67 +++++++++++ >> lib/librte_hash/crc_x86.h | 68 +++++++++++ >> lib/librte_hash/meson.build | 3 +- >> lib/librte_hash/rte_crc_arm64.h | 183 ------------------------------ >> lib/librte_hash/rte_hash_crc.h | 193 +++++++++++++------------------- >> 7 files changed, 219 insertions(+), 306 deletions(-) create mode >100644 >> lib/librte_hash/crc_arm64.h create mode 100644 >lib/librte_hash/crc_x86.h >> delete mode 100644 lib/librte_hash/rte_crc_arm64.h >> >> diff --git a/app/test/test_hash.c b/app/test/test_hash.c index >> afa3a1a3c..7bd457dac 100644 >> --- a/app/test/test_hash.c >> +++ b/app/test/test_hash.c >> @@ -195,7 +195,13 @@ test_crc32_hash_alg_equiv(void) >> } >> >> /* Resetting to best available algorithm */ >> +#if defined RTE_ARCH_X86 >> rte_hash_crc_set_alg(CRC32_SSE42_x64); >> +#elif defined RTE_ARCH_ARM64 >> + rte_hash_crc_set_alg(CRC32_ARM64); >> +#else >> + rte_hash_crc_set_alg(CRC32_SW); >> +#endif >> >> if (i =3D=3D CRC32_ITERATIONS) >> return 0; >> diff --git a/lib/librte_hash/Makefile b/lib/librte_hash/Makefile index >> ec9f86499..f640afc42 100644 >> --- a/lib/librte_hash/Makefile >> +++ b/lib/librte_hash/Makefile >> @@ -19,11 +19,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) +=3D >rte_fbk_hash.c >> # install this header file SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)- >include :=3D >> rte_hash.h SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include +=3D >> rte_hash_crc.h -ifeq ($(CONFIG_RTE_ARCH_ARM64),y) -ifneq >($(findstring >> RTE_MACHINE_CPUFLAG_CRC32,$(CFLAGS)),) >> -SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include +=3D rte_crc_arm64.h >-endif >> -endif SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include +=3D >rte_jhash.h >> SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include +=3D rte_thash.h >SYMLINK- >> $(CONFIG_RTE_LIBRTE_HASH)-include +=3D rte_fbk_hash.h diff --git >> a/lib/librte_hash/crc_arm64.h b/lib/librte_hash/crc_arm64.h new file >mode >> 100644 index 000000000..8e75f8297 >> --- /dev/null >> +++ b/lib/librte_hash/crc_arm64.h >> @@ -0,0 +1,67 @@ >> +/* SPDX-License-Identifier: BSD-3-Clause >> + * Copyright(c) 2015 Cavium, Inc >> + */ >> + >> +#ifndef _CRC_ARM64_H_ >> +#define _CRC_ARM64_H_ >> + >> +/** >> + * @file >> + * >> + * CRC arm64 Hash >> + */ >[Wang, Yipeng] It is hidden now so do we still need this doxygen region >above? I will remove in next version. >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> +#include >> +#include >> +#include >> +#include >[Wang, Yipeng] I don't think we need all these headers in this file. >> + Will remove in next version. >> +static inline uint32_t >> +crc32c_arm64_u8(uint8_t data, uint32_t init_val) { >> + __asm__ volatile( >> + "crc32cb %w[crc], %w[crc], %w[value]" >> + : [crc] "+r" (init_val) >> + : [value] "r" (data)); >> + return init_val; >> +} >> + >> +static inline uint32_t >> +crc32c_arm64_u16(uint16_t data, uint32_t init_val) { >> + __asm__ volatile( >> + "crc32ch %w[crc], %w[crc], %w[value]" >> + : [crc] "+r" (init_val) >> + : [value] "r" (data)); >> + return init_val; >> +} >> + >> +static inline uint32_t >> +crc32c_arm64_u32(uint32_t data, uint32_t init_val) { >> + __asm__ volatile( >> + "crc32cw %w[crc], %w[crc], %w[value]" >> + : [crc] "+r" (init_val) >> + : [value] "r" (data)); >> + return init_val; >> +} >> + >> +static inline uint32_t >> +crc32c_arm64_u64(uint64_t data, uint32_t init_val) { >> + __asm__ volatile( >> + "crc32cx %w[crc], %w[crc], %x[value]" >> + : [crc] "+r" (init_val) >> + : [value] "r" (data)); >> + return init_val; >> +} >> + >> +#ifdef __cplusplus >> +} >> +#endif >> + >> +#endif /* _CRC_ARM64_H_ */ >> diff --git a/lib/librte_hash/crc_x86.h b/lib/librte_hash/crc_x86.h new >file >> mode 100644 index 000000000..fb45f2e98 >> --- /dev/null >> +++ b/lib/librte_hash/crc_x86.h >> @@ -0,0 +1,68 @@ >> +/* SPDX-License-Identifier: BSD-3-Clause >> + * Copyright(c) 2010-2014 Intel Corporation */ >> + >> +#ifndef _CRC_X86_H_ >> +#define _CRC_X86_H_ >> + >> +#if defined(RTE_ARCH_X86) >> +static inline uint32_t >> +crc32c_sse42_u8(uint8_t data, uint32_t init_val) { >> + __asm__ volatile( >> + "crc32b %[data], %[init_val];" >> + : [init_val] "+r" (init_val) >> + : [data] "rm" (data)); >> + return init_val; >> +} >> + >> +static inline uint32_t >> +crc32c_sse42_u16(uint16_t data, uint32_t init_val) { >> + __asm__ volatile( >> + "crc32w %[data], %[init_val];" >> + : [init_val] "+r" (init_val) >> + : [data] "rm" (data)); >> + return init_val; >> +} >> + >> +static inline uint32_t >> +crc32c_sse42_u32(uint32_t data, uint32_t init_val) { >> + __asm__ volatile( >> + "crc32l %[data], %[init_val];" >> + : [init_val] "+r" (init_val) >> + : [data] "rm" (data)); >> + return init_val; >> +} >> + >> +static inline uint32_t >> +crc32c_sse42_u64_mimic(uint64_t data, uint32_t init_val) { >> + union { >> + uint32_t u32[2]; >> + uint64_t u64; >> + } d; >> + >> + d.u64 =3D data; >> + init_val =3D crc32c_sse42_u32(d.u32[0], init_val); >> + init_val =3D crc32c_sse42_u32(d.u32[1], init_val); >> + return init_val; >> +} >> +#endif >> + >> +#ifdef RTE_ARCH_X86_64 >> +static inline uint32_t >> +crc32c_sse42_u64(uint64_t data, uint32_t init_val) { >> + uint64_t val =3D init_val; >> + >> + __asm__ volatile( >> + "crc32q %[data], %[init_val];" >> + : [init_val] "+r" (val) >> + : [data] "rm" (data)); >> + return (uint32_t)val; >> +} >> +#endif >> + >> +#endif >> diff --git a/lib/librte_hash/meson.build b/lib/librte_hash/meson.build >index >> 6ab46ae9d..90a180bc8 100644 >> --- a/lib/librte_hash/meson.build >> +++ b/lib/librte_hash/meson.build >> @@ -1,8 +1,7 @@ >> # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2017 Intel >> Corporation >> >> -headers =3D files('rte_crc_arm64.h', >> - 'rte_fbk_hash.h', >> +headers =3D files('rte_fbk_hash.h', >> 'rte_hash_crc.h', >> 'rte_hash.h', >> 'rte_jhash.h', >> diff --git a/lib/librte_hash/rte_crc_arm64.h >> b/lib/librte_hash/rte_crc_arm64.h deleted file mode 100644 index >> b4628cfc0..000000000 >> --- a/lib/librte_hash/rte_crc_arm64.h >> +++ /dev/null >> @@ -1,183 +0,0 @@ >> -/* SPDX-License-Identifier: BSD-3-Clause >> - * Copyright(c) 2015 Cavium, Inc >> - */ >> - >> -#ifndef _RTE_CRC_ARM64_H_ >> -#define _RTE_CRC_ARM64_H_ >> - >> -/** >> - * @file >> - * >> - * RTE CRC arm64 Hash >> - */ >> - >> -#ifdef __cplusplus >> -extern "C" { >> -#endif >> - >> -#include >> -#include >> -#include >> -#include >> - >> -static inline uint32_t >> -crc32c_arm64_u8(uint8_t data, uint32_t init_val) -{ >> - __asm__ volatile( >> - "crc32cb %w[crc], %w[crc], %w[value]" >> - : [crc] "+r" (init_val) >> - : [value] "r" (data)); >> - return init_val; >> -} >> - >> -static inline uint32_t >> -crc32c_arm64_u16(uint16_t data, uint32_t init_val) -{ >> - __asm__ volatile( >> - "crc32ch %w[crc], %w[crc], %w[value]" >> - : [crc] "+r" (init_val) >> - : [value] "r" (data)); >> - return init_val; >> -} >> - >> -static inline uint32_t >> -crc32c_arm64_u32(uint32_t data, uint32_t init_val) -{ >> - __asm__ volatile( >> - "crc32cw %w[crc], %w[crc], %w[value]" >> - : [crc] "+r" (init_val) >> - : [value] "r" (data)); >> - return init_val; >> -} >> - >> -static inline uint32_t >> -crc32c_arm64_u64(uint64_t data, uint32_t init_val) -{ >> - __asm__ volatile( >> - "crc32cx %w[crc], %w[crc], %x[value]" >> - : [crc] "+r" (init_val) >> - : [value] "r" (data)); >> - return init_val; >> -} >> - >> -/** >> - * Allow or disallow use of arm64 SIMD instrinsics for CRC32 hash >> - * calculation. >> - * >> - * @param alg >> - * An OR of following flags: >> - * - (CRC32_SW) Don't use arm64 crc intrinsics >> - * - (CRC32_ARM64) Use ARMv8 CRC intrinsic if available >> - * >> - */ >> -static inline void >> -rte_hash_crc_set_alg(uint8_t alg) >> -{ >> - switch (alg) { >> - case CRC32_ARM64: >> - if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_CRC32)) >> - alg =3D CRC32_SW; >> - /* fall-through */ >> - case CRC32_SW: >> - crc32_alg =3D alg; >> - /* fall-through */ >> - default: >> - break; >> - } >> -} >> - >> -/* Setting the best available algorithm */ >> -RTE_INIT(rte_hash_crc_init_alg) >> -{ >> - rte_hash_crc_set_alg(CRC32_ARM64); >> -} >> - >> -/** >> - * Use single crc32 instruction to perform a hash on a 1 byte value. >> - * Fall back to software crc32 implementation in case arm64 crc >intrinsics is >> - * not supported >> - * >> - * @param data >> - * Data to perform hash on. >> - * @param init_val >> - * Value to initialise hash generator. >> - * @return >> - * 32bit calculated hash value. >> - */ >> -static inline uint32_t >> -rte_hash_crc_1byte(uint8_t data, uint32_t init_val) -{ >> - if (likely(crc32_alg & CRC32_ARM64)) >> - return crc32c_arm64_u8(data, init_val); >> - >> - return crc32c_1byte(data, init_val); >> -} >> - >> -/** >> - * Use single crc32 instruction to perform a hash on a 2 bytes value. >> - * Fall back to software crc32 implementation in case arm64 crc >intrinsics is >> - * not supported >> - * >> - * @param data >> - * Data to perform hash on. >> - * @param init_val >> - * Value to initialise hash generator. >> - * @return >> - * 32bit calculated hash value. >> - */ >> -static inline uint32_t >> -rte_hash_crc_2byte(uint16_t data, uint32_t init_val) -{ >> - if (likely(crc32_alg & CRC32_ARM64)) >> - return crc32c_arm64_u16(data, init_val); >> - >> - return crc32c_2bytes(data, init_val); >> -} >> - >> -/** >> - * Use single crc32 instruction to perform a hash on a 4 byte value. >> - * Fall back to software crc32 implementation in case arm64 crc >intrinsics is >> - * not supported >> - * >> - * @param data >> - * Data to perform hash on. >> - * @param init_val >> - * Value to initialise hash generator. >> - * @return >> - * 32bit calculated hash value. >> - */ >> -static inline uint32_t >> -rte_hash_crc_4byte(uint32_t data, uint32_t init_val) -{ >> - if (likely(crc32_alg & CRC32_ARM64)) >> - return crc32c_arm64_u32(data, init_val); >> - >> - return crc32c_1word(data, init_val); >> -} >> - >> -/** >> - * Use single crc32 instruction to perform a hash on a 8 byte value. >> - * Fall back to software crc32 implementation in case arm64 crc >intrinsics is >> - * not supported >> - * >> - * @param data >> - * Data to perform hash on. >> - * @param init_val >> - * Value to initialise hash generator. >> - * @return >> - * 32bit calculated hash value. >> - */ >> -static inline uint32_t >> -rte_hash_crc_8byte(uint64_t data, uint32_t init_val) -{ >> - if (likely(crc32_alg =3D=3D CRC32_ARM64)) >> - return crc32c_arm64_u64(data, init_val); >> - >> - return crc32c_2words(data, init_val); >> -} >> - >> -#ifdef __cplusplus >> -} >> -#endif >> - >> -#endif /* _RTE_CRC_ARM64_H_ */ >> diff --git a/lib/librte_hash/rte_hash_crc.h >b/lib/librte_hash/rte_hash_crc.h >> index cf28031b3..292697db1 100644 >> --- a/lib/librte_hash/rte_hash_crc.h >> +++ b/lib/librte_hash/rte_hash_crc.h >> @@ -21,6 +21,15 @@ extern "C" { >> #include >> #include >> >> +typedef uint32_t >> +(*crc32_handler_1b)(uint8_t data, uint32_t init_val); typedef >uint32_t >> +(*crc32_handler_2b)(uint16_t data, uint32_t init_val); typedef >uint32_t >> +(*crc32_handler_4b)(uint32_t data, uint32_t init_val); typedef >uint32_t >> +(*crc32_handler_8b)(uint64_t data, uint32_t init_val); > >[Wang, Yipeng] I guess functions pointers makes the code cleaner. >I am just not sure on performance implications for indirect call. >@Harry for his comment. > Sadly there is no perf testcase for crc hash computation. Maybe we could have a simple testcase in hash_perf_autotest in next version. >> + >> /* Lookup tables for software implementation of CRC32C */ static >const >> uint32_t crc32c_tables[8][256] =3D {{ >> 0x00000000, 0xF26B8303, 0xE13B70F7, 0x1350F3F4, 0xC79A971F, >0x35F1141C, >> 0x26A1E7E8, 0xD4CA64EB, @@ -322,7 +331,7 @@ >crc32c_2bytes(uint16_t >> data, uint32_t init_val) } >> >> static inline uint32_t >> -crc32c_1word(uint32_t data, uint32_t init_val) >> +crc32c_4bytes(uint32_t data, uint32_t init_val) >> { >> uint32_t crc, term1, term2; >> crc =3D init_val; >> @@ -336,7 +345,7 @@ crc32c_1word(uint32_t data, uint32_t init_val) >} >> >> static inline uint32_t >> -crc32c_2words(uint64_t data, uint32_t init_val) >> +crc32c_8bytes(uint64_t data, uint32_t init_val) >> { >> uint32_t crc, term1, term2; >> union { >> @@ -357,109 +366,94 @@ crc32c_2words(uint64_t data, uint32_t >init_val) >> >> return crc; >> } >> - >> -#if defined(RTE_ARCH_X86) >> -static inline uint32_t >> -crc32c_sse42_u8(uint8_t data, uint32_t init_val) -{ >> - __asm__ volatile( >> - "crc32b %[data], %[init_val];" >> - : [init_val] "+r" (init_val) >> - : [data] "rm" (data)); >> - return init_val; >> -} >> - >> -static inline uint32_t >> -crc32c_sse42_u16(uint16_t data, uint32_t init_val) -{ >> - __asm__ volatile( >> - "crc32w %[data], %[init_val];" >> - : [init_val] "+r" (init_val) >> - : [data] "rm" (data)); >> - return init_val; >> -} >> - >> -static inline uint32_t >> -crc32c_sse42_u32(uint32_t data, uint32_t init_val) -{ >> - __asm__ volatile( >> - "crc32l %[data], %[init_val];" >> - : [init_val] "+r" (init_val) >> - : [data] "rm" (data)); >> - return init_val; >> -} >> - >> -static inline uint32_t >> -crc32c_sse42_u64_mimic(uint64_t data, uint64_t init_val) -{ >> - union { >> - uint32_t u32[2]; >> - uint64_t u64; >> - } d; >> - >> - d.u64 =3D data; >> - init_val =3D crc32c_sse42_u32(d.u32[0], (uint32_t)init_val); >> - init_val =3D crc32c_sse42_u32(d.u32[1], (uint32_t)init_val); >> - return (uint32_t)init_val; >> -} >> -#endif >> - >> -#ifdef RTE_ARCH_X86_64 >> -static inline uint32_t >> -crc32c_sse42_u64(uint64_t data, uint64_t init_val) -{ >> - __asm__ volatile( >> - "crc32q %[data], %[init_val];" >> - : [init_val] "+r" (init_val) >> - : [data] "rm" (data)); >> - return (uint32_t)init_val; >> -} >> -#endif >> - >[Wang, Yipeng] keep the blank line before define. Will add in next version. >> #define CRC32_SW (1U << 0) >> #define CRC32_SSE42 (1U << 1) >> #define CRC32_x64 (1U << 2) >> #define CRC32_SSE42_x64 (CRC32_x64|CRC32_SSE42) >> #define CRC32_ARM64 (1U << 3) >> >> -static uint8_t crc32_alg =3D CRC32_SW; >> +static crc32_handler_1b crc32_1b =3D crc32c_1byte; static >> +crc32_handler_2b crc32_2b =3D crc32c_2bytes; static crc32_handler_4b >> +crc32_4b =3D crc32c_4bytes; static crc32_handler_8b crc32_8b =3D >> +crc32c_8bytes; >> >> #if defined(RTE_ARCH_ARM64) && >> defined(RTE_MACHINE_CPUFLAG_CRC32) >> -#include "rte_crc_arm64.h" >> -#else >> +#include "crc_arm64.h" >> +#endif >> + >> +#if defined(RTE_ARCH_X86) >> +#include "crc_x86.h" >> +#endif >> >> /** >> - * Allow or disallow use of SSE4.2 instrinsics for CRC32 hash >> + * Allow or disallow use of SSE4.2/ARMv8 instrinsics for CRC32 hash >> * calculation. >> * >> * @param alg >> * An OR of following flags: >> - * - (CRC32_SW) Don't use SSE4.2 intrinsics >> + * - (CRC32_SW) Don't use SSE4.2 intrinsics (default non- >[x86/ARMv8]) >> * - (CRC32_SSE42) Use SSE4.2 intrinsics if available >> - * - (CRC32_SSE42_x64) Use 64-bit SSE4.2 intrinsic if available >(default) >> - * >> + * - (CRC32_SSE42_x64) Use 64-bit SSE4.2 intrinsic if available >(default x86) >> + * - (CRC32_ARM64) Use ARMv8 CRC intrinsic if available >> */ >> static inline void >> rte_hash_crc_set_alg(uint8_t alg) >> { >> -#if defined(RTE_ARCH_X86) >> - if (alg =3D=3D CRC32_SSE42_x64 && >> - > !rte_cpu_get_flag_enabled(RTE_CPUFLAG_EM64T)) >> - alg =3D CRC32_SSE42; >> + switch (alg) { >> + case CRC32_SSE42_x64: >> +#if defined RTE_ARCH_X86 >> + crc32_1b =3D crc32c_sse42_u8; >> + crc32_2b =3D crc32c_sse42_u16; >> + crc32_4b =3D crc32c_sse42_u32; >> + >> + if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_EM64T)) >> + crc32_8b =3D crc32c_sse42_u64_mimic; >> + else >> + crc32_8b =3D crc32c_sse42_u64; >> #endif >> - crc32_alg =3D alg; >> + break; >> + case CRC32_SSE42: >> +#if defined RTE_ARCH_X86 >> + crc32_1b =3D crc32c_sse42_u8; >> + crc32_2b =3D crc32c_sse42_u16; >> + crc32_4b =3D crc32c_sse42_u32; >> + crc32_8b =3D crc32c_sse42_u64_mimic; >> +#endif >> + break; >> + case CRC32_ARM64: >> +#if defined RTE_ARCH_ARM64 >> + if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_CRC32)) { >> + crc32_1b =3D crc32c_arm64_u8; >> + crc32_2b =3D crc32c_arm64_u16; >> + crc32_4b =3D crc32c_arm64_u32; >> + crc32_8b =3D crc32c_arm64_u64; >> + } >> +#endif >> + break; >> + case CRC32_SW: >> + default: >> + crc32_1b =3D crc32c_1byte; >> + crc32_2b =3D crc32c_2bytes; >> + crc32_4b =3D crc32c_4bytes; >> + crc32_8b =3D crc32c_8bytes; >> + break; >> + } >> } >> >> /* Setting the best available algorithm */ >> RTE_INIT(rte_hash_crc_init_alg) >> { >> +#if defined RTE_ARCH_X86 >> rte_hash_crc_set_alg(CRC32_SSE42_x64); >> +#elif defined RTE_ARCH_ARM64 >> + rte_hash_crc_set_alg(CRC32_ARM64); >> +#else >> + rte_hash_crc_set_alg(CRC32_SW); >> +#endif >> } >> >> /** >> - * Use single crc32 instruction to perform a hash on a byte value. >> - * Fall back to software crc32 implementation in case SSE4.2 is >> - * not supported >> + * Calculate crc32 hash value of 1bytes. >> * >> * @param data >> * Data to perform hash on. >> @@ -471,18 +465,11 @@ RTE_INIT(rte_hash_crc_init_alg) static inline >> uint32_t rte_hash_crc_1byte(uint8_t data, uint32_t init_val) { -#if >defined >> RTE_ARCH_X86 >> - if (likely(crc32_alg & CRC32_SSE42)) >> - return crc32c_sse42_u8(data, init_val); >> -#endif >> - >> - return crc32c_1byte(data, init_val); >> + return (crc32_1b)(data, init_val); >> } >> >> /** >> - * Use single crc32 instruction to perform a hash on a 2 bytes value. >> - * Fall back to software crc32 implementation in case SSE4.2 is >> - * not supported >> + * Calculate crc32 hash value of 2bytes. >> * >> * @param data >> * Data to perform hash on. >> @@ -494,18 +481,11 @@ rte_hash_crc_1byte(uint8_t data, uint32_t >init_val) >> static inline uint32_t rte_hash_crc_2byte(uint16_t data, uint32_t >init_val) { - >> #if defined RTE_ARCH_X86 >> - if (likely(crc32_alg & CRC32_SSE42)) >> - return crc32c_sse42_u16(data, init_val); >> -#endif >> - >> - return crc32c_2bytes(data, init_val); >> + return (crc32_2b)(data, init_val); >> } >> >> /** >> - * Use single crc32 instruction to perform a hash on a 4 byte value. >> - * Fall back to software crc32 implementation in case SSE4.2 is >> - * not supported >> + * Calculate crc32 hash value of 4bytes. >> * >> * @param data >> * Data to perform hash on. >> @@ -517,18 +497,11 @@ rte_hash_crc_2byte(uint16_t data, uint32_t >init_val) >> static inline uint32_t rte_hash_crc_4byte(uint32_t data, uint32_t >init_val) { - >> #if defined RTE_ARCH_X86 >> - if (likely(crc32_alg & CRC32_SSE42)) >> - return crc32c_sse42_u32(data, init_val); >> -#endif >> - >> - return crc32c_1word(data, init_val); >> + return (crc32_4b)(data, init_val); >> } >> >> /** >> - * Use single crc32 instruction to perform a hash on a 8 byte value. >> - * Fall back to software crc32 implementation in case SSE4.2 is >> - * not supported >> + * Calculate crc32 hash value of 8bytes. >> * >> * @param data >> * Data to perform hash on. >> @@ -540,21 +513,9 @@ rte_hash_crc_4byte(uint32_t data, uint32_t >init_val) >> static inline uint32_t rte_hash_crc_8byte(uint64_t data, uint32_t >init_val) { - >> #ifdef RTE_ARCH_X86_64 >> - if (likely(crc32_alg =3D=3D CRC32_SSE42_x64)) >> - return crc32c_sse42_u64(data, init_val); >> -#endif >> - >> -#if defined RTE_ARCH_X86 >> - if (likely(crc32_alg & CRC32_SSE42)) >> - return crc32c_sse42_u64_mimic(data, init_val); >> -#endif >> - >> - return crc32c_2words(data, init_val); >> + return (crc32_8b)(data, init_val); >> } >> >> -#endif >> - >> /** >> * Calculate CRC32 hash on user-supplied byte array. >> * >> -- >> 2.17.1 >[Wang, Yipeng] >Thanks for the patch, please see my comment inlined. > Thanks for the review. >I think it is good to hide the architecture specific headers. >And I agree with Harry that we shouldn't silently remove a public >header. >I don't know the best practice on handling this though (e.g. symlink or >dummyfile), either way looks fine to me. >Bruce/Thomas may comment. > >