[PATCH v2 2/5] lib/eal: add portable version of __builtin_add_overflow

[Andre Muezerie]

On Mon, Jan 06, 2025 at 12:58:44PM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson at intel.com]
> > Sent: Monday, 6 January 2025 12.34
> > 
> > On Mon, Jan 06, 2025 at 12:21:39PM +0100, Morten Brørup wrote:
> > > > From: Bruce Richardson [mailto:bruce.richardson at intel.com]
> > > > Sent: Monday, 6 January 2025 12.07
> > > >
> > > > On Fri, Jan 03, 2025 at 12:39:38PM -0800, Andre Muezerie wrote:
> > > > > __builtin_add_overflow is gcc specific. There's a need for a
> > portable
> > > > > version that can also be used with other compilers.
> > > > >
> > > > > This patch introduces rte_add_overflow.
> > > > >
> > > > > +/*
> > > > > + * Function that allows performing simple arithmetic operations
> > > > together with
> > > > > + * checking whether the operation overflowed.
> > > > > + * Example of usage:
> > > > > + *     uint8_t overflow;
> > > > > + *     uint16_t a, b, result;
> > > > > + *     a = 1;
> > > > > + *     b = 2;
> > > > > + *     overflow = rte_add_overflow(a, b, &result);
> > > > > + */
> > > > > +#ifdef RTE_TOOLCHAIN_MSVC
> > > > > +#define rte_add_overflow(a, b, res) _Generic((a), \
> > > > > +	uint8_t : _addcarry_u8, \
> > > > > +	uint16_t : _addcarry_u16, \
> > > > > +	uint32_t : _addcarry_u32, \
> > > > > +	uint64_t : _addcarry_u64)(0, a, b, res)
> > > > > +#else
> > > > > +#define rte_add_overflow(a, b, res) _Generic((a), \
> > > > > +	uint8_t : __builtin_add_overflow, \
> > > > > +	uint16_t : __builtin_add_overflow, \
> > > > > +	uint32_t : __builtin_add_overflow, \
> > > > > +	uint64_t : __builtin_add_overflow)(a, b, res)
> > > > > +#endif
> > > >
> > > > For the gcc version, can you just simplify to the one-line below?
> > > >
> > > > #define rte_add_overflow __builtin_add_overflow
> > >
> > > Yes, but then GCC compilation would not fail if "a" has some other
> > type than the four types explicitly supported.
> > > I prefer keeping the method used this v2 patch.
> > >
> > Is that really a problem? Should our DPDK macro not support all the
> > types
> > that the GCC builtin supports?
> 
> The DPDK macro should support all the types that both MSVC and GCC supports.
> Using _Generic() for GCC is an improvement for the CI to catch MSVC incompatible code when building for GCC.
> 
> Only these four unsigned types are supported by the x86_64 intrinsics.
> I don't think we need support for more types; but if the need should arise, it can be added later.

Great. Let me know if any further action is needed on this patch.