RE: [PATCH v5 0/3] add portable version of __builtin_add_overflow

["Morten Brørup" <mb@smartsharesystems.com>]

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Wednesday, 4 June 2025 12.08
> 
> On Fri, Mar 14, 2025 at 3:34 PM Andre Muezerie
> <andremue@linux.microsoft.com> wrote:
> >
> > __builtin_add_overflow is gcc specific. There's a need for a portable
> > version that can also be used with other compilers.
> >
> > v5:
> > - Combined patches 1 with 5 and 2 with 3.
> >
> > v4:
> > - Added define in ice_osdep.h to use portable version of
> >   __builtin_add_overflow when using MSVC.
> > - Undid all changes from drivers/net/intel/ice/base/ice_nvm.c.
> >
> > v3:
> > - Rebase on top of latest main.
> >
> > Andre Muezerie (3):
> >   eal: add portable version of __builtin_add_overflow
> >   net/intel: use portable version of __builtin_add_overflow
> >   app/test: add tests for portable version of __builtin_add_overflow
> >
> >  MAINTAINERS                            |   1 +
> >  app/test/meson.build                   |   1 +
> >  app/test/test_math.c                   | 170
> +++++++++++++++++++++++++
> >  doc/api/doxy-api-index.md              |   1 +
> >  drivers/net/intel/ice/base/ice_osdep.h |   5 +
> >  lib/eal/include/meson.build            |   1 +
> >  lib/eal/include/rte_math.h             |  46 +++++++
> >  7 files changed, 225 insertions(+)
> >  create mode 100644 app/test/test_math.c
> >  create mode 100644 lib/eal/include/rte_math.h
> 
> I am not a fan of adding such public API, an internal API would be
> enough.
> Do you plan to add more helpers for math operations?
> 
> For the current helper, the only user is a driver (base code).
> Can't we just wrap a __builtin_add_overflow (under #ifdef msvc) in the
> osdep.h header?

We already have public APIs for bit operations in rte_bitops.h.
This math API follows the same principle; and math operations - just like bit operations - might be useful for DPDK applications, so let's keep it public.

The only issue I have with these (incl. the bit operations) are that they are in the EAL library, although they have absolutely nothing to do with hardware or O/S abstraction, so they really should be in a "utils" library.
But that's another story, so let's not burden Andre with that.