From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 04D12A0A02; Wed, 19 May 2021 02:18:30 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 729B640143; Wed, 19 May 2021 02:18:30 +0200 (CEST) Received: from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35]) by mails.dpdk.org (Postfix) with ESMTP id A318940041 for ; Wed, 19 May 2021 02:18:29 +0200 (CEST) Received: from dggems703-chm.china.huawei.com (unknown [172.30.72.59]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4FlD0r5YmTzBvX4; Wed, 19 May 2021 08:15:40 +0800 (CST) Received: from dggpeml500024.china.huawei.com (7.185.36.10) by dggems703-chm.china.huawei.com (10.3.19.180) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Wed, 19 May 2021 08:18:26 +0800 Received: from [127.0.0.1] (10.40.190.165) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Wed, 19 May 2021 08:18:26 +0800 To: Ferruh Yigit , Honnappa Nagarahalli , "thomas@monjalon.net" , Bruce Richardson CC: "dev@dpdk.org" , "jerinj@marvell.com" , Ruifeng Wang , "viktorin@rehivetech.com" , "jerinjacobk@gmail.com" , "juraj.linkes@pantheon.tech" , nd References: <1620808126-18876-1-git-send-email-fengchengwen@huawei.com> <1620986039-29475-1-git-send-email-fengchengwen@huawei.com> <1620986039-29475-3-git-send-email-fengchengwen@huawei.com> <3028dea0-97f6-ed06-8017-418fd55e72a3@intel.com> <3aaf306b-79c6-42e7-d00c-07fac13ac640@intel.com> From: fengchengwen Message-ID: <8765778c-c891-1039-c15e-8a4039e7950a@huawei.com> Date: Wed, 19 May 2021 08:18:26 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.40.190.165] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To dggpeml500024.china.huawei.com (7.185.36.10) X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH v5 2/2] net/hns3: refactor SVE code compile method X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 2021/5/19 0:37, Ferruh Yigit wrote: > On 5/18/2021 5:12 PM, Honnappa Nagarahalli wrote: >>>> >>>>> >>>>> On 5/14/2021 10:53 AM, Chengwen Feng wrote: >>>>>> Currently, the SVE code is compiled only when -march supports SVE >>>>>> (e.g. '-march=armv8.2a+sve'), there maybe some problem[1] with this >>>>>> approach. >>>>>> >>>>>> The solution: >>>>>> a. If the minimum instruction set support SVE then compiles it. >>>>>> b. Else if the compiler support SVE then compiles it. >>>>>> c. Otherwise don't compile it. >>>>>> >>>>>> [1] https://mails.dpdk.org/archives/dev/2021-April/208189.html >>>>>> >>>>> >>>>> Hi Chengwen, >>>>> >>>>> As far as I understand from above problem statement, you want to >>>>> produce a binary that can run in two different platforms, one >>>>> supports only NEON instructions, other supports NEON + SVE. >>>>> >>>>> For this driver should be compiled in a way to support min >>>>> instruction set, which is NEON. >>>>> >>>>> There are two build items, >>>>> >>>>> 1) hns3_rxtx_vec_sve.c >>>>> 2) rest of the library >>>>> >>>>> There is already runtime checks to select Rx/Tx functions, so it is >>>>> safe to build >>>>> (1) as long as compiler supports. If the platform doesn't support >>>>> SVE, the SVE path won't be selected during runtime. >>>>> >>>>> For (2), it should be build to support NEON only, if it is compiled >>>>> to support SVE, it won't run on the platform that only supports NEON. >>>>> >>>>> So, in below, if '__ARM_FEATURE_SVE' is supported, all driver is >>>>> build with SVE support, won't this cause a problem on the NEON platform? >>>> The first if statement checks if the user has enabled SVE during compilation >>> which indicates that the user will run the binary on a platform that has SVE >>> (the minimum ISA level supported by this binary), hence it is ok to compile all >>> the code with SVE. >>>> >>> >>> So it is related to the what user provided (I assume as compiler flag), instead >>> of host HW capability. >> It is the HW host capability as provided in the compiler flag. It is coming from config/arm/meson.build. >> > > Is this patch has dependency to 1/2, that updates 'config/arm/meson.build'? > > What I understand is, if user provides compiler argument to request SVE, > something like '-march=armv8.2-a+sve', and host HW supports it, whole driver > will be built with SVE support. > > If user not request SVE, driver won't be compiled with SVE support even if HW > support it, but only 'hns3_rxtx_vec_sve.c' will be compiled if compiler supports > SVE. > > Is above correct and does it have any dependency to first patch, I thought this > is independent from first patch. > Yes, you are right, it's independent from first patch. > >>> >>>> If the user has not enabled SVE during compilation which indicates the user >>> might run the binary on a platform that does not have SVE, the second if >>> statement, checks if the compiler supports SVE. If yes, it will compile the SVE >>> version of the driver as well and the run time checks choose the correct >>> version. >>>> >>> >>> OK, this sounds good, thanks for clarification. >>> >>>>> >>>>> What do you think to only keep the else leg of the below check, which >>>>> is if compiler supports SVE, set '-DCC_SVE_SUPPORT' flag and only >>>>> build (1) with SVE flag? >>>>> >>>>>> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx") >>>>>> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx") >>>>>> Cc: stable@dpdk.org >>>>>> >>>>>> Signed-off-by: Chengwen Feng >>>>>> --- >>>>>> drivers/net/hns3/hns3_rxtx.c | 2 +- drivers/net/hns3/meson.build >>>>>> | 13 +++++++++++++ >>>>>> 2 files changed, 14 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/net/hns3/hns3_rxtx.c >>>>>> b/drivers/net/hns3/hns3_rxtx.c index 1d7a769..4ef20c6 100644 >>>>>> --- a/drivers/net/hns3/hns3_rxtx.c >>>>>> +++ b/drivers/net/hns3/hns3_rxtx.c >>>>>> @@ -2808,7 +2808,7 @@ hns3_get_default_vec_support(void) >>>>>> static bool >>>>>> hns3_get_sve_support(void) >>>>>> { >>>>>> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE) >>>>>> +#if defined(CC_SVE_SUPPORT) >>>>>> if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256) >>>>>> return false; >>>>>> if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE)) >>>>>> diff --git a/drivers/net/hns3/meson.build >>>>>> b/drivers/net/hns3/meson.build index 53c7df7..8563d70 100644 >>>>>> --- a/drivers/net/hns3/meson.build >>>>>> +++ b/drivers/net/hns3/meson.build >>>>>> @@ -35,7 +35,20 @@ deps += ['hash'] >>>>>> >>>>>> if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64') >>>>>> sources += files('hns3_rxtx_vec.c') >>>>>> + >>>>>> + # compile SVE when: >>>>>> + # a. support SVE in minimum instruction set baseline >>>>>> + # b. it's not minimum instruction set, but compiler support >>>>>> if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != '' >>>>>> + cflags += ['-DCC_SVE_SUPPORT'] >>>>>> sources += files('hns3_rxtx_vec_sve.c') >>>>>> + elif cc.has_argument('-march=armv8.2-a+sve') >>>>>> + cflags += ['-DCC_SVE_SUPPORT'] >>>>>> + hns3_sve_lib = static_library('hns3_sve_lib', >>>>>> + 'hns3_rxtx_vec_sve.c', >>>>>> + dependencies: [static_rte_ethdev], >>>>>> + include_directories: includes, >>>>>> + c_args: [cflags, '-march=armv8.2-a+sve']) >>>>>> + objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c') >>>>>> endif >>>>>> endif >>>>>> >>>> >> > > > . >