diff mbox series

[v3,2/7] drivers/bus: eliminate dependency on non-portable __SIZEOF_LONG__

Message ID 1733372429-3996-3-git-send-email-andremue@linux.microsoft.com (mailing list archive)
State New
Delegated to: David Marchand
Headers
Series eliminate dependency on non-portable __SIZEOF_LONG__ |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Andre Muezerie Dec. 5, 2024, 4:20 a.m. UTC 
Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it.
Therefore the errors below are seen with MSVC:

../lib/mldev/mldev_utils_scalar.c(465): error C2065:
    '__SIZEOF_LONG__': undeclared identifier
../lib/mldev/mldev_utils_scalar.c(478): error C2051:
    case expression not constant

../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065:
    '__SIZEOF_LONG__': undeclared identifier
../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051:
    case expression not constant

Turns out that the places where __SIZEOF_LONG__ is currently
being used can equally well use sizeof(long) instead.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 drivers/bus/fslmc/mc/fsl_mc_cmd.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Konstantin Ananyev Dec. 6, 2024, 12:22 p.m. UTC | #1 
> 
> Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it.
> Therefore the errors below are seen with MSVC:
> 
> ../lib/mldev/mldev_utils_scalar.c(465): error C2065:
>     '__SIZEOF_LONG__': undeclared identifier
> ../lib/mldev/mldev_utils_scalar.c(478): error C2051:
>     case expression not constant
> 
> ../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065:
>     '__SIZEOF_LONG__': undeclared identifier
> ../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051:
>     case expression not constant
> 
> Turns out that the places where __SIZEOF_LONG__ is currently
> being used can equally well use sizeof(long) instead.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> ---
>  drivers/bus/fslmc/mc/fsl_mc_cmd.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/bus/fslmc/mc/fsl_mc_cmd.h b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> index a768774c89..f27a18905d 100644
> --- a/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> +++ b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> @@ -29,9 +29,8 @@
>  #define le32_to_cpu	rte_le_to_cpu_32
>  #define le16_to_cpu	rte_le_to_cpu_16
> 
> -#define BITS_PER_LONG	(__SIZEOF_LONG__ * 8)
>  #define GENMASK(h, l) \
> -		(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
> +		(((~0UL) << (l)) & (~0UL >> (RTE_BITS_PER_LONG - 1 - (h))))

Inside 
There are macros: RTE_GENMASK64 (and RTE_GENMASK32).
Which as I understand does same thing.
Might be better to just use these ones instead of hand-crafting
same thing over different PMDs.

> 
>  struct mc_cmd_header {
>  	union {
> --
> 2.47.0.vfs.0.3
Andre Muezerie Dec. 6, 2024, 4:19 p.m. UTC | #2 
On Fri, Dec 06, 2024 at 12:22:42PM +0000, Konstantin Ananyev wrote:
> 
> 
> > 
> > Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it.
> > Therefore the errors below are seen with MSVC:
> > 
> > ../lib/mldev/mldev_utils_scalar.c(465): error C2065:
> >     '__SIZEOF_LONG__': undeclared identifier
> > ../lib/mldev/mldev_utils_scalar.c(478): error C2051:
> >     case expression not constant
> > 
> > ../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065:
> >     '__SIZEOF_LONG__': undeclared identifier
> > ../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051:
> >     case expression not constant
> > 
> > Turns out that the places where __SIZEOF_LONG__ is currently
> > being used can equally well use sizeof(long) instead.
> > 
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > ---
> >  drivers/bus/fslmc/mc/fsl_mc_cmd.h | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/bus/fslmc/mc/fsl_mc_cmd.h b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > index a768774c89..f27a18905d 100644
> > --- a/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > +++ b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > @@ -29,9 +29,8 @@
> >  #define le32_to_cpu	rte_le_to_cpu_32
> >  #define le16_to_cpu	rte_le_to_cpu_16
> > 
> > -#define BITS_PER_LONG	(__SIZEOF_LONG__ * 8)
> >  #define GENMASK(h, l) \
> > -		(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
> > +		(((~0UL) << (l)) & (~0UL >> (RTE_BITS_PER_LONG - 1 - (h))))
> 
> Inside 
> There are macros: RTE_GENMASK64 (and RTE_GENMASK32).
> Which as I understand does same thing.
> Might be better to just use these ones instead of hand-crafting
> same thing over different PMDs.
> 

I agree. I can submit a separate patch for that once this series aimed at
unblocking MSVC from being used on this code goes in, if that is OK. If
instead you feel strongly that this change should be made as part of this
series let me know.
--
Andre Muezerie

> > 
> >  struct mc_cmd_header {
> >  	union {
> > --
> > 2.47.0.vfs.0.3
Konstantin Ananyev Dec. 6, 2024, 4:41 p.m. UTC | #3 
> > > Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it.
> > > Therefore the errors below are seen with MSVC:
> > >
> > > ../lib/mldev/mldev_utils_scalar.c(465): error C2065:
> > >     '__SIZEOF_LONG__': undeclared identifier
> > > ../lib/mldev/mldev_utils_scalar.c(478): error C2051:
> > >     case expression not constant
> > >
> > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065:
> > >     '__SIZEOF_LONG__': undeclared identifier
> > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051:
> > >     case expression not constant
> > >
> > > Turns out that the places where __SIZEOF_LONG__ is currently
> > > being used can equally well use sizeof(long) instead.
> > >
> > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > > ---
> > >  drivers/bus/fslmc/mc/fsl_mc_cmd.h | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/bus/fslmc/mc/fsl_mc_cmd.h b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > index a768774c89..f27a18905d 100644
> > > --- a/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > +++ b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > @@ -29,9 +29,8 @@
> > >  #define le32_to_cpu	rte_le_to_cpu_32
> > >  #define le16_to_cpu	rte_le_to_cpu_16
> > >
> > > -#define BITS_PER_LONG	(__SIZEOF_LONG__ * 8)
> > >  #define GENMASK(h, l) \
> > > -		(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
> > > +		(((~0UL) << (l)) & (~0UL >> (RTE_BITS_PER_LONG - 1 - (h))))
> >
> > Inside
> > There are macros: RTE_GENMASK64 (and RTE_GENMASK32).
> > Which as I understand does same thing.
> > Might be better to just use these ones instead of hand-crafting
> > same thing over different PMDs.
> >
> 
> I agree. I can submit a separate patch for that once this series aimed at
> unblocking MSVC from being used on this code goes in, if that is OK. If
> instead you feel strongly that this change should be made as part of this
> series let me know.

I am fine anyway.
Just thought that doing something like:
#define GENMASK(h, l)	RTE_GENMASK64(h, l)
Would be less work for you and less code churn.

> --
> Andre Muezerie
> 
> > >
> > >  struct mc_cmd_header {
> > >  	union {
> > > --
> > > 2.47.0.vfs.0.3
Konstantin Ananyev Dec. 6, 2024, 4:43 p.m. UTC | #4 
> -----Original Message-----
> From: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> Sent: Friday, December 6, 2024 4:41 PM
> To: Andre Muezerie <andremue@linux.microsoft.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v3 2/7] drivers/bus: eliminate dependency on non-portable __SIZEOF_LONG__
> 
> 
> > > > Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it.
> > > > Therefore the errors below are seen with MSVC:
> > > >
> > > > ../lib/mldev/mldev_utils_scalar.c(465): error C2065:
> > > >     '__SIZEOF_LONG__': undeclared identifier
> > > > ../lib/mldev/mldev_utils_scalar.c(478): error C2051:
> > > >     case expression not constant
> > > >
> > > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065:
> > > >     '__SIZEOF_LONG__': undeclared identifier
> > > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051:
> > > >     case expression not constant
> > > >
> > > > Turns out that the places where __SIZEOF_LONG__ is currently
> > > > being used can equally well use sizeof(long) instead.
> > > >
> > > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > > > ---
> > > >  drivers/bus/fslmc/mc/fsl_mc_cmd.h | 3 +--
> > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/bus/fslmc/mc/fsl_mc_cmd.h b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > > index a768774c89..f27a18905d 100644
> > > > --- a/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > > +++ b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > > @@ -29,9 +29,8 @@
> > > >  #define le32_to_cpu	rte_le_to_cpu_32
> > > >  #define le16_to_cpu	rte_le_to_cpu_16
> > > >
> > > > -#define BITS_PER_LONG	(__SIZEOF_LONG__ * 8)
> > > >  #define GENMASK(h, l) \
> > > > -		(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
> > > > +		(((~0UL) << (l)) & (~0UL >> (RTE_BITS_PER_LONG - 1 - (h))))
> > >
> > > Inside
> > > There are macros: RTE_GENMASK64 (and RTE_GENMASK32).
> > > Which as I understand does same thing.
> > > Might be better to just use these ones instead of hand-crafting
> > > same thing over different PMDs.
> > >
> >
> > I agree. I can submit a separate patch for that once this series aimed at
> > unblocking MSVC from being used on this code goes in, if that is OK. If
> > instead you feel strongly that this change should be made as part of this
> > series let me know.
> 
> I am fine anyway.

Sorry meant to say: I am fine either way.
:)
 
> Just thought that doing something like:
> #define GENMASK(h, l)	RTE_GENMASK64(h, l)
> Would be less work for you and less code churn.
> 
> > --
> > Andre Muezerie
> >
> > > >
> > > >  struct mc_cmd_header {
> > > >  	union {
> > > > --
> > > > 2.47.0.vfs.0.3
Andre Muezerie Dec. 6, 2024, 6:14 p.m. UTC | #5 
On Fri, Dec 06, 2024 at 04:41:16PM +0000, Konstantin Ananyev wrote:
> 
> > > > Macro __SIZEOF_LONG__ is not standardized and MSVC does not define it.
> > > > Therefore the errors below are seen with MSVC:
> > > >
> > > > ../lib/mldev/mldev_utils_scalar.c(465): error C2065:
> > > >     '__SIZEOF_LONG__': undeclared identifier
> > > > ../lib/mldev/mldev_utils_scalar.c(478): error C2051:
> > > >     case expression not constant
> > > >
> > > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(33): error C2065:
> > > >     '__SIZEOF_LONG__': undeclared identifier
> > > > ../lib/mldev/mldev_utils_scalar_bfloat16.c(49): error C2051:
> > > >     case expression not constant
> > > >
> > > > Turns out that the places where __SIZEOF_LONG__ is currently
> > > > being used can equally well use sizeof(long) instead.
> > > >
> > > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > > > ---
> > > >  drivers/bus/fslmc/mc/fsl_mc_cmd.h | 3 +--
> > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/bus/fslmc/mc/fsl_mc_cmd.h b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > > index a768774c89..f27a18905d 100644
> > > > --- a/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > > +++ b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
> > > > @@ -29,9 +29,8 @@
> > > >  #define le32_to_cpu	rte_le_to_cpu_32
> > > >  #define le16_to_cpu	rte_le_to_cpu_16
> > > >
> > > > -#define BITS_PER_LONG	(__SIZEOF_LONG__ * 8)
> > > >  #define GENMASK(h, l) \
> > > > -		(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
> > > > +		(((~0UL) << (l)) & (~0UL >> (RTE_BITS_PER_LONG - 1 - (h))))
> > >
> > > Inside
> > > There are macros: RTE_GENMASK64 (and RTE_GENMASK32).
> > > Which as I understand does same thing.
> > > Might be better to just use these ones instead of hand-crafting
> > > same thing over different PMDs.
> > >
> > 
> > I agree. I can submit a separate patch for that once this series aimed at
> > unblocking MSVC from being used on this code goes in, if that is OK. If
> > instead you feel strongly that this change should be made as part of this
> > series let me know.
> 
> I am fine anyway.
> Just thought that doing something like:
> #define GENMASK(h, l)	RTE_GENMASK64(h, l)
> Would be less work for you and less code churn.
> 

I thought about that option too, but getting rid of that "redefine" and just 
replace all occurrences of GENMASK would be better I think, despite
the code churn.

> > --
> > Andre Muezerie
> > 
> > > >
> > > >  struct mc_cmd_header {
> > > >  	union {
> > > > --
> > > > 2.47.0.vfs.0.3
diff mbox series

Patch

diff --git a/drivers/bus/fslmc/mc/fsl_mc_cmd.h b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
index a768774c89..f27a18905d 100644
--- a/drivers/bus/fslmc/mc/fsl_mc_cmd.h
+++ b/drivers/bus/fslmc/mc/fsl_mc_cmd.h
@@ -29,9 +29,8 @@ 
 #define le32_to_cpu	rte_le_to_cpu_32
 #define le16_to_cpu	rte_le_to_cpu_16
 
-#define BITS_PER_LONG	(__SIZEOF_LONG__ * 8)
 #define GENMASK(h, l) \
-		(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
+		(((~0UL) << (l)) & (~0UL >> (RTE_BITS_PER_LONG - 1 - (h))))
 
 struct mc_cmd_header {
 	union {