diff mbox series

[v2,2/5] net/hns3: fix build with sve enabled

Message ID 20210108082523.1062058-3-ruifeng.wang@arm.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series lpm lookup with sve support |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Ruifeng Wang Jan. 8, 2021, 8:25 a.m. UTC 
  Building with SVE extension enabled stopped with error:

 error: ACLE function ‘svwhilelt_b64_s32’ requires ISA extension ‘sve’
   18 | #define PG64_256BIT  svwhilelt_b64(0, 4)

This is caused by unintentional cflags reset.
Fixed the issue by appending required flag to cflags instead of
overriding it.

Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
Cc: xavier.huwei@huawei.com
Cc: stable@dpdk.org

Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 drivers/net/hns3/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Honnappa Nagarahalli Jan. 9, 2021, 12:06 a.m. UTC | #1 
<snip>

> 
> Building with SVE extension enabled stopped with error:
> 
>  error: ACLE function ‘svwhilelt_b64_s32’ requires ISA extension ‘sve’
>    18 | #define PG64_256BIT  svwhilelt_b64(0, 4)
> 
> This is caused by unintentional cflags reset.
> Fixed the issue by appending required flag to cflags instead of overriding it.
> 
> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> Cc: xavier.huwei@huawei.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>  drivers/net/hns3/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
> index 45cee34d9..798086357 100644
> --- a/drivers/net/hns3/meson.build
> +++ b/drivers/net/hns3/meson.build
> @@ -32,7 +32,7 @@ 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) != ''
> -		cflags = ['-DCC_SVE_SUPPORT']
> +		cflags += ['-DCC_SVE_SUPPORT']
This comment is unrelated to this patch. We need to be consistent with the macro definitions. Is '__ARM_FEATURE_SVE' not enough? If we need to define an additional flag, I would name it something like 'RTE_ARM_FEATURE_SVE'.

>  		sources += files('hns3_rxtx_vec_sve.c')
>  	endif
>  endif
> --
> 2.25.1
Lijun Ou Jan. 9, 2021, 2:11 a.m. UTC | #2 
在 2021/1/9 8:06, Honnappa Nagarahalli 写道:
> <snip>
> 
>>
>> Building with SVE extension enabled stopped with error:
>>
>>   error: ACLE function ‘svwhilelt_b64_s32’ requires ISA extension ‘sve’
>>     18 | #define PG64_256BIT  svwhilelt_b64(0, 4)
>>
>> This is caused by unintentional cflags reset.
>> Fixed the issue by appending required flag to cflags instead of overriding it.
>>
>> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
>> Cc: xavier.huwei@huawei.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
>> ---
>>   drivers/net/hns3/meson.build | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
>> index 45cee34d9..798086357 100644
>> --- a/drivers/net/hns3/meson.build
>> +++ b/drivers/net/hns3/meson.build
>> @@ -32,7 +32,7 @@ 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) != ''
>> -		cflags = ['-DCC_SVE_SUPPORT']
>> +		cflags += ['-DCC_SVE_SUPPORT']
> This comment is unrelated to this patch. We need to be consistent with the macro definitions. Is '__ARM_FEATURE_SVE' not enough? If we need to define an additional flag, I would name it something like 'RTE_ARM_FEATURE_SVE'.
> 
I think the __ARM_FEATURE_SVE is ok. if use the gcc version included SVE 
flag, it will be identified as __ARM_FEATURE_SVE. it is defined in the 
ARM SVE document.
>>   		sources += files('hns3_rxtx_vec_sve.c')
>>   	endif
>>   endif
>> --
>> 2.25.1
>
Lijun Ou Jan. 9, 2021, 2:15 a.m. UTC | #3 
在 2021/1/8 16:25, Ruifeng Wang 写道:
> Building with SVE extension enabled stopped with error:
> 
>   error: ACLE function ‘svwhilelt_b64_s32’ requires ISA extension ‘sve’
>     18 | #define PG64_256BIT  svwhilelt_b64(0, 4)
> 
> This is caused by unintentional cflags reset.
> Fixed the issue by appending required flag to cflags instead of
> overriding it.
> 
> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> Cc: xavier.huwei@huawei.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>   drivers/net/hns3/meson.build | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
> index 45cee34d9..798086357 100644
> --- a/drivers/net/hns3/meson.build
> +++ b/drivers/net/hns3/meson.build
> @@ -32,7 +32,7 @@ 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) != ''
> -		cflags = ['-DCC_SVE_SUPPORT']
> +		cflags += ['-DCC_SVE_SUPPORT']
Hi
   I noticed this patch, but I checked that the hns3 driver did not use 
