Re: [PATCH v4 0/7] eliminate dependency on non-portable __SIZEOF_LONG__

[Andre Muezerie <andremue@linux.microsoft.com>]

On Wed, Feb 05, 2025 at 03:50:03PM +0000, Konstantin Ananyev wrote:
> 
> > On Wed, Feb 05, 2025 at 07:37:21AM -0800, Andre Muezerie wrote:
> > > On Wed, Feb 05, 2025 at 09:15:43AM +0000, Bruce Richardson wrote:
> > > > On Tue, Feb 04, 2025 at 10:54:24AM -0800, Andre Muezerie 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.
> > > > >
> > > > > v4:
> > > > >  * rebased on latest main as previous patch was not applying cleanly
> > > > >    anymore.
> > > > >
> > > > > v3:
> > > > >  * added prefix RTE_ to BITS_PER_LONG* and moved them to rte_common.h
> > > > >  * defined PLT_BITS_PER_LONG* in drivers/common/cnxk/roc_platform.h to
> > > > >    avoid warnings from checkpatches.sh like:
> > > > >
> > > > >    Warning in drivers/common/cnxk/roc_bits.h:
> > > > >    Warning in drivers/common/cnxk/roc_ie_ot.h:
> > > > >    Warning in drivers/common/cnxk/roc_ie_ot_tls.h:
> > > > >    Use plt_ symbols instead of rte_ API in cnxk base driver
> > > > >
> > > > >    It can be seen that the same was done in the past for similar
> > > > >    macros like PLT_CACHE_LINE_SIZE
> > > > >
> > > > > v2:
> > > > >  * fixed typo in commit message
> > > > >
> > > > > Andre Muezerie (7):
> > > > >   eal: eliminate dependency on non-portable __SIZEOF_LONG__
> > > > >   drivers/bus: eliminate dependency on non-portable __SIZEOF_LONG__
> > > > >   drivers/common: eliminate dependency on non-portable __SIZEOF_LONG__
> > > > >   drivers/dma: eliminate dependency on non-portable __SIZEOF_LONG__
> > > > >   drivers/net: eliminate dependency on non-portable __SIZEOF_LONG__
> > > > >   drivers/raw: eliminate dependency on non-portable __SIZEOF_LONG__
> > > > >   mldev: eliminate dependency on non-portable __SIZEOF_LONG__
> > > > >
> > > > Just out of interest, is there are reason why the simple solution of just
> > > > putting "#define __SIZEOF_LONG__ (sizeof(long))" in a header file for MSVC
> > > > is not done? Should be a couple of lines in a single patch, rather than a
> > > > 7-patch series, no?
> > > >
> > > > After all, just because something is non-standard, doesn't mean that we
> > > > can't use it if its widely available.
> > > >
> > > > /Bruce
> > >
> > > Yes, that can be done instead. I'll send out a new series with that approach.
> > >
> > Maybe wait in case there is input from others before spending time on a
> > patch? I think the simpler solution is better, but others may feel that
> > removing the macro completely is the better approach.
> 
> +1 to what Bruce suggested.
>  

Thomas also mentioned he would prefer to minimize changes. I'm fine with that too, as
that is good enough to get the code to compile with MSVC.