From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f194.google.com (mail-wr0-f194.google.com [209.85.128.194]) by dpdk.org (Postfix) with ESMTP id E50E26CC5 for ; Thu, 26 Apr 2018 10:30:20 +0200 (CEST) Received: by mail-wr0-f194.google.com with SMTP id d1-v6so55802407wrj.13 for ; Thu, 26 Apr 2018 01:30:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:user-agent:in-reply-to:references:mime-version :content-transfer-encoding:subject:to:cc:from:message-id; bh=YwZFjbeloFL5nOX6lAnhGiAx+dJSXFqO4dILOzlAK2c=; b=ANnX19zX9qxLifDbx/aLFZBGU9jgeR/KibQmpZXFAnn/ncxHvCR6Q+quqwqyh058rP J/5LbIPdCvQFzcvg6ofkN1C4OK2qqlwFgqxPItyaD7nLKDgE3CYcwYgBfmB1/8P1ztcK jb+q2sHiLtH5yF1cXYRZn1oNjSOqJdnEHkrXM4Weo0AJhth3ekxMAHHqhYfpx0DckHY2 LGO48XMwm/9Zxfm5CZcfFhIlQqmhTn2TAmpzDb0VeyKcq+zfUSXh/UUYfB5ZS+PySxlB GHDloZeSDprRxm3XiB1sky21Caw7nyqcSPFwFijOC7mEuaw7sMElnRidA6yp5Hf5vkrM Ky4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:user-agent:in-reply-to:references :mime-version:content-transfer-encoding:subject:to:cc:from :message-id; bh=YwZFjbeloFL5nOX6lAnhGiAx+dJSXFqO4dILOzlAK2c=; b=jYNIFTL25CZgNQdz0nobRdKFDm7zPUVYRGpUExF7MI57O1CKXpcIDQRpI8OlInFHE7 Oc5x/0lJLA8r3nI5TXesyjzzJjC7c4iRbZFLY95v+iBTOz7ccVKU9LZpt2QQQFcvu6eo rxaxqdknrGXrmqnVhVc2ZHd/6vdfus5Z4fC4A0R3HtBwnw9NKpg3WsGOtQseR0Mx8EXT nBFYFkI5mZd6A8YBmWNqOelW51U1oQAz89DW4Hyx0J0PprJm7LCzOZoilkX8h6PkRCvI fLres4qwiv4u1p7os1Vj+0WSwql8UBgIZKQbSpyHvgLaE+wY8Lf3DIhv4oiVhr7e/GPe C5vw== X-Gm-Message-State: ALQs6tAV8ohxP8/pDeAvvJc2leF8S4LsEYiMEAQ1Ecc+ycF7Cwjoj71C naDT/PFXvOCPknh/A5I3PuK/SQ== X-Google-Smtp-Source: AIpwx4/1gOE9Q8CpP8N1uelcTXsRBcx6ateujorzGAOiIgFIpc+dBVh8dIdzTiGpjJd24dAjLN4IwA== X-Received: by 2002:adf:884c:: with SMTP id e12-v6mr27676782wre.30.1524731420346; Thu, 26 Apr 2018 01:30:20 -0700 (PDT) Received: from [100.73.236.124] ([37.168.121.72]) by smtp.gmail.com with ESMTPSA id q188sm19032282wmb.37.2018.04.26.01.30.15 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 26 Apr 2018 01:30:19 -0700 (PDT) Date: Thu, 26 Apr 2018 10:29:07 +0200 User-Agent: K-9 Mail for Android In-Reply-To: <20180425155158.GA14975@ltp-pvn> References: <20180419185159.11266-1-pbhagavatula@caviumnetworks.com> <20180423090009.vpzv74r3uknh4jnx@platinum> <20180425155158.GA14975@ltp-pvn> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable To: Pavan Nikhilesh , thomas@monjalon.net, jerin.jacob@caviumnetworks.com, hemant.agrawal@nxp.com, beilei.xing@intel.com, rasesh.mody@cavium.com, harish.patil@cavium.com, jianbo.liu@arm.com CC: dev@dpdk.org From: Olivier Matz Message-ID: Subject: Re: [dpdk-dev] [PATCH] drivers: cleanup unnecessary global variables 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: , X-List-Received-Date: Thu, 26 Apr 2018 08:30:21 -0000 Le 25 avril 2018 17:52:00 GMT+02:00, Pavan Nikhilesh a =C3=A9crit : >On Mon, Apr 23, 2018 at 11:00:09AM +0200, Olivier Matz wrote: >> On Fri, Apr 20, 2018 at 12:21:59AM +0530, Pavan Nikhilesh wrote: >> > Signed-off-by: Pavan Nikhilesh >> > --- >> > drivers/bus/dpaa/base/fman/netcfg_layer=2Ec | 5 ----- >> > drivers/bus/dpaa/base/qbman/bman_driver=2Ec | 4 ++-- >> > drivers/bus/dpaa/base/qbman/qman=2Ec | 2 +- >> > drivers/bus/dpaa/base/qbman/qman_driver=2Ec | 4 ++-- >> > drivers/bus/dpaa/base/qbman/qman_priv=2Eh | 1 - >> > drivers/bus/dpaa/dpaa_bus=2Ec | 2 +- >> > drivers/bus/fslmc/qbman/qbman_portal=2Ec | 3 +-- >> > drivers/bus/fslmc/qbman/qbman_portal=2Eh | 1 - >> > drivers/net/i40e/i40e_flow=2Ec | 2 +- >> > drivers/net/qede/base/bcm_osal=2Ec | 2 +- >> > drivers/raw/skeleton_rawdev/skeleton_rawdev=2Ec | 2 +- >> > lib/librte_net/net_crc_neon=2Eh | 4 ++-- >> > 12 files changed, 12 insertions(+), 20 deletions(-) >> >> [=2E=2E=2E] >> >> > diff --git a/lib/librte_net/net_crc_neon=2Eh >b/lib/librte_net/net_crc_neon=2Eh >> > index 63fa1d4a1=2E=2Ecb3da72ed 100644 >> > --- a/lib/librte_net/net_crc_neon=2Eh >> > +++ b/lib/librte_net/net_crc_neon=2Eh >> > @@ -21,8 +21,8 @@ struct crc_pmull_ctx { >> > uint64x2_t rk7_rk8; >> > }; >> > >> > -struct crc_pmull_ctx crc32_eth_pmull __rte_aligned(16); >> > -struct crc_pmull_ctx crc16_ccitt_pmull __rte_aligned(16); >> > +static struct crc_pmull_ctx crc32_eth_pmull __rte_aligned(16); >> > +static struct crc_pmull_ctx crc16_ccitt_pmull __rte_aligned(16); >> > >> > /** >> >> Not sure it will still work after that=2E >> >> From what I see, these global variables are initialized once in >> rte_net_crc_neon_init, and used as a const parameter in >> crc32_eth_calc_pmull()=2E >> >> Changing them to static will create an instance of these variables >for >> each included file, which is not what we want=2E >> >> I think that the proper way to solve it would be to add the >definition >> in a new =2Ec file, and only have a declaration in the =2Eh=2E >> >> >Hi Olivier, > >Thanks for the heads up, the second solution seems more viable and >while >implementing it I faced few Issues=2E GCC doesnt suport const vector >instructions >i=2Ee=2E the following assignment throw as compiler error=2E > > static const struct crc_pmull_ctx crc32_eth_pmull =3D { > =2Erk1_rk2 =3D vld1q_u64((uint64_t[2]){0xccaa009eLLU, 0x1751997d0LLU}= ), > =2Erk5_rk6 =3D vld1q_u64((uint64_t[2]){0xccaa009eLLU, 0x163cd6124LLU}= ), > =2Erk7_rk8 =3D vld1q_u64((uint64_t[2]){0x1f7011640LLU, 0x1db710641LLU}= ), > } __rte_aligned(16); > >I have gotten path the error by modifying struct crc_pmull_ctx as >follows: > > struct crc_pmull_ctx { > union { > uint64_t rk12[2]; > uint64x2_t rk1_rk2; > }; > union { > uint64_t rk56[2]; > uint64x2_t rk5_rk6; > }; > union { > uint64_t rk78[2]; > uint64x2_t rk7_rk8; > }; > }; > > static const struct crc_pmull_ctx crc32_eth_pmull __rte_aligned(16) =3D >{ > =2Erk12 =3D {0xccaa009eLLU, 0x1751997d0LLU}, > =2Erk56 =3D {0xccaa009eLLU, 0x163cd6124LLU}, > =2Erk78 =3D {0x1f7011640LLU, 0x1db710641LLU}, > }; > > static const struct crc_pmull_ctx crc16_ccitt_pmull __rte_aligned(16) >=3D { > =2Erk12 =3D {0x189aeLLU, 0x8e10LLU}, > =2Erk56 =3D {0x189aeLLU, 0x114aaLLU}, > =2Erk78 =3D {0x11c581910LLU, 0x10811LLU}, > }; > >I have checked the hex dump of the assignment with the current code and >the >above piece of code and they are similar=2E > >Let me know if my solution seems viable I will send the v2=2E > Looks good, just wondering about possible endianness issues=2E Is arm arch= itecture supported with both little and big endian in dpdk ? >> An even better way would be to make variable const and initialize it >> with its content=2E It could even enhance performance=2E Something like= : >> >> net_crc_neon=2Eh: >> >> static const struct crc_pmull_ctx crc32_eth_pmull =3D { >> >> } __rte_aligned(16); >> >> static const struct crc_pmull_ctx crc16_ccitt_pmull =3D { >> >> } __rte_aligned(16); >> > >Thanks, >Pavan=2E