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 BCE02A0487 for ; Mon, 1 Jul 2019 22:41:46 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0E6021B4B6; Mon, 1 Jul 2019 22:41:46 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id CACA55B34 for ; Mon, 1 Jul 2019 22:41:43 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Jul 2019 13:41:42 -0700 X-IronPort-AV: E=Sophos;i="5.63,440,1557212400"; d="scan'208";a="157406946" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.3.62]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Jul 2019 13:41:41 -0700 Date: Mon, 1 Jul 2019 21:41:37 +0100 From: Bruce Richardson To: Thomas Monjalon Cc: dev@dpdk.org Message-ID: <20190701204137.GB393@bricha3-MOBL.ger.corp.intel.com> References: <20190529154132.49955-1-bruce.richardson@intel.com> <20190529154132.49955-5-bruce.richardson@intel.com> <2785804.vqP48ztxbI@xps> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2785804.vqP48ztxbI@xps> User-Agent: Mutt/1.11.4 (2019-03-13) Subject: Re: [dpdk-dev] [PATCH 4/4] net: replace ifdefs with runtime branches 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" On Mon, Jul 01, 2019 at 09:30:02PM +0200, Thomas Monjalon wrote: > 29/05/2019 17:41, Bruce Richardson: > > Use the flag checking functions and a couple of empty stubs to remove the > > ifdefs from the middle of the C code, and replace them with more readable > > regular if statements. Other ifdefs at the top of the file are kept, but > > are not mixed with C code, so there is a clean separation. > > > > Signed-off-by: Bruce Richardson > > --- > > lib/librte_net/rte_net_crc.c | 38 ++++++++++++++++++++++++------------ > > 1 file changed, 25 insertions(+), 13 deletions(-) > > The result is more lines of code :) > > > --- a/lib/librte_net/rte_net_crc.c > > +++ b/lib/librte_net/rte_net_crc.c > > @@ -18,8 +18,17 @@ > > > > #ifdef X86_64_SSE42_PCLMULQDQ > > #include > > -#elif defined ARM64_NEON_PMULL > > +#else > > +/* define stubs for the SSE functions to avoid compiler errors */ > > +#define handlers_sse42 handlers_scalar > > +#define rte_net_crc_sse42_init() do { } while(0) > > +#endif > > + > > +#ifdef ARM64_NEON_PMULL > > #include > > +#else > > +#define handlers_neon handlers_scalar > > +#define rte_net_crc_neon_init() do { } while(0) > > #endif > > Looking at the need for stubs, I don't see the benefit. > Yes, one needs stubs, but those are placed in a single place, and the main C-code itself is free of ifdefs running through it. I'd view this as a good thing in limiting the scope of any ifdef-ery, since it annoys me looking at #ifdefs in the middle of functions (since it messes up the indentation flow of the code if nothing else!). If you don't see this as a big benefit, then there is not a lot else to commend this set for you, sadly. It just seemed a nice improvement to me - irrespective of net lines of code. /Bruce