this function.How did you compile it?

Thanks
Lijun Ou
>   		sources += files('hns3_rxtx_vec_sve.c')
>   	endif
>   endif
>
Ruifeng Wang Jan. 11, 2021, 2:27 a.m. UTC | #4 
> -----Original Message-----
> From: oulijun <oulijun@huawei.com>
> Sent: Saturday, January 9, 2021 10:16 AM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>; Wei Hu (Xavier)
> <xavier.huwei@huawei.com>; Min Hu (Connor) <humin29@huawei.com>;
> Yisen Zhuang <yisen.zhuang@huawei.com>; Huisong Li
> <lihuisong@huawei.com>; Chengchang Tang
> <tangchengchang@huawei.com>; Chengwen Feng
> <fengchengwen@huawei.com>
> Cc: dev@dpdk.org; vladimir.medvedkin@intel.com; jerinj@marvell.com;
> hemant.agrawal@nxp.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; stable@dpdk.org
> Subject: Re: [PATCH v2 2/5] net/hns3: fix build with sve enabled
> 
> 
> 
> 在 2021/1/8 16:25, Ruifeng Wang 写道:
> > Building with SVE extension enabled stopped with error:
> >
> >   error: ACLE function ‘svwhilelt_b64_s32’ requires ISA extension ‘sve’
> >     18 | #define PG64_256BIT  svwhilelt_b64(0, 4)
> >
> > This is caused by unintentional cflags reset.
> > Fixed the issue by appending required flag to cflags instead of
> > overriding it.
> >
> > Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> > Cc: xavier.huwei@huawei.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> >   drivers/net/hns3/meson.build | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/hns3/meson.build
> > b/drivers/net/hns3/meson.build index 45cee34d9..798086357 100644
> > --- a/drivers/net/hns3/meson.build
> > +++ b/drivers/net/hns3/meson.build
> > @@ -32,7 +32,7 @@ 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) != ''
> > -		cflags = ['-DCC_SVE_SUPPORT']
> > +		cflags += ['-DCC_SVE_SUPPORT']
> Hi
>    I noticed this patch, but I checked that the hns3 driver did not use this
> function.How did you compile it?

Hi,
The hns3 driver has sve rx/tx implementation in hns3_rxtx_vec_sve.c. This path
will be enabled when compiling with sve feature enabled.

I compiled it by using gcc-10.2 with flag '-march=armv8.3-a+sve'. 
You can try compile for n2 with the cross file added in this series (5/5).

