[v5,11/14] eal: expand most macros to empty when using MSVC
Checks
Commit Message
For now expand a lot of common rte macros empty. The catch here is we
need to test that most of the macros do what they should but at the same
time they are blocking work needed to bootstrap of the unit tests.
Later we will return and provide (where possible) expansions that work
correctly for msvc and where not possible provide some alternate macros
to achieve the same outcome.
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
lib/eal/include/rte_branch_prediction.h | 8 ++++++
lib/eal/include/rte_common.h | 45 +++++++++++++++++++++++++++++++++
lib/eal/include/rte_compat.h | 20 +++++++++++++++
3 files changed, 73 insertions(+)
Comments
> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Thursday, 13 April 2023 23.26
>
> For now expand a lot of common rte macros empty. The catch here is we
> need to test that most of the macros do what they should but at the same
> time they are blocking work needed to bootstrap of the unit tests.
>
> Later we will return and provide (where possible) expansions that work
> correctly for msvc and where not possible provide some alternate macros
> to achieve the same outcome.
>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
> lib/eal/include/rte_branch_prediction.h | 8 ++++++
> lib/eal/include/rte_common.h | 45
> +++++++++++++++++++++++++++++++++
> lib/eal/include/rte_compat.h | 20 +++++++++++++++
> 3 files changed, 73 insertions(+)
>
> diff --git a/lib/eal/include/rte_branch_prediction.h
> b/lib/eal/include/rte_branch_prediction.h
> index 0256a9d..d9a0224 100644
> --- a/lib/eal/include/rte_branch_prediction.h
> +++ b/lib/eal/include/rte_branch_prediction.h
> @@ -25,7 +25,11 @@
> *
> */
> #ifndef likely
> +#ifndef RTE_TOOLCHAIN_MSVC
> #define likely(x) __builtin_expect(!!(x), 1)
> +#else
> +#define likely(x) (x)
This must be (!!(x)), because x may be non-Boolean, e.g. likely(n & 0x10), and likely() must return Boolean (0 or 1).
> +#endif
> #endif /* likely */
>
> /**
> @@ -39,7 +43,11 @@
> *
> */
> #ifndef unlikely
> +#ifndef RTE_TOOLCHAIN_MSVC
> #define unlikely(x) __builtin_expect(!!(x), 0)
> +#else
> +#define unlikely(x) (x)
This must also be (!!(x)), for the same reason as above.
> +#endif
> #endif /* unlikely */
>
> #ifdef __cplusplus
> diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> index 2f464e3..1bdaa2d 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -65,7 +65,11 @@
> /**
> * Force alignment
> */
> +#ifndef RTE_TOOLCHAIN_MSVC
> #define __rte_aligned(a) __attribute__((__aligned__(a)))
> +#else
> +#define __rte_aligned(a)
> +#endif
It should be reviewed that __rte_aligned() is only used for optimization purposes, and is not required for DPDK to function properly.
>
> #ifdef RTE_ARCH_STRICT_ALIGN
> typedef uint64_t unaligned_uint64_t __rte_aligned(1);
> @@ -80,16 +84,29 @@
> /**
> * Force a structure to be packed
> */
> +#ifndef RTE_TOOLCHAIN_MSVC
> #define __rte_packed __attribute__((__packed__))
> +#else
> +#define __rte_packed
> +#endif
Similar comment as for __rte_aligned(); however, I consider it more likely that structure packing is a functional requirement, and not just used for optimization. Based on my experience, it may be used for packing network structures; perhaps not in DPDK itself but maybe in DPDK applications.
The same risk applies to __rte_aligned(), but with lower probability.
On Fri, Apr 14, 2023 at 08:45:17AM +0200, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Thursday, 13 April 2023 23.26
> >
> > For now expand a lot of common rte macros empty. The catch here is we
> > need to test that most of the macros do what they should but at the same
> > time they are blocking work needed to bootstrap of the unit tests.
> >
> > Later we will return and provide (where possible) expansions that work
> > correctly for msvc and where not possible provide some alternate macros
> > to achieve the same outcome.
> >
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> > lib/eal/include/rte_branch_prediction.h | 8 ++++++
> > lib/eal/include/rte_common.h | 45
> > +++++++++++++++++++++++++++++++++
> > lib/eal/include/rte_compat.h | 20 +++++++++++++++
> > 3 files changed, 73 insertions(+)
> >
> > diff --git a/lib/eal/include/rte_branch_prediction.h
> > b/lib/eal/include/rte_branch_prediction.h
> > index 0256a9d..d9a0224 100644
> > --- a/lib/eal/include/rte_branch_prediction.h
> > +++ b/lib/eal/include/rte_branch_prediction.h
> > @@ -25,7 +25,11 @@
> > *
> > */
> > #ifndef likely
> > +#ifndef RTE_TOOLCHAIN_MSVC
> > #define likely(x) __builtin_expect(!!(x), 1)
> > +#else
> > +#define likely(x) (x)
>
> This must be (!!(x)), because x may be non-Boolean, e.g. likely(n & 0x10), and likely() must return Boolean (0 or 1).
>
Will this really make a difference? Is there somewhere likely/unlikely
would be used where we would not get the same conversion to boolean than we
get using "!!" operator. [NOTE: Not saying we shouldn't put in the !!, just
wondering if there are actual cases where it affects the output?]
> > +#endif
> > #endif /* likely */
> >
> > /**
> > @@ -39,7 +43,11 @@
> > *
> > */
> > #ifndef unlikely
> > +#ifndef RTE_TOOLCHAIN_MSVC
> > #define unlikely(x) __builtin_expect(!!(x), 0)
> > +#else
> > +#define unlikely(x) (x)
>
> This must also be (!!(x)), for the same reason as above.
>
> > +#endif
> > #endif /* unlikely */
> >
> > #ifdef __cplusplus
> > diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> > index 2f464e3..1bdaa2d 100644
> > --- a/lib/eal/include/rte_common.h
> > +++ b/lib/eal/include/rte_common.h
> > @@ -65,7 +65,11 @@
> > /**
> > * Force alignment
> > */
> > +#ifndef RTE_TOOLCHAIN_MSVC
> > #define __rte_aligned(a) __attribute__((__aligned__(a)))
> > +#else
> > +#define __rte_aligned(a)
> > +#endif
>
> It should be reviewed that __rte_aligned() is only used for optimization purposes, and is not required for DPDK to function properly.
>
Good point.
If we look across all of DPDK, things will likely break, as we are relying
on alignment in various places to use the aligned versions of instructions.
For example _mm256_load_si256() vs _mm256_loadu_si256() in our x86
vectorized driver code. A "git grep _load_si" shows quite a few aligned
vector load instructions used in our codebase. These will fault and cause a
crash if the data is not properly aligned. [I suspect that there are similar
restrictions on other architectures too, just not familiar with their
intrinsics to check.]
However, it may be that none of the code paths where these are used is
in code currently compiled on windows, so this may be safe for now. The
occurances are mostly in drivers.
$ git grep -l _load_si
drivers/common/idpf/idpf_common_rxtx_avx512.c
drivers/event/dlb2/dlb2.c
drivers/net/bnxt/bnxt_rxtx_vec_avx2.c
drivers/net/bnxt/bnxt_rxtx_vec_sse.c
drivers/net/enic/enic_rxtx_vec_avx2.c
drivers/net/i40e/i40e_rxtx_vec_avx2.c
drivers/net/i40e/i40e_rxtx_vec_avx512.c
drivers/net/iavf/iavf_rxtx_vec_avx2.c
drivers/net/iavf/iavf_rxtx_vec_avx512.c
drivers/net/iavf/iavf_rxtx_vec_sse.c
drivers/net/ice/ice_rxtx_vec_avx2.c
drivers/net/ice/ice_rxtx_vec_avx512.c
drivers/net/ice/ice_rxtx_vec_sse.c
drivers/net/mlx5/mlx5_rxtx_vec_sse.h
lib/acl/acl_bld.c
lib/distributor/rte_distributor_match_sse.c
lib/efd/rte_efd_x86.h
lib/hash/rte_cuckoo_hash.c
lib/member/rte_member_x86.h
lib/net/net_crc_avx512.c
lib/net/net_crc_sse.c
> >
> > #ifdef RTE_ARCH_STRICT_ALIGN
> > typedef uint64_t unaligned_uint64_t __rte_aligned(1);
> > @@ -80,16 +84,29 @@
> > /**
> > * Force a structure to be packed
> > */
> > +#ifndef RTE_TOOLCHAIN_MSVC
> > #define __rte_packed __attribute__((__packed__))
> > +#else
> > +#define __rte_packed
> > +#endif
>
> Similar comment as for __rte_aligned(); however, I consider it more likely that structure packing is a functional requirement, and not just used for optimization. Based on my experience, it may be used for packing network structures; perhaps not in DPDK itself but maybe in DPDK applications.
>
+1
Once libraries such as the net library in DPDK will form part of the
windows build this will need to be addressed or things will break.
> The same risk applies to __rte_aligned(), but with lower probability.
>
/Bruce
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Friday, 14 April 2023 11.22
>
> On Fri, Apr 14, 2023 at 08:45:17AM +0200, Morten Brørup wrote:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Thursday, 13 April 2023 23.26
> > >
> > > For now expand a lot of common rte macros empty. The catch here is we
> > > need to test that most of the macros do what they should but at the same
> > > time they are blocking work needed to bootstrap of the unit tests.
> > >
> > > Later we will return and provide (where possible) expansions that work
> > > correctly for msvc and where not possible provide some alternate macros
> > > to achieve the same outcome.
> > >
> > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > ---
> > > lib/eal/include/rte_branch_prediction.h | 8 ++++++
> > > lib/eal/include/rte_common.h | 45
> > > +++++++++++++++++++++++++++++++++
> > > lib/eal/include/rte_compat.h | 20 +++++++++++++++
> > > 3 files changed, 73 insertions(+)
> > >
> > > diff --git a/lib/eal/include/rte_branch_prediction.h
> > > b/lib/eal/include/rte_branch_prediction.h
> > > index 0256a9d..d9a0224 100644
> > > --- a/lib/eal/include/rte_branch_prediction.h
> > > +++ b/lib/eal/include/rte_branch_prediction.h
> > > @@ -25,7 +25,11 @@
> > > *
> > > */
> > > #ifndef likely
> > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > #define likely(x) __builtin_expect(!!(x), 1)
> > > +#else
> > > +#define likely(x) (x)
> >
> > This must be (!!(x)), because x may be non-Boolean, e.g. likely(n & 0x10),
> and likely() must return Boolean (0 or 1).
> >
>
> Will this really make a difference? Is there somewhere likely/unlikely
> would be used where we would not get the same conversion to boolean than we
> get using "!!" operator. [NOTE: Not saying we shouldn't put in the !!, just
> wondering if there are actual cases where it affects the output?]
I agree that it makes no difference the way it is typically used.
But there are creative developers out there, so these macros definitely need the "!!" conversion to Boolean.
>
> > > +#endif
> > > #endif /* likely */
> > >
> > > /**
> > > @@ -39,7 +43,11 @@
> > > *
> > > */
> > > #ifndef unlikely
> > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > #define unlikely(x) __builtin_expect(!!(x), 0)
> > > +#else
> > > +#define unlikely(x) (x)
> >
> > This must also be (!!(x)), for the same reason as above.
> >
> > > +#endif
> > > #endif /* unlikely */
> > >
> > > #ifdef __cplusplus
> > > diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> > > index 2f464e3..1bdaa2d 100644
> > > --- a/lib/eal/include/rte_common.h
> > > +++ b/lib/eal/include/rte_common.h
> > > @@ -65,7 +65,11 @@
> > > /**
> > > * Force alignment
> > > */
> > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > #define __rte_aligned(a) __attribute__((__aligned__(a)))
> > > +#else
> > > +#define __rte_aligned(a)
> > > +#endif
> >
> > It should be reviewed that __rte_aligned() is only used for optimization
> purposes, and is not required for DPDK to function properly.
> >
>
> Good point.
>
> If we look across all of DPDK, things will likely break, as we are relying
> on alignment in various places to use the aligned versions of instructions.
> For example _mm256_load_si256() vs _mm256_loadu_si256() in our x86
> vectorized driver code. A "git grep _load_si" shows quite a few aligned
> vector load instructions used in our codebase. These will fault and cause a
> crash if the data is not properly aligned. [I suspect that there are similar
> restrictions on other architectures too, just not familiar with their
> intrinsics to check.]
Another thing that has been annoying me with the use of vector instructions:
Vector instructions are often used in a way where they cast away the type they are working on, so if that type is modified (e.g. a field is moved), the code will happily build, but fail at runtime.
When casting away the type for vector instructions, _Static_assert or BUILD_BUG_ON should be used to verify the assumptions about the cast away type. Such a practice might catch some of the places where the missing alignment (and missing structure packing) would fail.
>
> However, it may be that none of the code paths where these are used is
> in code currently compiled on windows, so this may be safe for now. The
> occurances are mostly in drivers.
>
> $ git grep -l _load_si
> drivers/common/idpf/idpf_common_rxtx_avx512.c
> drivers/event/dlb2/dlb2.c
> drivers/net/bnxt/bnxt_rxtx_vec_avx2.c
> drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> drivers/net/enic/enic_rxtx_vec_avx2.c
> drivers/net/i40e/i40e_rxtx_vec_avx2.c
> drivers/net/i40e/i40e_rxtx_vec_avx512.c
> drivers/net/iavf/iavf_rxtx_vec_avx2.c
> drivers/net/iavf/iavf_rxtx_vec_avx512.c
> drivers/net/iavf/iavf_rxtx_vec_sse.c
> drivers/net/ice/ice_rxtx_vec_avx2.c
> drivers/net/ice/ice_rxtx_vec_avx512.c
> drivers/net/ice/ice_rxtx_vec_sse.c
> drivers/net/mlx5/mlx5_rxtx_vec_sse.h
> lib/acl/acl_bld.c
> lib/distributor/rte_distributor_match_sse.c
> lib/efd/rte_efd_x86.h
> lib/hash/rte_cuckoo_hash.c
> lib/member/rte_member_x86.h
> lib/net/net_crc_avx512.c
> lib/net/net_crc_sse.c
>
>
> > >
> > > #ifdef RTE_ARCH_STRICT_ALIGN
> > > typedef uint64_t unaligned_uint64_t __rte_aligned(1);
> > > @@ -80,16 +84,29 @@
> > > /**
> > > * Force a structure to be packed
> > > */
> > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > #define __rte_packed __attribute__((__packed__))
> > > +#else
> > > +#define __rte_packed
> > > +#endif
> >
> > Similar comment as for __rte_aligned(); however, I consider it more likely
> that structure packing is a functional requirement, and not just used for
> optimization. Based on my experience, it may be used for packing network
> structures; perhaps not in DPDK itself but maybe in DPDK applications.
> >
>
> +1
> Once libraries such as the net library in DPDK will form part of the
> windows build this will need to be addressed or things will break.
Yes. And for application developers, we should deprecate and replace the __rte_packed macro with something that works on both MSVC and GCC/CLANG. The same probably goes for __rte_aligned().
But, let's not hold back Tyler's work. Just put it on the long term TODO list for MSVC support.
>
> > The same risk applies to __rte_aligned(), but with lower probability.
> >
>
> /Bruce
On Fri, Apr 14, 2023 at 02:39:03PM +0200, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Friday, 14 April 2023 11.22
> >
> > On Fri, Apr 14, 2023 at 08:45:17AM +0200, Morten Brørup wrote:
> > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > > Sent: Thursday, 13 April 2023 23.26
> > > >
> > > > For now expand a lot of common rte macros empty. The catch here is we
> > > > need to test that most of the macros do what they should but at the same
> > > > time they are blocking work needed to bootstrap of the unit tests.
> > > >
> > > > Later we will return and provide (where possible) expansions that work
> > > > correctly for msvc and where not possible provide some alternate macros
> > > > to achieve the same outcome.
> > > >
> > > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > > ---
> > > > lib/eal/include/rte_branch_prediction.h | 8 ++++++
> > > > lib/eal/include/rte_common.h | 45
> > > > +++++++++++++++++++++++++++++++++
> > > > lib/eal/include/rte_compat.h | 20 +++++++++++++++
> > > > 3 files changed, 73 insertions(+)
> > > >
> > > > diff --git a/lib/eal/include/rte_branch_prediction.h
> > > > b/lib/eal/include/rte_branch_prediction.h
> > > > index 0256a9d..d9a0224 100644
> > > > --- a/lib/eal/include/rte_branch_prediction.h
> > > > +++ b/lib/eal/include/rte_branch_prediction.h
> > > > @@ -25,7 +25,11 @@
> > > > *
> > > > */
> > > > #ifndef likely
> > > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > > #define likely(x) __builtin_expect(!!(x), 1)
> > > > +#else
> > > > +#define likely(x) (x)
> > >
> > > This must be (!!(x)), because x may be non-Boolean, e.g. likely(n & 0x10),
> > and likely() must return Boolean (0 or 1).
> > >
> >
> > Will this really make a difference? Is there somewhere likely/unlikely
> > would be used where we would not get the same conversion to boolean than we
> > get using "!!" operator. [NOTE: Not saying we shouldn't put in the !!, just
> > wondering if there are actual cases where it affects the output?]
>
> I agree that it makes no difference the way it is typically used.
>
> But there are creative developers out there, so these macros definitely need the "!!" conversion to Boolean.
>
Sure.
> >
> > > > +#endif
> > > > #endif /* likely */
> > > >
> > > > /**
> > > > @@ -39,7 +43,11 @@
> > > > *
> > > > */
> > > > #ifndef unlikely
> > > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > > #define unlikely(x) __builtin_expect(!!(x), 0)
> > > > +#else
> > > > +#define unlikely(x) (x)
> > >
> > > This must also be (!!(x)), for the same reason as above.
> > >
> > > > +#endif
> > > > #endif /* unlikely */
> > > >
> > > > #ifdef __cplusplus
> > > > diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> > > > index 2f464e3..1bdaa2d 100644
> > > > --- a/lib/eal/include/rte_common.h
> > > > +++ b/lib/eal/include/rte_common.h
> > > > @@ -65,7 +65,11 @@
> > > > /**
> > > > * Force alignment
> > > > */
> > > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > > #define __rte_aligned(a) __attribute__((__aligned__(a)))
> > > > +#else
> > > > +#define __rte_aligned(a)
> > > > +#endif
> > >
> > > It should be reviewed that __rte_aligned() is only used for optimization
> > purposes, and is not required for DPDK to function properly.
> > >
> >
> > Good point.
> >
> > If we look across all of DPDK, things will likely break, as we are relying
> > on alignment in various places to use the aligned versions of instructions.
> > For example _mm256_load_si256() vs _mm256_loadu_si256() in our x86
> > vectorized driver code. A "git grep _load_si" shows quite a few aligned
> > vector load instructions used in our codebase. These will fault and cause a
> > crash if the data is not properly aligned. [I suspect that there are similar
> > restrictions on other architectures too, just not familiar with their
> > intrinsics to check.]
>
> Another thing that has been annoying me with the use of vector instructions:
>
> Vector instructions are often used in a way where they cast away the type they are working on, so if that type is modified (e.g. a field is moved), the code will happily build, but fail at runtime.
>
> When casting away the type for vector instructions, _Static_assert or BUILD_BUG_ON should be used to verify the assumptions about the cast away type. Such a practice might catch some of the places where the missing alignment (and missing structure packing) would fail.
>
Agreed. And, in fairness, this is sometimes done in our code, e.g. [1], but
should probably be more widely done. It's something we should try and catch
in reviews of vector code, as it also helps document what exactly we are
doing and why.
/Bruce
[1] http://git.dpdk.org/dpdk/tree/drivers/net/i40e/i40e_rxtx_vec_avx2.c#n183
On Fri, Apr 14, 2023 at 08:45:17AM +0200, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Thursday, 13 April 2023 23.26
> >
> > For now expand a lot of common rte macros empty. The catch here is we
> > need to test that most of the macros do what they should but at the same
> > time they are blocking work needed to bootstrap of the unit tests.
> >
> > Later we will return and provide (where possible) expansions that work
> > correctly for msvc and where not possible provide some alternate macros
> > to achieve the same outcome.
> >
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> > lib/eal/include/rte_branch_prediction.h | 8 ++++++
> > lib/eal/include/rte_common.h | 45
> > +++++++++++++++++++++++++++++++++
> > lib/eal/include/rte_compat.h | 20 +++++++++++++++
> > 3 files changed, 73 insertions(+)
> >
> > diff --git a/lib/eal/include/rte_branch_prediction.h
> > b/lib/eal/include/rte_branch_prediction.h
> > index 0256a9d..d9a0224 100644
> > --- a/lib/eal/include/rte_branch_prediction.h
> > +++ b/lib/eal/include/rte_branch_prediction.h
> > @@ -25,7 +25,11 @@
> > *
> > */
> > #ifndef likely
> > +#ifndef RTE_TOOLCHAIN_MSVC
> > #define likely(x) __builtin_expect(!!(x), 1)
> > +#else
> > +#define likely(x) (x)
>
> This must be (!!(x)), because x may be non-Boolean, e.g. likely(n & 0x10), and likely() must return Boolean (0 or 1).
yes, you're right. will fix.
>
> > +#endif
> > #endif /* likely */
> >
> > /**
> > @@ -39,7 +43,11 @@
> > *
> > */
> > #ifndef unlikely
> > +#ifndef RTE_TOOLCHAIN_MSVC
> > #define unlikely(x) __builtin_expect(!!(x), 0)
> > +#else
> > +#define unlikely(x) (x)
>
> This must also be (!!(x)), for the same reason as above.
ack
>
> > +#endif
> > #endif /* unlikely */
> >
> > #ifdef __cplusplus
> > diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> > index 2f464e3..1bdaa2d 100644
> > --- a/lib/eal/include/rte_common.h
> > +++ b/lib/eal/include/rte_common.h
> > @@ -65,7 +65,11 @@
> > /**
> > * Force alignment
> > */
> > +#ifndef RTE_TOOLCHAIN_MSVC
> > #define __rte_aligned(a) __attribute__((__aligned__(a)))
> > +#else
> > +#define __rte_aligned(a)
> > +#endif
>
> It should be reviewed that __rte_aligned() is only used for optimization purposes, and is not required for DPDK to function properly.
so to expand on what i have in mind (and explain why i leave it expanded
empty for now)
while msvc has a __declspec for align there is a mismatch between
where gcc and msvc want it placed to control alignment of objects.
msvc support won't be functional in 23.07 because of atomics. so once
we reach the 23.11 cycle (where we can merge c11 changes) it means we
can also use standard _Alignas which can accomplish the same thing
but portably.
full disclosure the catch is i still have to properly locate the <thing>
that does the alignment and some small questions about the expansion and
use of the existing macro.
on the subject of DPDK requiring proper alignment, you're right it
is generally for performance but also for pre-c11 atomics.
one question i have been asking myself is would the community see value
in more compile time assertions / testing of the size and alignment of
structures and offset of structure fields? we have a few key
RTE_BUILD_BUG_ON() assertions but i've discovered they don't offer
comprehensive protection.
>
> >
> > #ifdef RTE_ARCH_STRICT_ALIGN
> > typedef uint64_t unaligned_uint64_t __rte_aligned(1);
> > @@ -80,16 +84,29 @@
> > /**
> > * Force a structure to be packed
> > */
> > +#ifndef RTE_TOOLCHAIN_MSVC
> > #define __rte_packed __attribute__((__packed__))
> > +#else
> > +#define __rte_packed
> > +#endif
>
> Similar comment as for __rte_aligned(); however, I consider it more likely that structure packing is a functional requirement, and not just used for optimization. Based on my experience, it may be used for packing network structures; perhaps not in DPDK itself but maybe in DPDK applications.
so interestingly i've discovered this is kind of a mess and as you note
some places we can't just "fix" it for abi compatibility reasons.
in some instances the packing is being applied to structures where it is
essentially a noop. i.e. natural alignment gets you the same thing so it
is superfluous.
in some instances the packing is being applied to structures that are
private and it appears to be completely unnecessary e.g. some structure
that isn't nested into something else and sizeof() or offsetof() fields
don't matter in the context of their use.
in some instances it is completely necessary usually when type punning
buffers containing network framing etc...
unfortunately the standard doesn't offer me an out here as there is an
issue of placement of the pragma/attributes that do the packing.
for places it isn't needed it, whatever i just expand empty. for places
it is superfluous again because msvc has no stable abi (we're not
established yet) again i just expand empty. finally for the places where
it is needed i'll probably need to expand conditionally but i think the
instances are far fewer than current use.
>
> The same risk applies to __rte_aligned(), but with lower probability.
so that's the long winded story of why they are both expanded empty for
now for msvc. but when the time comes i want to submit patch series that
focus on each specifically to generate robust discussion.
ty
> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Friday, 14 April 2023 19.02
>
> On Fri, Apr 14, 2023 at 08:45:17AM +0200, Morten Brørup wrote:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Thursday, 13 April 2023 23.26
> > >
> > > For now expand a lot of common rte macros empty. The catch here is
> we
> > > need to test that most of the macros do what they should but at the
> same
> > > time they are blocking work needed to bootstrap of the unit tests.
> > >
> > > Later we will return and provide (where possible) expansions that
> work
> > > correctly for msvc and where not possible provide some alternate
> macros
> > > to achieve the same outcome.
> > >
> > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
[...]
> > > /**
> > > * Force alignment
> > > */
> > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > #define __rte_aligned(a) __attribute__((__aligned__(a)))
> > > +#else
> > > +#define __rte_aligned(a)
> > > +#endif
> >
> > It should be reviewed that __rte_aligned() is only used for
> optimization purposes, and is not required for DPDK to function
> properly.
>
> so to expand on what i have in mind (and explain why i leave it expanded
> empty for now)
>
> while msvc has a __declspec for align there is a mismatch between
> where gcc and msvc want it placed to control alignment of objects.
>
> msvc support won't be functional in 23.07 because of atomics. so once
> we reach the 23.11 cycle (where we can merge c11 changes) it means we
> can also use standard _Alignas which can accomplish the same thing
> but portably.
That (C11 standard _Alignas) should be the roadmap for solving the alignment requirements.
This should be a general principle for DPDK... if the C standard offers something, don't reinvent our own. And as a consequence of the upgrade to C11, we should deprecate all our own now-obsolete substitutes for these.
>
> full disclosure the catch is i still have to properly locate the <thing>
> that does the alignment and some small questions about the expansion and
> use of the existing macro.
>
> on the subject of DPDK requiring proper alignment, you're right it
> is generally for performance but also for pre-c11 atomics.
>
> one question i have been asking myself is would the community see value
> in more compile time assertions / testing of the size and alignment of
> structures and offset of structure fields? we have a few key
> RTE_BUILD_BUG_ON() assertions but i've discovered they don't offer
> comprehensive protection.
Absolutely. Catching bugs at build time is much better than any alternative!
> > > /**
> > > * Force a structure to be packed
> > > */
> > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > #define __rte_packed __attribute__((__packed__))
> > > +#else
> > > +#define __rte_packed
> > > +#endif
> >
> > Similar comment as for __rte_aligned(); however, I consider it more
> likely that structure packing is a functional requirement, and not just
> used for optimization. Based on my experience, it may be used for
> packing network structures; perhaps not in DPDK itself but maybe in DPDK
> applications.
>
> so interestingly i've discovered this is kind of a mess and as you note
> some places we can't just "fix" it for abi compatibility reasons.
>
> in some instances the packing is being applied to structures where it is
> essentially a noop. i.e. natural alignment gets you the same thing so it
> is superfluous.
>
> in some instances the packing is being applied to structures that are
> private and it appears to be completely unnecessary e.g. some structure
> that isn't nested into something else and sizeof() or offsetof() fields
> don't matter in the context of their use.
>
> in some instances it is completely necessary usually when type punning
> buffers containing network framing etc...
>
> unfortunately the standard doesn't offer me an out here as there is an
> issue of placement of the pragma/attributes that do the packing.
>
> for places it isn't needed it, whatever i just expand empty. for places
> it is superfluous again because msvc has no stable abi (we're not
> established yet) again i just expand empty. finally for the places where
> it is needed i'll probably need to expand conditionally but i think the
> instances are far fewer than current use.
Optimally, we will have a common macro (or other solution) to support both GCC/CLANG and MSVC to replace or supplement __rte_packed. However, the cost of this may be an API break if we replace __rte_packed.
>
> >
> > The same risk applies to __rte_aligned(), but with lower probability.
>
> so that's the long winded story of why they are both expanded empty for
> now for msvc. but when the time comes i want to submit patch series that
> focus on each specifically to generate robust discussion.
Sounds like the right path to take.
Now, I'm thinking ahead here...
We should be prepared to accept a major ABI/API break at one point in time, to replace our home-grown macros with C11 standard solutions and to fully support MSVC. This is not happening anytime soon, but the Techboard should acknowledge that this is going to happen (with an unspecified release), so it can be formally announced. The sooner it is announced, the more time developers will have to prepare for it.
All the details do not need to be known at the time of the announcement; they can be added along the way, based on the discussions from your future patches.
>
> ty
On Sat, Apr 15, 2023 at 09:16:21AM +0200, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Friday, 14 April 2023 19.02
> >
> > On Fri, Apr 14, 2023 at 08:45:17AM +0200, Morten Brørup wrote:
> > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > > Sent: Thursday, 13 April 2023 23.26
> > > >
> > > > For now expand a lot of common rte macros empty. The catch here is
> > we
> > > > need to test that most of the macros do what they should but at the
> > same
> > > > time they are blocking work needed to bootstrap of the unit tests.
> > > >
> > > > Later we will return and provide (where possible) expansions that
> > work
> > > > correctly for msvc and where not possible provide some alternate
> > macros
> > > > to achieve the same outcome.
> > > >
> > > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>
> [...]
>
> > > > /**
> > > > * Force alignment
> > > > */
> > > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > > #define __rte_aligned(a) __attribute__((__aligned__(a)))
> > > > +#else
> > > > +#define __rte_aligned(a)
> > > > +#endif
> > >
> > > It should be reviewed that __rte_aligned() is only used for
> > optimization purposes, and is not required for DPDK to function
> > properly.
> >
> > so to expand on what i have in mind (and explain why i leave it expanded
> > empty for now)
> >
> > while msvc has a __declspec for align there is a mismatch between
> > where gcc and msvc want it placed to control alignment of objects.
> >
> > msvc support won't be functional in 23.07 because of atomics. so once
> > we reach the 23.11 cycle (where we can merge c11 changes) it means we
> > can also use standard _Alignas which can accomplish the same thing
> > but portably.
>
> That (C11 standard _Alignas) should be the roadmap for solving the alignment requirements.
>
> This should be a general principle for DPDK... if the C standard offers something, don't reinvent our own. And as a consequence of the upgrade to C11, we should deprecate all our own now-obsolete substitutes for these.
>
> >
> > full disclosure the catch is i still have to properly locate the <thing>
> > that does the alignment and some small questions about the expansion and
> > use of the existing macro.
> >
> > on the subject of DPDK requiring proper alignment, you're right it
> > is generally for performance but also for pre-c11 atomics.
> >
> > one question i have been asking myself is would the community see value
> > in more compile time assertions / testing of the size and alignment of
> > structures and offset of structure fields? we have a few key
> > RTE_BUILD_BUG_ON() assertions but i've discovered they don't offer
> > comprehensive protection.
>
> Absolutely. Catching bugs at build time is much better than any alternative!
that's handy feedback. i am now encouraged to include more compile time
checks in advance of or along with changes related to structure abi.
follow on question, once we do get to use c11 would something like
_Static_assert be preferable over RTE_BUILD_BUG_ON? structures sensitive
to layout could be co-located with the asserts right at the point of
definition. or is there something extra RTE_BUILD_BUG_ON gives us?
>
> > > > /**
> > > > * Force a structure to be packed
> > > > */
> > > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > > #define __rte_packed __attribute__((__packed__))
> > > > +#else
> > > > +#define __rte_packed
> > > > +#endif
> > >
> > > Similar comment as for __rte_aligned(); however, I consider it more
> > likely that structure packing is a functional requirement, and not just
> > used for optimization. Based on my experience, it may be used for
> > packing network structures; perhaps not in DPDK itself but maybe in DPDK
> > applications.
> >
> > so interestingly i've discovered this is kind of a mess and as you note
> > some places we can't just "fix" it for abi compatibility reasons.
> >
> > in some instances the packing is being applied to structures where it is
> > essentially a noop. i.e. natural alignment gets you the same thing so it
> > is superfluous.
> >
> > in some instances the packing is being applied to structures that are
> > private and it appears to be completely unnecessary e.g. some structure
> > that isn't nested into something else and sizeof() or offsetof() fields
> > don't matter in the context of their use.
> >
> > in some instances it is completely necessary usually when type punning
> > buffers containing network framing etc...
> >
> > unfortunately the standard doesn't offer me an out here as there is an
> > issue of placement of the pragma/attributes that do the packing.
> >
> > for places it isn't needed it, whatever i just expand empty. for places
> > it is superfluous again because msvc has no stable abi (we're not
> > established yet) again i just expand empty. finally for the places where
> > it is needed i'll probably need to expand conditionally but i think the
> > instances are far fewer than current use.
>
> Optimally, we will have a common macro (or other solution) to support both GCC/CLANG and MSVC to replace or supplement __rte_packed. However, the cost of this may be an API break if we replace __rte_packed.
>
> >
> > >
> > > The same risk applies to __rte_aligned(), but with lower probability.
> >
> > so that's the long winded story of why they are both expanded empty for
> > now for msvc. but when the time comes i want to submit patch series that
> > focus on each specifically to generate robust discussion.
>
> Sounds like the right path to take.
>
> Now, I'm thinking ahead here...
>
> We should be prepared to accept a major ABI/API break at one point in time, to replace our home-grown macros with C11 standard solutions and to fully support MSVC. This is not happening anytime soon, but the Techboard should acknowledge that this is going to happen (with an unspecified release), so it can be formally announced. The sooner it is announced, the more time developers will have to prepare for it.
so, just to avoid any confusion i want to make it clear that i am not
planning to submit changes that would change abi as a part of supporting
msvc (aside from changing to standard atomics which we agreed on).
in general there are some cleanups we could make in the area of code
maintainability and portability and we may want to discuss the
advantages or disadvantages of making those changes. but i think those
changes are a topic unrelated to windows or msvc specifically.
>
> All the details do not need to be known at the time of the announcement; they can be added along the way, based on the discussions from your future patches.
>
> >
> > ty
> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Saturday, 15 April 2023 22.52
>
> On Sat, Apr 15, 2023 at 09:16:21AM +0200, Morten Brørup wrote:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Friday, 14 April 2023 19.02
> > >
> > > On Fri, Apr 14, 2023 at 08:45:17AM +0200, Morten Brørup wrote:
> > > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > > > Sent: Thursday, 13 April 2023 23.26
> > > > >
> > > > > For now expand a lot of common rte macros empty. The catch here
> is
> > > we
> > > > > need to test that most of the macros do what they should but at
> the
> > > same
> > > > > time they are blocking work needed to bootstrap of the unit
> tests.
> > > > >
> > > > > Later we will return and provide (where possible) expansions
> that
> > > work
> > > > > correctly for msvc and where not possible provide some alternate
> > > macros
> > > > > to achieve the same outcome.
> > > > >
> > > > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> >
> > [...]
> >
> > > > > /**
> > > > > * Force alignment
> > > > > */
> > > > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > > > #define __rte_aligned(a) __attribute__((__aligned__(a)))
> > > > > +#else
> > > > > +#define __rte_aligned(a)
> > > > > +#endif
> > > >
> > > > It should be reviewed that __rte_aligned() is only used for
> > > optimization purposes, and is not required for DPDK to function
> > > properly.
> > >
> > > so to expand on what i have in mind (and explain why i leave it
> expanded
> > > empty for now)
> > >
> > > while msvc has a __declspec for align there is a mismatch between
> > > where gcc and msvc want it placed to control alignment of objects.
> > >
> > > msvc support won't be functional in 23.07 because of atomics. so
> once
> > > we reach the 23.11 cycle (where we can merge c11 changes) it means
> we
> > > can also use standard _Alignas which can accomplish the same thing
> > > but portably.
> >
> > That (C11 standard _Alignas) should be the roadmap for solving the
> alignment requirements.
> >
> > This should be a general principle for DPDK... if the C standard
> offers something, don't reinvent our own. And as a consequence of the
> upgrade to C11, we should deprecate all our own now-obsolete substitutes
> for these.
> >
> > >
> > > full disclosure the catch is i still have to properly locate the
> <thing>
> > > that does the alignment and some small questions about the expansion
> and
> > > use of the existing macro.
> > >
> > > on the subject of DPDK requiring proper alignment, you're right it
> > > is generally for performance but also for pre-c11 atomics.
> > >
> > > one question i have been asking myself is would the community see
> value
> > > in more compile time assertions / testing of the size and alignment
> of
> > > structures and offset of structure fields? we have a few key
> > > RTE_BUILD_BUG_ON() assertions but i've discovered they don't offer
> > > comprehensive protection.
> >
> > Absolutely. Catching bugs at build time is much better than any
> alternative!
>
> that's handy feedback. i am now encouraged to include more compile time
> checks in advance of or along with changes related to structure abi.
Sounds good.
Disclaimer: "Absolutely" was my personal response. But I seriously doubt that anyone in the DPDK community would object to more build time checks. Stability and code quality carries a lot of weight in DPDK community discussions.
With that said, please expect that maintainers might want you to split your patches, so the additional checks are separated from the MSVC changes.
> follow on question, once we do get to use c11 would something like
> _Static_assert be preferable over RTE_BUILD_BUG_ON? structures sensitive
> to layout could be co-located with the asserts right at the point of
> definition. or is there something extra RTE_BUILD_BUG_ON gives us?
People may have different opinions on RTE_BUILD_BUG_ON vs. _Static_assert or static_assert.
Personally, I prefer static_assert/_Static_assert. It also has the advantage that it can be used in the global scope, directly following the structure definitions (like you mention), whereas RTE_BUILD_BUG_ON must be inside a code block (which can probably be worked around by making a dummy static inline function only containing the RTE_BUILD_BUG_ON).
And in the spirit of my proposal of not using home-grown macros as alternatives to what the C standard provides, I think we should deprecate and get rid of RTE_BUILD_BUG_ON in favor of static_assert/_Static_assert introduced by the C11 standard. (My personal opinion, no such principle decision has been made!)
If we want to keep RTE_BUILD_BUG_ON for some reason, we could change its implementation to use static_assert/_Static_assert instead of creating an invalid pointer to make the compilation fail.
>
> >
> > > > > /**
> > > > > * Force a structure to be packed
> > > > > */
> > > > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > > > #define __rte_packed __attribute__((__packed__))
> > > > > +#else
> > > > > +#define __rte_packed
> > > > > +#endif
> > > >
> > > > Similar comment as for __rte_aligned(); however, I consider it
> more
> > > likely that structure packing is a functional requirement, and not
> just
> > > used for optimization. Based on my experience, it may be used for
> > > packing network structures; perhaps not in DPDK itself but maybe in
> DPDK
> > > applications.
> > >
> > > so interestingly i've discovered this is kind of a mess and as you
> note
> > > some places we can't just "fix" it for abi compatibility reasons.
> > >
> > > in some instances the packing is being applied to structures where
> it is
> > > essentially a noop. i.e. natural alignment gets you the same thing
> so it
> > > is superfluous.
> > >
> > > in some instances the packing is being applied to structures that
> are
> > > private and it appears to be completely unnecessary e.g. some
> structure
> > > that isn't nested into something else and sizeof() or offsetof()
> fields
> > > don't matter in the context of their use.
> > >
> > > in some instances it is completely necessary usually when type
> punning
> > > buffers containing network framing etc...
> > >
> > > unfortunately the standard doesn't offer me an out here as there is
> an
> > > issue of placement of the pragma/attributes that do the packing.
> > >
> > > for places it isn't needed it, whatever i just expand empty. for
> places
> > > it is superfluous again because msvc has no stable abi (we're not
> > > established yet) again i just expand empty. finally for the places
> where
> > > it is needed i'll probably need to expand conditionally but i think
> the
> > > instances are far fewer than current use.
> >
> > Optimally, we will have a common macro (or other solution) to support
> both GCC/CLANG and MSVC to replace or supplement __rte_packed. However,
> the cost of this may be an API break if we replace __rte_packed.
> >
> > >
> > > >
> > > > The same risk applies to __rte_aligned(), but with lower
> probability.
> > >
> > > so that's the long winded story of why they are both expanded empty
> for
> > > now for msvc. but when the time comes i want to submit patch series
> that
> > > focus on each specifically to generate robust discussion.
> >
> > Sounds like the right path to take.
> >
> > Now, I'm thinking ahead here...
> >
> > We should be prepared to accept a major ABI/API break at one point in
> time, to replace our home-grown macros with C11 standard solutions and
> to fully support MSVC. This is not happening anytime soon, but the
> Techboard should acknowledge that this is going to happen (with an
> unspecified release), so it can be formally announced. The sooner it is
> announced, the more time developers will have to prepare for it.
>
> so, just to avoid any confusion i want to make it clear that i am not
> planning to submit changes that would change abi as a part of supporting
> msvc (aside from changing to standard atomics which we agreed on).
Thank you for clarifying.
>
> in general there are some cleanups we could make in the area of code
> maintainability and portability and we may want to discuss the
> advantages or disadvantages of making those changes. but i think those
> changes are a topic unrelated to windows or msvc specifically.
This was the point I was trying to make, when I proposed accepting a major ABI/API break. Sorry about my unclear wording.
If we collect a wish list of breaking changes, I would personally prefer a "big bang" major ABI/API break, rather than a series of incremental API/ABI breaks over multiple DPDK release. In this regard, we could mix both changes driven by the migration to pure C11 (e.g. getting rid of now-obsolete macros, such as RTE_BUILD_BUG_ON, and compiler intrinsics, such as __rte_aligned) and MSVC portability changes (e.g. an improved macro to support structure packing).
>
> >
> > All the details do not need to be known at the time of the
> announcement; they can be added along the way, based on the discussions
> from your future patches.
>
> >
> > >
> > > ty
On Sun, 16 Apr 2023 00:41:54 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:
> > > > > > /**
> > > > > > * Force a structure to be packed
> > > > > > */
> > > > > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > > > > #define __rte_packed __attribute__((__packed__))
> > > > > > +#else
> > > > > > +#define __rte_packed
> > > > > > +#endif
Could there be cases this gets used for protocol headers or interacting with
HW memory map, And if not packed then the code will not function correctly?
On Sat, Apr 15, 2023 at 03:52:27PM -0700, Stephen Hemminger wrote:
> On Sun, 16 Apr 2023 00:41:54 +0200
> Morten Brørup <mb@smartsharesystems.com> wrote:
>
> > > > > > > /**
> > > > > > > * Force a structure to be packed
> > > > > > > */
> > > > > > > +#ifndef RTE_TOOLCHAIN_MSVC
> > > > > > > #define __rte_packed __attribute__((__packed__))
> > > > > > > +#else
> > > > > > > +#define __rte_packed
> > > > > > > +#endif
>
> Could there be cases this gets used for protocol headers or interacting with
> HW memory map, And if not packed then the code will not function correctly?
yes, that's one of the valid / correct use cases and it can't be avoided
if the structs are nested in recursively composed layout.
there are a few instances where we don't need compiler generated
static-stride. that is we don't need to force packing to get sizeof(T)
== offsetof(T.lastfield) + sizeof(T.lastfield).
anyway, this is more of an evaluate on a case by case basis for
candidates that aren't needed.
ty
@@ -25,7 +25,11 @@
*
*/
#ifndef likely
+#ifndef RTE_TOOLCHAIN_MSVC
#define likely(x) __builtin_expect(!!(x), 1)
+#else
+#define likely(x) (x)
+#endif
#endif /* likely */
/**
@@ -39,7 +43,11 @@
*
*/
#ifndef unlikely
+#ifndef RTE_TOOLCHAIN_MSVC
#define unlikely(x) __builtin_expect(!!(x), 0)
+#else
+#define unlikely(x) (x)
+#endif
#endif /* unlikely */
#ifdef __cplusplus
@@ -65,7 +65,11 @@
/**
* Force alignment
*/
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_aligned(a) __attribute__((__aligned__(a)))
+#else
+#define __rte_aligned(a)
+#endif
#ifdef RTE_ARCH_STRICT_ALIGN
typedef uint64_t unaligned_uint64_t __rte_aligned(1);
@@ -80,16 +84,29 @@
/**
* Force a structure to be packed
*/
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_packed __attribute__((__packed__))
+#else
+#define __rte_packed
+#endif
/**
* Macro to mark a type that is not subject to type-based aliasing rules
*/
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_may_alias __attribute__((__may_alias__))
+#else
+#define __rte_may_alias
+#endif
/******* Macro to mark functions and fields scheduled for removal *****/
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_deprecated __attribute__((__deprecated__))
#define __rte_deprecated_msg(msg) __attribute__((__deprecated__(msg)))
+#else
+#define __rte_deprecated
+#define __rte_deprecated_msg(msg)
+#endif
/**
* Macro to mark macros and defines scheduled for removal
@@ -110,14 +127,22 @@
/**
* Force symbol to be generated even if it appears to be unused.
*/
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_used __attribute__((used))
+#else
+#define __rte_used
+#endif
/*********** Macros to eliminate unused variable warnings ********/
/**
* short definition to mark a function parameter unused
*/
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_unused __attribute__((__unused__))
+#else
+#define __rte_unused
+#endif
/**
* Mark pointer as restricted with regard to pointer aliasing.
@@ -141,6 +166,7 @@
* even if the underlying stdio implementation is ANSI-compliant,
* so this must be overridden.
*/
+#ifndef RTE_TOOLCHAIN_MSVC
#if RTE_CC_IS_GNU
#define __rte_format_printf(format_index, first_arg) \
__attribute__((format(gnu_printf, format_index, first_arg)))
@@ -148,6 +174,9 @@
#define __rte_format_printf(format_index, first_arg) \
__attribute__((format(printf, format_index, first_arg)))
#endif
+#else
+#define __rte_format_printf(format_index, first_arg)
+#endif
/**
* Tells compiler that the function returns a value that points to
@@ -222,7 +251,11 @@ static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
/**
* Hint never returning function
*/
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_noreturn __attribute__((noreturn))
+#else
+#define __rte_noreturn
+#endif
/**
* Issue a warning in case the function's return value is ignored.
@@ -247,12 +280,20 @@ static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
* }
* @endcode
*/
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_warn_unused_result __attribute__((warn_unused_result))
+#else
+#define __rte_warn_unused_result
+#endif
/**
* Force a function to be inlined
*/
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_always_inline inline __attribute__((always_inline))
+#else
+#define __rte_always_inline
+#endif
/**
* Force a function to be noinlined
@@ -437,7 +478,11 @@ static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
#define RTE_CACHE_LINE_MIN_SIZE 64
/** Force alignment to cache line. */
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_cache_aligned __rte_aligned(RTE_CACHE_LINE_SIZE)
+#else
+#define __rte_cache_aligned
+#endif
/** Force minimum cache line alignment. */
#define __rte_cache_min_aligned __rte_aligned(RTE_CACHE_LINE_MIN_SIZE)
@@ -12,14 +12,22 @@
#ifndef ALLOW_EXPERIMENTAL_API
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_experimental \
__attribute__((deprecated("Symbol is not yet part of stable ABI"), \
section(".text.experimental")))
+#else
+#define __rte_experimental
+#endif
#else
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_experimental \
__attribute__((section(".text.experimental")))
+#else
+#define __rte_experimental
+#endif
#endif
@@ -30,23 +38,35 @@
#if !defined ALLOW_INTERNAL_API && __has_attribute(error) /* For GCC */
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_internal \
__attribute__((error("Symbol is not public ABI"), \
section(".text.internal")))
+#else
+#define __rte_internal
+#endif
#elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_internal \
_Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
__attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
section(".text.internal"))) \
_Pragma("GCC diagnostic pop")
+#else
+#define __rte_internal
+#endif
#else
+#ifndef RTE_TOOLCHAIN_MSVC
#define __rte_internal \
__attribute__((section(".text.internal")))
+#else
+#define __rte_internal
+#endif
#endif