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: sta...@dpdk.org >>> >>> Signed-off-by: Chengwen Feng <fengcheng...@huawei.com> >> >> 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 >>> >> >> >> . >> >
