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 94842A04BB; Tue, 6 Oct 2020 11:58:41 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D2B80F64; Tue, 6 Oct 2020 11:58:39 +0200 (CEST) Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by dpdk.org (Postfix) with ESMTP id 4CFADF3E for ; Tue, 6 Oct 2020 11:58:39 +0200 (CEST) Received: by mail-wm1-f68.google.com with SMTP id k18so2392514wmj.5 for ; Tue, 06 Oct 2020 02:58:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=5u2VKcfBw/5pPGIO139QpPL3UDEhC6NnioS9O4ZukAI=; b=L7ns6kW+J7A9ijqVPwy9p9STZZFskfP5faFSpebw4QwiDUpHvn7v/3mF2mC3gtA/Ll nwjTXrhY+jJ981z/B2vXbj4ExzI4W1X0D1MUOlW0m5QCXXhJVa1P5E0K7ObadxWU6rcg OqoaN1Pi85cpoUec+Ka+/vjr6CLWYXZFyw9kR1bMQOW+LzxAywP4n6g6xMtsOWHcv7Xo CNroB2wLKhvM9FA4O0OMC4cuSeNJiFtf9SAQg606Xa1C38QU4qA08aEAVTrAnAJlDC/I GqbtducCwdN8lk/SMVdpxqOyqS3OKJw6CBB7GmUtUodxGzaK4GaZPSCui0/yjv61Fdw4 oDBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=5u2VKcfBw/5pPGIO139QpPL3UDEhC6NnioS9O4ZukAI=; b=tb9vk2jAdIEcKYPRMgHowMWDX+VKhP0JHwQvbO8WwfniVg1hYocy5TzYmZAE/RWiZO CAHKDbxXkVbmCICIVNsW0EjG1D68VW89jmAqvhvXaEfZs07qBWC5AWDwAsKfhjCJgwm8 Viwi9/HQqiw2ZLZ/WypHAp98VGFTiAP5yqzkslv2tqr+YAuwymPzyUAkMR7qKFE6ncu9 6aJ1GyzmpRwYASbOb08jvn9I5v1nzEPXwtoWTeMOcOnXEWDOxY9AZ1izAguF5ajqekWG 9aO93KHkx3T9Qx6eltRIc/a95yjeJFPcvWFMuecgPqxVVvWHJqADr0rPDKyOLwOsmK3Q 3mrw== X-Gm-Message-State: AOAM531t6Hs2vnV+Ml33HInrIhH+J8AD+RC7h1n/rH8fadXeTvLRw92I lBsQWeJ2uQPziR0hvD+PgSuBLA== X-Google-Smtp-Source: ABdhPJy8F/1689+abCbG+vMOEnDK911pIXPSveB3Yv+k1hgT8GL41x/dRohcbfMR7Jzs1N7akktaDw== X-Received: by 2002:a1c:8104:: with SMTP id c4mr4059938wmd.112.1601978317925; Tue, 06 Oct 2020 02:58:37 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0]) by smtp.gmail.com with ESMTPSA id n6sm2549926wrx.58.2020.10.06.02.58.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 06 Oct 2020 02:58:37 -0700 (PDT) Date: Tue, 6 Oct 2020 11:58:36 +0200 From: Olivier Matz To: Ciara Power Cc: dev@dpdk.org, Jasvinder Singh Message-ID: <20201006095836.GH21395@platinum> References: <20200807155859.63888-1-ciara.power@intel.com> <20200930130415.11211-1-ciara.power@intel.com> <20200930130415.11211-18-ciara.power@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200930130415.11211-18-ciara.power@intel.com> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH v3 17/18] net: add checks for max SIMD bitwidth 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" Hi, On Wed, Sep 30, 2020 at 02:04:13PM +0100, Ciara Power wrote: > When choosing a vector path to take, an extra condition must be > satisfied to ensure the max SIMD bitwidth allows for the CPU enabled > path. > > The vector path was initially chosen in RTE_INIT, however this is no > longer suitable as we cannot check the max SIMD bitwidth at that time. > The default chosen in RTE_INIT is now scalar. For best performance > and to use vector paths, apps must explicitly call the set algorithm > function before using other functions from this library, as this is > where vector handlers are now chosen. > > Suggested-by: Jasvinder Singh > > Signed-off-by: Ciara Power > > --- > v3: > - Moved choosing vector paths out of RTE_INIT. > - Moved checking max_simd_bitwidth into the set_alg function. > --- > lib/librte_net/rte_net_crc.c | 26 +++++++++++++++++--------- > lib/librte_net/rte_net_crc.h | 3 ++- > 2 files changed, 19 insertions(+), 10 deletions(-) > > diff --git a/lib/librte_net/rte_net_crc.c b/lib/librte_net/rte_net_crc.c > index 9fd4794a9d..241eb16399 100644 > --- a/lib/librte_net/rte_net_crc.c > +++ b/lib/librte_net/rte_net_crc.c > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include > > #if defined(RTE_ARCH_X86_64) && defined(RTE_MACHINE_CPUFLAG_PCLMULQDQ) > #define X86_64_SSE42_PCLMULQDQ 1 > @@ -60,6 +61,9 @@ static rte_net_crc_handler handlers_neon[] = { > }; > #endif > > +static uint16_t max_simd_bitwidth; > +#define RTE_LOGTYPE_NET RTE_LOGTYPE_USER1 RTE_LOG_REGISTER() should be used instead. > + > /** > * Reflect the bits about the middle > * > @@ -145,18 +149,26 @@ rte_crc32_eth_handler(const uint8_t *data, uint32_t data_len) > void > rte_net_crc_set_alg(enum rte_net_crc_alg alg) > { > + if (max_simd_bitwidth == 0) > + max_simd_bitwidth = rte_get_max_simd_bitwidth(); > + > switch (alg) { > #ifdef X86_64_SSE42_PCLMULQDQ > case RTE_NET_CRC_SSE42: > - handlers = handlers_sse42; > - break; > + if (max_simd_bitwidth >= RTE_MAX_128_SIMD) { > + handlers = handlers_sse42; > + return; > + } > + RTE_LOG(INFO, NET, "Max SIMD Bitwidth too low, using scalar\n"); If max_simd_bitwidth is too low, it will keep the previous value. I think we should avoid to say "using scalar" in the log, even if it is correct today. For instance, when the avx implementation will be added, the log will become wrong. > #elif defined ARM64_NEON_PMULL > /* fall-through */ > case RTE_NET_CRC_NEON: > - if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_PMULL)) { > + if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_PMULL) && > + max_simd_bitwidth >= RTE_MAX_128_SIMD) { > handlers = handlers_neon; > - break; > + return; > } > + RTE_LOG(INFO, NET, "Max SIMD Bitwidth too low or CPU flag not enabled, using scalar\n"); > #endif > /* fall-through */ > case RTE_NET_CRC_SCALAR: > @@ -184,19 +196,15 @@ rte_net_crc_calc(const void *data, > /* Select highest available crc algorithm as default one */ > RTE_INIT(rte_net_crc_init) > { > - enum rte_net_crc_alg alg = RTE_NET_CRC_SCALAR; > - > rte_net_crc_scalar_init(); > > #ifdef X86_64_SSE42_PCLMULQDQ > - alg = RTE_NET_CRC_SSE42; > rte_net_crc_sse42_init(); > #elif defined ARM64_NEON_PMULL > if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_PMULL)) { > - alg = RTE_NET_CRC_NEON; > rte_net_crc_neon_init(); > } > #endif > > - rte_net_crc_set_alg(alg); > + rte_net_crc_set_alg(RTE_NET_CRC_SCALAR); > } > diff --git a/lib/librte_net/rte_net_crc.h b/lib/librte_net/rte_net_crc.h > index 16e85ca970..7a45ebe193 100644 > --- a/lib/librte_net/rte_net_crc.h > +++ b/lib/librte_net/rte_net_crc.h > @@ -28,7 +28,8 @@ enum rte_net_crc_alg { > /** > * This API set the CRC computation algorithm (i.e. scalar version, > * x86 64-bit sse4.2 intrinsic version, etc.) and internal data > - * structure. > + * structure. This should be called before any other functions, to > + * choose the algorithm for best performance. > * > * @param alg > * This parameter is used to select the CRC implementation version. > -- > 2.17.1 >