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 CDD43A0A0A; Thu, 20 May 2021 09:57:17 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8AB1F40143; Thu, 20 May 2021 09:57:17 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mails.dpdk.org (Postfix) with ESMTP id 25B8140041 for ; Thu, 20 May 2021 09:57:15 +0200 (CEST) IronPort-SDR: EYRxAzIp+Fxs1GBaj0oY9x670SXy9+sd4wJw2s7GuHwJPneHelqcOTYRMRhw0EMTcT/wIt2/Gw s5xg5lxVnGZw== X-IronPort-AV: E=McAfee;i="6200,9189,9989"; a="201225449" X-IronPort-AV: E=Sophos;i="5.82,313,1613462400"; d="scan'208";a="201225449" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 May 2021 00:57:14 -0700 IronPort-SDR: 6Xv5fT/PuAHke9g/tztqfoIaNy1MwDSAi/76et139+hRC6DrgFwVYFdJxq9K1jqSL4Yj/3+lzC kREfdm3l4cAg== X-IronPort-AV: E=Sophos;i="5.82,313,1613462400"; d="scan'208";a="412084816" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.241.8]) ([10.213.241.8]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 May 2021 00:57:10 -0700 To: fengchengwen , thomas@monjalon.net Cc: dev@dpdk.org, jerinj@marvell.com, ruifeng.wang@arm.com, viktorin@rehivetech.com, bruce.richardson@intel.com, Honnappa.Nagarahalli@arm.com, jerinjacobk@gmail.com, juraj.linkes@pantheon.tech, nd@arm.com References: <1620808126-18876-1-git-send-email-fengchengwen@huawei.com> <1621430731-27753-1-git-send-email-fengchengwen@huawei.com> <1621430731-27753-3-git-send-email-fengchengwen@huawei.com> <73ffa983-e44b-a7d2-456c-f010db0d5c48@intel.com> <8eed2ffc-0d8f-fb2e-7b0a-83e7a8727999@huawei.com> From: Ferruh Yigit X-User: ferruhy Message-ID: <4fff6150-81d5-cd90-fe9a-4dfd4dff7176@intel.com> Date: Thu, 20 May 2021 08:57:06 +0100 MIME-Version: 1.0 In-Reply-To: <8eed2ffc-0d8f-fb2e-7b0a-83e7a8727999@huawei.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v6 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 5/20/2021 2:11 AM, fengchengwen wrote: > > > On 2021/5/19 23:02, Ferruh Yigit wrote: >> On 5/19/2021 2:25 PM, 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 >>> >>> 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 >> >> The patch passes the CI build [1], but in that test sve file is not compiled at >> all because of missing header file [2]. >> > > Yes, it was designed as it. > In hns3 meson.build we call cc.check_header('arm_sve.h'), and gcc9 don't have this headerfile. > >> I wonder if the warning caused by conflicting switch [3] is still valid, we need >> a platform that sve file is compiled to verify this. Do you have this >> environment, that sets '-mcpu=armv8.1-a'. >> > > It already fix by filterout '-march' '-mcpu' '-mtune' in hns3 meson.build of this patch > If don't add the above filtout logic: > a) Test result with gcc8.3 and crossfile thunder2: > cc1: warning: switch ‘-mcpu=thunderx2t99’ conflicts with ‘-march=armv8.2-a+sve’ switch > b) Test result with gcc9.2 and crossfile thunder2: > cc1: warning: switch ‘-mcpu=armv8.1-a’ conflicts with ‘-march=armv8.2-a’ switch > > Note: the gcc8.3/9.2 version detail: > ./aarch64-linux-gnu-gcc --version > aarch64-linux-gnu-gcc (GNU Toolchain for the A-profile Architecture 8.3-2019.03 (arm-rel-8.36)) 8.3.0 > ./aarch64-none-linux-gnu-gcc --version > aarch64-none-linux-gnu-gcc (GNU Toolchain for the A-profile Architecture 9.2-2019.12 (arm-9.10)) 9.2.1 20191025 > Hi Chengwen, -rc4 is already out and release is planned for tomorrow, so it is late to get this patch now, let's proceed with it in next release. >> >> Btw, CI reports unit test failure, I don't think it is related with this patch >> but can you please double check? >> > > Agree, it is atomic_autotest and malloc_autotest failed, it hasn't run any hns3 driver's code. > >> >> >> [1] >> https://lab.dpdk.org/results/dashboard/patchsets/17135/ >> >> [2] >> Check usable header "arm_sve.h" : NO >> >> [3] >> error: switch ‘-mcpu=armv8.1-a’ conflicts with ‘-march=armv8.2-a’ switch [-Werror] >> >>> --- >>> drivers/net/hns3/hns3_rxtx.c | 2 +- >>> drivers/net/hns3/meson.build | 21 ++++++++++++++++++++- >>> 2 files changed, 21 insertions(+), 2 deletions(-) >>> >>> 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..5f9af9b 100644 >>> --- a/drivers/net/hns3/meson.build >>> +++ b/drivers/net/hns3/meson.build >>> @@ -35,7 +35,26 @@ deps += ['hash'] >>> >>> if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64') >>> sources += files('hns3_rxtx_vec.c') >>> - if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != '' >>> + >>> + # 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) != '' and cc.check_header('arm_sve.h') >>> + cflags += ['-DCC_SVE_SUPPORT'] >>> sources += files('hns3_rxtx_vec_sve.c') >>> + elif cc.has_argument('-march=armv8.2-a+sve') and cc.check_header('arm_sve.h') >>> + sve_cflags = ['-DCC_SVE_SUPPORT'] >>> + foreach flag: cflags >>> + # filterout -march -mcpu -mtune >>> + if not (flag.startswith('-march=') or flag.startswith('-mcpu=') or flag.startswith('-mtune=')) >>> + sve_cflags += flag >>> + endif >>> + endforeach >>> + hns3_sve_lib = static_library('hns3_sve_lib', >>> + 'hns3_rxtx_vec_sve.c', >>> + dependencies: [static_rte_ethdev], >>> + include_directories: includes, >>> + c_args: [sve_cflags, '-march=armv8.2-a+sve']) >>> + objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c') >>> endif >>> endif >>> >> >> >> . >> >