Thanks,
Ruifeng
> 
> Thanks
> Lijun Ou
> >   		sources += files('hns3_rxtx_vec_sve.c')
> >   	endif
> >   endif
> >
Ruifeng Wang Jan. 11, 2021, 2:39 a.m. UTC | #5 
> -----Original Message-----
> From: oulijun <oulijun@huawei.com>
> Sent: Saturday, January 9, 2021 10:12 AM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Wei Hu (Xavier) <xavier.huwei@huawei.com>;
> Min Hu (Connor) <humin29@huawei.com>; Yisen Zhuang
> <yisen.zhuang@huawei.com>; Huisong Li <lihuisong@huawei.com>;
> Chengchang Tang <tangchengchang@huawei.com>; Chengwen Feng
> <fengchengwen@huawei.com>
> Cc: dev@dpdk.org; vladimir.medvedkin@intel.com; jerinj@marvell.com;
> hemant.agrawal@nxp.com; nd <nd@arm.com>; stable@dpdk.org
> Subject: Re: [PATCH v2 2/5] net/hns3: fix build with sve enabled
> 
> 
> 在 2021/1/9 8:06, Honnappa Nagarahalli 写道:
> > <snip>
> >
> >>
> >> Building with SVE extension enabled stopped with error:
> >>
> >>   error: ACLE function ‘svwhilelt_b64_s32’ requires ISA extension ‘sve’
> >>     18 | #define PG64_256BIT  svwhilelt_b64(0, 4)
> >>
> >> This is caused by unintentional cflags reset.
> >> Fixed the issue by appending required flag to cflags instead of overriding it.
> >>
> >> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> >> Cc: xavier.huwei@huawei.com
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> >> ---
> >>   drivers/net/hns3/meson.build | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/hns3/meson.build
> >> b/drivers/net/hns3/meson.build index 45cee34d9..798086357 100644
> >> --- a/drivers/net/hns3/meson.build
> >> +++ b/drivers/net/hns3/meson.build
> >> @@ -32,7 +32,7 @@ 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) != ''
> >> -		cflags = ['-DCC_SVE_SUPPORT']
> >> +		cflags += ['-DCC_SVE_SUPPORT']
> > This comment is unrelated to this patch. We need to be consistent with the
> macro definitions. Is '__ARM_FEATURE_SVE' not enough? If we need to
> define an additional flag, I would name it something like
> 'RTE_ARM_FEATURE_SVE'.
> >
> I think the __ARM_FEATURE_SVE is ok. if use the gcc version included SVE
> flag, it will be identified as __ARM_FEATURE_SVE. it is defined in the ARM
> SVE document.

Yes, we can rely on flags defined by compiler and no extra flag is needed.
I can update in next version to remove this section from meson file and replace CC_SVE_SUPPORT in code.
> >>   		sources += files('hns3_rxtx_vec_sve.c')
> >>   	endif
> >>   endif
> >> --
> >> 2.25.1
> >
Honnappa Nagarahalli Jan. 11, 2021, 1:38 p.m. UTC | #6 
<snip>

> > >
> > >>
> > >> Building with SVE extension enabled stopped with error:
> > >>
> > >>   error: ACLE function ‘svwhilelt_b64_s32’ requires ISA extension ‘sve’
> > >>     18 | #define PG64_256BIT  svwhilelt_b64(0, 4)
> > >>
> > >> This is caused by unintentional cflags reset.
> > >> Fixed the issue by appending required flag to cflags instead of overriding it.
> > >>
> > >> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> > >> Cc: xavier.huwei@huawei.com
> > >> Cc: stable@dpdk.org
> > >>
> > >> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > >> ---
> > >>   drivers/net/hns3/meson.build | 2 +-
> > >>   1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/net/hns3/meson.build
> > >> b/drivers/net/hns3/meson.build index 45cee34d9..798086357 100644
> > >> --- a/drivers/net/hns3/meson.build
> > >> +++ b/drivers/net/hns3/meson.build
> > >> @@ -32,7 +32,7 @@ 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) != ''
> > >> -cflags = ['-DCC_SVE_SUPPORT']
> > >> +cflags += ['-DCC_SVE_SUPPORT']
> > > This comment is unrelated to this patch. We need to be consistent
> > > with the
> > macro definitions. Is '__ARM_FEATURE_SVE' not enough? If we need to
> > define an additional flag, I would name it something like
> > 'RTE_ARM_FEATURE_SVE'.
> > >
> > I think the __ARM_FEATURE_SVE is ok. if use the gcc version included
> > SVE flag, it will be identified as __ARM_FEATURE_SVE. it is defined in
> > the ARM SVE document.
> 
> Yes, we can rely on flags defined by compiler and no extra flag is needed.
> I can update in next version to remove this section from meson file and replace
> CC_SVE_SUPPORT in code.
Sounds good to me.

> > >>   sources += files('hns3_rxtx_vec_sve.c')
> > >>   endif
> > >>   endif
> > >> --
> > >> 2.25.1
> > >
diff mbox series

Patch

diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
index 45cee34d9..798086357 100644
--- a/drivers/net/hns3/meson.build
+++ b/drivers/net/hns3/meson.build
@@ -32,7 +32,7 @@  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) != ''
-		cflags = ['-DCC_SVE_SUPPORT']
+		cflags += ['-DCC_SVE_SUPPORT']
 		sources += files('hns3_rxtx_vec_sve.c')
 	endif
 endif