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

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Jan 06, 2025 at 12:21:39PM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@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?

/Bruce