* [PATCH 0/5] add portable version of __builtin_add_overflow @ 2025-01-02 22:32 Andre Muezerie 2025-01-02 22:32 ` [PATCH 1/5] maintainers: " Andre Muezerie ` (6 more replies) 0 siblings, 7 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-02 22:32 UTC (permalink / raw) Cc: dev, Andre Muezerie __builtin_add_overflow is gcc specific. There's a need for a portable version that can also be used with other compilers. Andre Muezerie (5): maintainers: add portable version of __builtin_add_overflow lib/eal: add portable version of __builtin_add_overflow doc/api: add portable version of __builtin_add_overflow drivers/net: use portable version of __builtin_add_overflow app/test: add tests for portable versions of __builtin_add_overflow MAINTAINERS | 1 + app/test/meson.build | 1 + app/test/test_math.c | 125 +++++++++++++++++++++++++++++++++ doc/api/doxy-api-index.md | 1 + drivers/net/ice/base/ice_nvm.c | 9 ++- lib/eal/include/meson.build | 1 + lib/eal/include/rte_math.h | 42 +++++++++++ 7 files changed, 175 insertions(+), 5 deletions(-) create mode 100644 app/test/test_math.c create mode 100644 lib/eal/include/rte_math.h -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/5] maintainers: add portable version of __builtin_add_overflow 2025-01-02 22:32 [PATCH 0/5] add portable version of __builtin_add_overflow Andre Muezerie @ 2025-01-02 22:32 ` Andre Muezerie 2025-01-02 22:32 ` [PATCH 2/5] lib/eal: " Andre Muezerie ` (5 subsequent siblings) 6 siblings, 0 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-02 22:32 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev, Andre Muezerie __builtin_add_overflow is gcc specific. There's a need for a portable version that can also be used with other compilers. Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com> --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 60bdcce543..4b03b6752e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -180,6 +180,7 @@ F: app/test/test_devargs.c F: app/test/test_eal* F: app/test/test_errno.c F: app/test/test_lcores.c +F: app/test/test_math.c F: app/test/test_memcpy* F: app/test/test_per_lcore.c F: app/test/test_pflock.c -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] lib/eal: add portable version of __builtin_add_overflow 2025-01-02 22:32 [PATCH 0/5] add portable version of __builtin_add_overflow Andre Muezerie 2025-01-02 22:32 ` [PATCH 1/5] maintainers: " Andre Muezerie @ 2025-01-02 22:32 ` Andre Muezerie 2025-01-03 8:19 ` Morten Brørup 2025-01-02 22:32 ` [PATCH 3/5] doc/api: " Andre Muezerie ` (4 subsequent siblings) 6 siblings, 1 reply; 21+ messages in thread From: Andre Muezerie @ 2025-01-02 22:32 UTC (permalink / raw) To: Tyler Retzlaff; +Cc: dev, Andre Muezerie __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_u8, __rte_add_overflow_u16 and __rte_add_overflow_u32. Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com> --- lib/eal/include/meson.build | 1 + lib/eal/include/rte_math.h | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 lib/eal/include/rte_math.h diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build index d903577caa..041a4105b5 100644 --- a/lib/eal/include/meson.build +++ b/lib/eal/include/meson.build @@ -31,6 +31,7 @@ headers += files( 'rte_lcore_var.h', 'rte_lock_annotations.h', 'rte_malloc.h', + 'rte_math.h', 'rte_mcslock.h', 'rte_memory.h', 'rte_memzone.h', diff --git a/lib/eal/include/rte_math.h b/lib/eal/include/rte_math.h new file mode 100644 index 0000000000..df2f3d4d34 --- /dev/null +++ b/lib/eal/include/rte_math.h @@ -0,0 +1,42 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2025 Microsoft Corporation + */ + +#ifndef _RTE_MATH_H_ +#define _RTE_MATH_H_ + +/** + * @file + * + * Math function definitions for DPDK. + */ + +#ifdef __cplusplus +extern "C" { +#endif + +/* + * Functions that allow performing simple arithmetic operations together with + * checking whether the operations overflowed. + * Example of usage: + * uint8_t overflow; + * uint16_t a, b, result; + * a = 1; + * b = 2; + * overflow = __rte_add_overflow_u16(a, b, &result); + */ +#ifdef RTE_TOOLCHAIN_MSVC +#define __rte_add_overflow_u8(a, b, res) _addcarry_u8(0, a, b, res) +#define __rte_add_overflow_u16(a, b, res) _addcarry_u16(0, a, b, res) +#define __rte_add_overflow_u32(a, b, res) _addcarry_u32(0, a, b, res) +#else +#define __rte_add_overflow_u8(a, b, res) __builtin_add_overflow(a, b, res) +#define __rte_add_overflow_u16(a, b, res) __builtin_add_overflow(a, b, res) +#define __rte_add_overflow_u32(a, b, res) __builtin_add_overflow(a, b, res) +#endif + +#ifdef __cplusplus +} +#endif + +#endif -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 2/5] lib/eal: add portable version of __builtin_add_overflow 2025-01-02 22:32 ` [PATCH 2/5] lib/eal: " Andre Muezerie @ 2025-01-03 8:19 ` Morten Brørup 2025-01-03 20:38 ` Andre Muezerie 0 siblings, 1 reply; 21+ messages in thread From: Morten Brørup @ 2025-01-03 8:19 UTC (permalink / raw) To: Andre Muezerie, Tyler Retzlaff; +Cc: dev > From: Andre Muezerie [mailto:andremue@linux.microsoft.com] > Sent: Thursday, 2 January 2025 23.33 > > __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_u8, __rte_add_overflow_u16 > and __rte_add_overflow_u32. Instead of the proposed three type-specific macros, add one generic rte_add_overflow(a, b, res) macro, like rte_bit_test() [1] using "_Generic". [1]: https://elixir.bootlin.com/dpdk/v24.11.1/source/lib/eal/include/rte_bitops.h#L130 You may still need the type-specific macros as internal helpers for the MSVC implementation. Furthermore, a 64 bit variant might be useful. PS: DPDK naming convention typically uses just "8"/"16"/"32" postfix to the function name, not "_u8"/"_u16"/"_u32". ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] lib/eal: add portable version of __builtin_add_overflow 2025-01-03 8:19 ` Morten Brørup @ 2025-01-03 20:38 ` Andre Muezerie 0 siblings, 0 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-03 20:38 UTC (permalink / raw) To: Morten Brørup; +Cc: Tyler Retzlaff, dev On Fri, Jan 03, 2025 at 09:19:42AM +0100, Morten Brørup wrote: > > From: Andre Muezerie [mailto:andremue@linux.microsoft.com] > > Sent: Thursday, 2 January 2025 23.33 > > > > __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_u8, __rte_add_overflow_u16 > > and __rte_add_overflow_u32. > > Instead of the proposed three type-specific macros, add one generic rte_add_overflow(a, b, res) macro, like rte_bit_test() [1] using "_Generic". > > [1]: https://elixir.bootlin.com/dpdk/v24.11.1/source/lib/eal/include/rte_bitops.h#L130 > > You may still need the type-specific macros as internal helpers for the MSVC implementation. > > Furthermore, a 64 bit variant might be useful. > > PS: DPDK naming convention typically uses just "8"/"16"/"32" postfix to the function name, not "_u8"/"_u16"/"_u32". These are great suggestions. I'll incorporate them in the v2 of this series. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/5] doc/api: add portable version of __builtin_add_overflow 2025-01-02 22:32 [PATCH 0/5] add portable version of __builtin_add_overflow Andre Muezerie 2025-01-02 22:32 ` [PATCH 1/5] maintainers: " Andre Muezerie 2025-01-02 22:32 ` [PATCH 2/5] lib/eal: " Andre Muezerie @ 2025-01-02 22:32 ` Andre Muezerie 2025-01-02 22:32 ` [PATCH 4/5] drivers/net: use " Andre Muezerie ` (3 subsequent siblings) 6 siblings, 0 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-02 22:32 UTC (permalink / raw) Cc: dev, Andre Muezerie __builtin_add_overflow is gcc specific. There's a need for a portable version that can also be used with other compilers. Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com> --- doc/api/doxy-api-index.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md index f0193502bc..c95a86448d 100644 --- a/doc/api/doxy-api-index.md +++ b/doc/api/doxy-api-index.md @@ -226,6 +226,7 @@ The public API headers are grouped by topics: [checksum](@ref rte_cksum.h), [config file](@ref rte_cfgfile.h), [key/value args](@ref rte_kvargs.h), + [math](@ref rte_math.h), [argument parsing](@ref rte_argparse.h), [ptr_compress](@ref rte_ptr_compress.h), [string](@ref rte_string_fns.h), -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/5] drivers/net: use portable version of __builtin_add_overflow 2025-01-02 22:32 [PATCH 0/5] add portable version of __builtin_add_overflow Andre Muezerie ` (2 preceding siblings ...) 2025-01-02 22:32 ` [PATCH 3/5] doc/api: " Andre Muezerie @ 2025-01-02 22:32 ` Andre Muezerie 2025-01-02 22:32 ` [PATCH 5/5] app/test: add tests for portable versions " Andre Muezerie ` (2 subsequent siblings) 6 siblings, 0 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-02 22:32 UTC (permalink / raw) To: Bruce Richardson, Anatoly Burakov; +Cc: dev, Andre Muezerie __builtin_add_overflow is gcc specific. It should be replaced with a portable version that can also be used with other compilers. Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com> --- drivers/net/ice/base/ice_nvm.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/net/ice/base/ice_nvm.c b/drivers/net/ice/base/ice_nvm.c index 56c6c96a95..1fc64b502c 100644 --- a/drivers/net/ice/base/ice_nvm.c +++ b/drivers/net/ice/base/ice_nvm.c @@ -3,6 +3,7 @@ */ #include "ice_common.h" +#include <rte_math.h> #define GL_MNG_DEF_DEVID 0x000B611C @@ -469,8 +470,6 @@ int ice_read_sr_word(struct ice_hw *hw, u16 offset, u16 *data) return status; } -#define check_add_overflow __builtin_add_overflow - /** * ice_get_pfa_module_tlv - Reads sub module TLV from NVM PFA * @hw: pointer to hardware structure @@ -500,7 +499,7 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len, return status; } - if (check_add_overflow(pfa_ptr, (u16)(pfa_len - 1), &max_tlv)) { + if (__rte_add_overflow_u16(pfa_ptr, (u16)(pfa_len - 1), &max_tlv)) { ice_debug(hw, ICE_DBG_INIT, "PFA starts at offset %u. PFA length of %u caused 16-bit arithmetic overflow.\n", pfa_ptr, pfa_len); return ICE_ERR_INVAL_SIZE; @@ -541,8 +540,8 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len, return ICE_ERR_INVAL_SIZE; } - if (check_add_overflow(next_tlv, (u16)2, &next_tlv) || - check_add_overflow(next_tlv, tlv_len, &next_tlv)) { + if (__rte_add_overflow_u16(next_tlv, (u16)2, &next_tlv) || + __rte_add_overflow_u16(next_tlv, tlv_len, &next_tlv)) { ice_debug(hw, ICE_DBG_INIT, "TLV of type %u and length 0x%04x caused 16-bit arithmetic overflow. The PFA starts at 0x%04x and has length of 0x%04x\n", tlv_sub_module_type, tlv_len, pfa_ptr, pfa_len); return ICE_ERR_INVAL_SIZE; -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/5] app/test: add tests for portable versions of __builtin_add_overflow 2025-01-02 22:32 [PATCH 0/5] add portable version of __builtin_add_overflow Andre Muezerie ` (3 preceding siblings ...) 2025-01-02 22:32 ` [PATCH 4/5] drivers/net: use " Andre Muezerie @ 2025-01-02 22:32 ` Andre Muezerie 2025-01-02 23:51 ` [PATCH 0/5] add portable version " Stephen Hemminger 2025-01-03 20:39 ` [PATCH v2 " Andre Muezerie 6 siblings, 0 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-02 22:32 UTC (permalink / raw) Cc: dev, Andre Muezerie __builtin_add_overflow is gcc specific. There's a need for a portable version that can also be used with other compilers. This patch adds tests for these new portable functions, to confirm they behave the same way across different compilers. Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com> --- app/test/meson.build | 1 + app/test/test_math.c | 125 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 app/test/test_math.c diff --git a/app/test/meson.build b/app/test/meson.build index 22b3291fa6..ab23f8dc79 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -118,6 +118,7 @@ source_file_deps = { 'test_lpm_perf.c': ['net', 'lpm'], 'test_malloc.c': [], 'test_malloc_perf.c': [], + 'test_math.c': [], 'test_mbuf.c': ['net'], 'test_mcslock.c': [], 'test_member.c': ['member', 'net'], diff --git a/app/test/test_math.c b/app/test/test_math.c new file mode 100644 index 0000000000..55fb11f22c --- /dev/null +++ b/app/test/test_math.c @@ -0,0 +1,125 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright (C) 2025 Microsoft Corporation + */ + +#include <rte_math.h> +#include <rte_debug.h> + +#include "test.h" + +/* Check condition and return if true. */ +#define TEST_MATH_RETURN_IF_ERROR(X) \ +do { \ + if (X) { \ + return -1; \ + } \ +} while (0) + +RTE_LOG_REGISTER(math_logtype_test, test.math, INFO); + +static int +verify_add_overflow_u8(uint8_t a, uint8_t b, + uint8_t expected_res, uint8_t expected_overflow) +{ + uint8_t res; + uint8_t overflow = __rte_add_overflow_u8(a, b, &res); + RTE_TEST_ASSERT_EQUAL(res, expected_res, + "ERROR: __rte_add_overflow_u8(0x%x, 0x%x) returned result 0x%x," + " but 0x%x was expected.", a, b, res, expected_res); + RTE_TEST_ASSERT_EQUAL(overflow, expected_overflow, + "ERROR: __rte_add_overflow_u8(0x%x, 0x%x) returned overflow 0x%x," + " but 0x%x was expected.", a, b, overflow, expected_overflow); + + return 0; +} + +static int +verify_add_overflow_u16(uint16_t a, uint16_t b, + uint16_t expected_res, uint16_t expected_overflow) +{ + uint16_t res; + uint8_t overflow = __rte_add_overflow_u16(a, b, &res); + RTE_TEST_ASSERT_EQUAL(res, expected_res, + "ERROR: __rte_add_overflow_u16(0x%x, 0x%x) returned result 0x%x," + " but 0x%x was expected.", a, b, res, expected_res); + RTE_TEST_ASSERT_EQUAL(overflow, expected_overflow, + "ERROR: __rte_add_overflow_u16(0x%x, 0x%x) returned overflow 0x%x," + " but 0x%x was expected.", a, b, overflow, expected_overflow); + + return 0; +} + +static int +verify_add_overflow_u32(uint32_t a, uint32_t b, + uint32_t expected_res, uint32_t expected_overflow) +{ + uint32_t res; + uint8_t overflow = __rte_add_overflow_u32(a, b, &res); + RTE_TEST_ASSERT_EQUAL(res, expected_res, + "ERROR: __rte_add_overflow_u32(0x%x, 0x%x) returned result 0x%x," + " but 0x%x was expected.", a, b, res, expected_res); + RTE_TEST_ASSERT_EQUAL(overflow, expected_overflow, + "ERROR: __rte_add_overflow_u32(0x%x, 0x%x) returned overflow 0x%x," + " but 0x%x was expected.", a, b, overflow, expected_overflow); + + return 0; +} + +static int +test_add_overflow_u8(void) +{ + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u8(0, 0, 0, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u8(0, 1, 1, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u8(0, 0xFF, 0xFF, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u8(1, 0xFF, 0, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u8(2, 0xFF, 1, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u8(4, 0xFE, 2, 1)); + + return 0; +} + +static int +test_add_overflow_u16(void) +{ + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u16(0, 0, 0, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u16(0, 1, 1, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u16(0, 0xFFFF, 0xFFFF, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u16(1, 0xFFFF, 0, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u16(2, 0xFFFF, 1, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u16(4, 0xFFFE, 2, 1)); + + return 0; +} + +static int +test_add_overflow_u32(void) +{ + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u32(0, 0, 0, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u32(0, 1, 1, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u32(0, 0xFFFFFFFF, 0xFFFFFFFF, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u32(1, 0xFFFFFFFF, 0, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u32(2, 0xFFFFFFFF, 1, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow_u32(4, 0xFFFFFFFE, 2, 1)); + + return 0; +} + +static struct unit_test_suite math_test_suite = { + .suite_name = "math autotest", + .setup = NULL, + .teardown = NULL, + .unit_test_cases = { + TEST_CASE(test_add_overflow_u8), + TEST_CASE(test_add_overflow_u16), + TEST_CASE(test_add_overflow_u32), + TEST_CASES_END() + } +}; + +static int +test_math(void) +{ + return unit_test_suite_runner(&math_test_suite); +} + +REGISTER_FAST_TEST(math_autotest, true, true, test_math); -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] add portable version of __builtin_add_overflow 2025-01-02 22:32 [PATCH 0/5] add portable version of __builtin_add_overflow Andre Muezerie ` (4 preceding siblings ...) 2025-01-02 22:32 ` [PATCH 5/5] app/test: add tests for portable versions " Andre Muezerie @ 2025-01-02 23:51 ` Stephen Hemminger 2025-01-03 0:15 ` Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 " Andre Muezerie 6 siblings, 1 reply; 21+ messages in thread From: Stephen Hemminger @ 2025-01-02 23:51 UTC (permalink / raw) To: Andre Muezerie; +Cc: dev On Thu, 2 Jan 2025 14:32:43 -0800 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. > > Andre Muezerie (5): > maintainers: add portable version of __builtin_add_overflow > lib/eal: add portable version of __builtin_add_overflow > doc/api: add portable version of __builtin_add_overflow > drivers/net: use portable version of __builtin_add_overflow > app/test: add tests for portable versions of __builtin_add_overflow > > MAINTAINERS | 1 + > app/test/meson.build | 1 + > app/test/test_math.c | 125 +++++++++++++++++++++++++++++++++ > doc/api/doxy-api-index.md | 1 + > drivers/net/ice/base/ice_nvm.c | 9 ++- > lib/eal/include/meson.build | 1 + > lib/eal/include/rte_math.h | 42 +++++++++++ > 7 files changed, 175 insertions(+), 5 deletions(-) > create mode 100644 app/test/test_math.c > create mode 100644 lib/eal/include/rte_math.h > > -- > 2.47.0.vfs.0.3 > You should add _builtin_add_overflow into the checkpatch naughty list. Or maybe all the _builtin_XXX functions? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] add portable version of __builtin_add_overflow 2025-01-02 23:51 ` [PATCH 0/5] add portable version " Stephen Hemminger @ 2025-01-03 0:15 ` Andre Muezerie 2025-01-03 0:41 ` Andre Muezerie 0 siblings, 1 reply; 21+ messages in thread From: Andre Muezerie @ 2025-01-03 0:15 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev On Thu, Jan 02, 2025 at 03:51:55PM -0800, Stephen Hemminger wrote: > On Thu, 2 Jan 2025 14:32:43 -0800 > 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. > > > > Andre Muezerie (5): > > maintainers: add portable version of __builtin_add_overflow > > lib/eal: add portable version of __builtin_add_overflow > > doc/api: add portable version of __builtin_add_overflow > > drivers/net: use portable version of __builtin_add_overflow > > app/test: add tests for portable versions of __builtin_add_overflow > > > > MAINTAINERS | 1 + > > app/test/meson.build | 1 + > > app/test/test_math.c | 125 +++++++++++++++++++++++++++++++++ > > doc/api/doxy-api-index.md | 1 + > > drivers/net/ice/base/ice_nvm.c | 9 ++- > > lib/eal/include/meson.build | 1 + > > lib/eal/include/rte_math.h | 42 +++++++++++ > > 7 files changed, 175 insertions(+), 5 deletions(-) > > create mode 100644 app/test/test_math.c > > create mode 100644 lib/eal/include/rte_math.h > > > > -- > > 2.47.0.vfs.0.3 > > > > You should add _builtin_add_overflow into the checkpatch naughty list. > Or maybe all the _builtin_XXX functions? Absolutely! Let me add that for a v2 series. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] add portable version of __builtin_add_overflow 2025-01-03 0:15 ` Andre Muezerie @ 2025-01-03 0:41 ` Andre Muezerie 0 siblings, 0 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-03 0:41 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev On Thu, Jan 02, 2025 at 04:15:31PM -0800, Andre Muezerie wrote: > On Thu, Jan 02, 2025 at 03:51:55PM -0800, Stephen Hemminger wrote: > > On Thu, 2 Jan 2025 14:32:43 -0800 > > 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. > > > > > > Andre Muezerie (5): > > > maintainers: add portable version of __builtin_add_overflow > > > lib/eal: add portable version of __builtin_add_overflow > > > doc/api: add portable version of __builtin_add_overflow > > > drivers/net: use portable version of __builtin_add_overflow > > > app/test: add tests for portable versions of __builtin_add_overflow > > > > > > MAINTAINERS | 1 + > > > app/test/meson.build | 1 + > > > app/test/test_math.c | 125 +++++++++++++++++++++++++++++++++ > > > doc/api/doxy-api-index.md | 1 + > > > drivers/net/ice/base/ice_nvm.c | 9 ++- > > > lib/eal/include/meson.build | 1 + > > > lib/eal/include/rte_math.h | 42 +++++++++++ > > > 7 files changed, 175 insertions(+), 5 deletions(-) > > > create mode 100644 app/test/test_math.c > > > create mode 100644 lib/eal/include/rte_math.h > > > > > > -- > > > 2.47.0.vfs.0.3 > > > > > > > You should add _builtin_add_overflow into the checkpatch naughty list. > > Or maybe all the _builtin_XXX functions? > > Absolutely! Let me add that for a v2 series. Turns out such check was already added recently (MESSAGE='Using __builtin helpers, prefer EAL macros'), so further changes needed at this point. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 0/5] add portable version of __builtin_add_overflow 2025-01-02 22:32 [PATCH 0/5] add portable version of __builtin_add_overflow Andre Muezerie ` (5 preceding siblings ...) 2025-01-02 23:51 ` [PATCH 0/5] add portable version " Stephen Hemminger @ 2025-01-03 20:39 ` Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 1/5] maintainers: " Andre Muezerie ` (4 more replies) 6 siblings, 5 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-03 20:39 UTC (permalink / raw) To: andremue; +Cc: dev, mb __builtin_add_overflow is gcc specific. There's a need for a portable version that can also be used with other compilers. Andre Muezerie (5): maintainers: add portable version of __builtin_add_overflow lib/eal: add portable version of __builtin_add_overflow doc/api: add portable version of __builtin_add_overflow drivers/net: 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/ice/base/ice_nvm.c | 9 +- lib/eal/include/meson.build | 1 + lib/eal/include/rte_math.h | 46 +++++++++ 7 files changed, 224 insertions(+), 5 deletions(-) create mode 100644 app/test/test_math.c create mode 100644 lib/eal/include/rte_math.h -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 1/5] maintainers: add portable version of __builtin_add_overflow 2025-01-03 20:39 ` [PATCH v2 " Andre Muezerie @ 2025-01-03 20:39 ` Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 2/5] lib/eal: " Andre Muezerie ` (3 subsequent siblings) 4 siblings, 0 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-03 20:39 UTC (permalink / raw) To: andremue; +Cc: dev, mb __builtin_add_overflow is gcc specific. There's a need for a portable version that can also be used with other compilers. Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com> --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 0f940ad713..fb64ba893b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -179,6 +179,7 @@ F: app/test/test_devargs.c F: app/test/test_eal* F: app/test/test_errno.c F: app/test/test_lcores.c +F: app/test/test_math.c F: app/test/test_memcpy* F: app/test/test_per_lcore.c F: app/test/test_pflock.c -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 2/5] lib/eal: add portable version of __builtin_add_overflow 2025-01-03 20:39 ` [PATCH v2 " Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 1/5] maintainers: " Andre Muezerie @ 2025-01-03 20:39 ` Andre Muezerie 2025-01-06 11:07 ` Bruce Richardson 2025-01-03 20:39 ` [PATCH v2 3/5] doc/api: " Andre Muezerie ` (2 subsequent siblings) 4 siblings, 1 reply; 21+ messages in thread From: Andre Muezerie @ 2025-01-03 20:39 UTC (permalink / raw) To: andremue; +Cc: dev, mb __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. Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com> --- lib/eal/include/meson.build | 1 + lib/eal/include/rte_math.h | 46 +++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 lib/eal/include/rte_math.h diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build index d903577caa..041a4105b5 100644 --- a/lib/eal/include/meson.build +++ b/lib/eal/include/meson.build @@ -31,6 +31,7 @@ headers += files( 'rte_lcore_var.h', 'rte_lock_annotations.h', 'rte_malloc.h', + 'rte_math.h', 'rte_mcslock.h', 'rte_memory.h', 'rte_memzone.h', diff --git a/lib/eal/include/rte_math.h b/lib/eal/include/rte_math.h new file mode 100644 index 0000000000..2f4581f81b --- /dev/null +++ b/lib/eal/include/rte_math.h @@ -0,0 +1,46 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2025 Microsoft Corporation + */ + +#ifndef _RTE_MATH_H_ +#define _RTE_MATH_H_ + +/** + * @file + * + * Math function definitions for DPDK. + */ + +#ifdef __cplusplus +extern "C" { +#endif + +/* + * 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 + +#ifdef __cplusplus +} +#endif + +#endif -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/5] lib/eal: add portable version of __builtin_add_overflow 2025-01-03 20:39 ` [PATCH v2 2/5] lib/eal: " Andre Muezerie @ 2025-01-06 11:07 ` Bruce Richardson 2025-01-06 11:21 ` Morten Brørup 0 siblings, 1 reply; 21+ messages in thread From: Bruce Richardson @ 2025-01-06 11:07 UTC (permalink / raw) To: Andre Muezerie; +Cc: dev, mb 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. > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com> > --- > lib/eal/include/meson.build | 1 + > lib/eal/include/rte_math.h | 46 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 47 insertions(+) > create mode 100644 lib/eal/include/rte_math.h > > diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build > index d903577caa..041a4105b5 100644 > --- a/lib/eal/include/meson.build > +++ b/lib/eal/include/meson.build > @@ -31,6 +31,7 @@ headers += files( > 'rte_lcore_var.h', > 'rte_lock_annotations.h', > 'rte_malloc.h', > + 'rte_math.h', > 'rte_mcslock.h', > 'rte_memory.h', > 'rte_memzone.h', > diff --git a/lib/eal/include/rte_math.h b/lib/eal/include/rte_math.h > new file mode 100644 > index 0000000000..2f4581f81b > --- /dev/null > +++ b/lib/eal/include/rte_math.h > @@ -0,0 +1,46 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2025 Microsoft Corporation > + */ > + > +#ifndef _RTE_MATH_H_ > +#define _RTE_MATH_H_ > + > +/** > + * @file > + * > + * Math function definitions for DPDK. > + */ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +/* > + * 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 /Bruce ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2 2/5] lib/eal: add portable version of __builtin_add_overflow 2025-01-06 11:07 ` Bruce Richardson @ 2025-01-06 11:21 ` Morten Brørup 2025-01-06 11:34 ` Bruce Richardson 0 siblings, 1 reply; 21+ messages in thread From: Morten Brørup @ 2025-01-06 11:21 UTC (permalink / raw) To: Bruce Richardson, Andre Muezerie; +Cc: dev > 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/5] lib/eal: add portable version of __builtin_add_overflow 2025-01-06 11:21 ` Morten Brørup @ 2025-01-06 11:34 ` Bruce Richardson 2025-01-06 11:58 ` Morten Brørup 0 siblings, 1 reply; 21+ messages in thread From: Bruce Richardson @ 2025-01-06 11:34 UTC (permalink / raw) To: Morten Brørup; +Cc: Andre Muezerie, dev 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2 2/5] lib/eal: add portable version of __builtin_add_overflow 2025-01-06 11:34 ` Bruce Richardson @ 2025-01-06 11:58 ` Morten Brørup 0 siblings, 0 replies; 21+ messages in thread From: Morten Brørup @ 2025-01-06 11:58 UTC (permalink / raw) To: Bruce Richardson; +Cc: Andre Muezerie, dev > From: Bruce Richardson [mailto:bruce.richardson@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@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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 3/5] doc/api: add portable version of __builtin_add_overflow 2025-01-03 20:39 ` [PATCH v2 " Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 1/5] maintainers: " Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 2/5] lib/eal: " Andre Muezerie @ 2025-01-03 20:39 ` Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 4/5] drivers/net: use " Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 5/5] app/test: add tests for " Andre Muezerie 4 siblings, 0 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-03 20:39 UTC (permalink / raw) To: andremue; +Cc: dev, mb __builtin_add_overflow is gcc specific. There's a need for a portable version that can also be used with other compilers. Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com> --- doc/api/doxy-api-index.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md index f0193502bc..c95a86448d 100644 --- a/doc/api/doxy-api-index.md +++ b/doc/api/doxy-api-index.md @@ -226,6 +226,7 @@ The public API headers are grouped by topics: [checksum](@ref rte_cksum.h), [config file](@ref rte_cfgfile.h), [key/value args](@ref rte_kvargs.h), + [math](@ref rte_math.h), [argument parsing](@ref rte_argparse.h), [ptr_compress](@ref rte_ptr_compress.h), [string](@ref rte_string_fns.h), -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 4/5] drivers/net: use portable version of __builtin_add_overflow 2025-01-03 20:39 ` [PATCH v2 " Andre Muezerie ` (2 preceding siblings ...) 2025-01-03 20:39 ` [PATCH v2 3/5] doc/api: " Andre Muezerie @ 2025-01-03 20:39 ` Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 5/5] app/test: add tests for " Andre Muezerie 4 siblings, 0 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-03 20:39 UTC (permalink / raw) To: andremue; +Cc: dev, mb __builtin_add_overflow is gcc specific. It should be replaced with a portable version that can also be used with other compilers. Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com> --- drivers/net/ice/base/ice_nvm.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/net/ice/base/ice_nvm.c b/drivers/net/ice/base/ice_nvm.c index 56c6c96a95..1002a6b59f 100644 --- a/drivers/net/ice/base/ice_nvm.c +++ b/drivers/net/ice/base/ice_nvm.c @@ -3,6 +3,7 @@ */ #include "ice_common.h" +#include <rte_math.h> #define GL_MNG_DEF_DEVID 0x000B611C @@ -469,8 +470,6 @@ int ice_read_sr_word(struct ice_hw *hw, u16 offset, u16 *data) return status; } -#define check_add_overflow __builtin_add_overflow - /** * ice_get_pfa_module_tlv - Reads sub module TLV from NVM PFA * @hw: pointer to hardware structure @@ -500,7 +499,7 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len, return status; } - if (check_add_overflow(pfa_ptr, (u16)(pfa_len - 1), &max_tlv)) { + if (rte_add_overflow(pfa_ptr, (u16)(pfa_len - 1), &max_tlv)) { ice_debug(hw, ICE_DBG_INIT, "PFA starts at offset %u. PFA length of %u caused 16-bit arithmetic overflow.\n", pfa_ptr, pfa_len); return ICE_ERR_INVAL_SIZE; @@ -541,8 +540,8 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len, return ICE_ERR_INVAL_SIZE; } - if (check_add_overflow(next_tlv, (u16)2, &next_tlv) || - check_add_overflow(next_tlv, tlv_len, &next_tlv)) { + if (rte_add_overflow(next_tlv, (u16)2, &next_tlv) || + rte_add_overflow(next_tlv, tlv_len, &next_tlv)) { ice_debug(hw, ICE_DBG_INIT, "TLV of type %u and length 0x%04x caused 16-bit arithmetic overflow. The PFA starts at 0x%04x and has length of 0x%04x\n", tlv_sub_module_type, tlv_len, pfa_ptr, pfa_len); return ICE_ERR_INVAL_SIZE; -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 5/5] app/test: add tests for portable version of __builtin_add_overflow 2025-01-03 20:39 ` [PATCH v2 " Andre Muezerie ` (3 preceding siblings ...) 2025-01-03 20:39 ` [PATCH v2 4/5] drivers/net: use " Andre Muezerie @ 2025-01-03 20:39 ` Andre Muezerie 4 siblings, 0 replies; 21+ messages in thread From: Andre Muezerie @ 2025-01-03 20:39 UTC (permalink / raw) To: andremue; +Cc: dev, mb __builtin_add_overflow is gcc specific. There's a need for a portable version that can also be used with other compilers. This patch adds tests for these new portable functions, to confirm they behave the same way across different compilers. Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com> --- app/test/meson.build | 1 + app/test/test_math.c | 170 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 app/test/test_math.c diff --git a/app/test/meson.build b/app/test/meson.build index d5cb6a7f7a..49e13c5505 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -118,6 +118,7 @@ source_file_deps = { 'test_lpm_perf.c': ['net', 'lpm'], 'test_malloc.c': [], 'test_malloc_perf.c': [], + 'test_math.c': [], 'test_mbuf.c': ['net'], 'test_mcslock.c': [], 'test_member.c': ['member', 'net'], diff --git a/app/test/test_math.c b/app/test/test_math.c new file mode 100644 index 0000000000..5b53eba71d --- /dev/null +++ b/app/test/test_math.c @@ -0,0 +1,170 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright (C) 2025 Microsoft Corporation + */ + +#include <rte_math.h> +#include <rte_debug.h> + +#include "test.h" +#include <inttypes.h> + +/* Check condition and return if true. */ +#define TEST_MATH_RETURN_IF_ERROR(X) \ +do { \ + if (X) { \ + return -1; \ + } \ +} while (0) + +RTE_LOG_REGISTER(math_logtype_test, test.math, INFO); + +static int +verify_add_overflow8(uint8_t a, uint8_t b, + uint8_t expected_res, uint8_t expected_overflow) +{ + uint8_t res; + uint8_t overflow = rte_add_overflow(a, b, &res); + RTE_TEST_ASSERT_EQUAL(res, expected_res, + "ERROR: rte_add_overflow(0x%x, 0x%x) returned result 0x%x," + " but 0x%x was expected.", a, b, res, expected_res); + RTE_TEST_ASSERT_EQUAL(overflow, expected_overflow, + "ERROR: rte_add_overflow(0x%x, 0x%x) returned overflow 0x%x," + " but 0x%x was expected.", a, b, overflow, expected_overflow); + + return 0; +} + +static int +verify_add_overflow16(uint16_t a, uint16_t b, + uint16_t expected_res, uint8_t expected_overflow) +{ + uint16_t res; + uint8_t overflow = rte_add_overflow(a, b, &res); + RTE_TEST_ASSERT_EQUAL(res, expected_res, + "ERROR: rte_add_overflow(0x%x, 0x%x) returned result 0x%x," + " but 0x%x was expected.", a, b, res, expected_res); + RTE_TEST_ASSERT_EQUAL(overflow, expected_overflow, + "ERROR: rte_add_overflow(0x%x, 0x%x) returned overflow 0x%x," + " but 0x%x was expected.", a, b, overflow, expected_overflow); + + return 0; +} + +static int +verify_add_overflow32(uint32_t a, uint32_t b, + uint32_t expected_res, uint8_t expected_overflow) +{ + uint32_t res; + uint8_t overflow = rte_add_overflow(a, b, &res); + RTE_TEST_ASSERT_EQUAL(res, expected_res, + "ERROR: rte_add_overflow(0x%x, 0x%x) returned result 0x%x," + " but 0x%x was expected.", a, b, res, expected_res); + RTE_TEST_ASSERT_EQUAL(overflow, expected_overflow, + "ERROR: rte_add_overflow(0x%x, 0x%x) returned overflow 0x%x," + " but 0x%x was expected.", a, b, overflow, expected_overflow); + + return 0; +} + +static int +verify_add_overflow64(uint64_t a, uint64_t b, + uint64_t expected_res, uint8_t expected_overflow) +{ + uint64_t res; + uint8_t overflow = rte_add_overflow(a, b, &res); + RTE_TEST_ASSERT_EQUAL(res, expected_res, + "ERROR: rte_add_overflow(0x%" PRIx64 ", 0x%" PRIx64 ") returned" + " result 0x%" PRIx64 ", but 0x%" PRIx64 " was expected.", + a, b, res, expected_res); + RTE_TEST_ASSERT_EQUAL(res, expected_res, + "ERROR: rte_add_overflow(0x%" PRIx64 ", 0x%" PRIx64 ") returned" + " overflow 0x%x, but 0x%x was expected.", + a, b, overflow, expected_overflow); + + return 0; +} + +static int +test_add_overflow8(void) +{ + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow8(0, 0, 0, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow8(0, 1, 1, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow8(0, 0xFF, 0xFF, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow8(1, 0xFF, 0, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow8(2, 0xFF, 1, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow8(4, 0xFE, 2, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow8(0xFF, 0xFF, 0xFE, 1)); + + return 0; +} + +static int +test_add_overflow16(void) +{ + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow16(0, 0, 0, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow16(0, 1, 1, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow16(0, 0xFFFF, 0xFFFF, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow16(1, 0xFFFF, 0, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow16(2, 0xFFFF, 1, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow16(4, 0xFFFE, 2, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow16( + 0xFFFF, 0xFFFF, 0xFFFE, 1)); + + return 0; +} + +static int +test_add_overflow32(void) +{ + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow32(0, 0, 0, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow32(0, 1, 1, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow32( + 0, 0xFFFFFFFF, 0xFFFFFFFF, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow32(1, 0xFFFFFFFF, 0, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow32(2, 0xFFFFFFFF, 1, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow32(4, 0xFFFFFFFE, 2, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow32( + 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFE, 1)); + + return 0; +} + +static int +test_add_overflow64(void) +{ + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow64(0, 0, 0, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow64(0, 1, 1, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow64( + 0, 0xFFFFFFFFFFFFFFFF, 0xFFFFFFFFFFFFFFFF, 0)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow64( + 1, 0xFFFFFFFFFFFFFFFF, 0, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow64( + 2, 0xFFFFFFFFFFFFFFFF, 1, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow64( + 4, 0xFFFFFFFFFFFFFFFE, 2, 1)); + TEST_MATH_RETURN_IF_ERROR(verify_add_overflow64( + 0xFFFFFFFFFFFFFFFF, 0xFFFFFFFFFFFFFFFF, 0xFFFFFFFFFFFFFFFE, 1)); + + return 0; +} + +static struct unit_test_suite math_test_suite = { + .suite_name = "math autotest", + .setup = NULL, + .teardown = NULL, + .unit_test_cases = { + TEST_CASE(test_add_overflow8), + TEST_CASE(test_add_overflow16), + TEST_CASE(test_add_overflow32), + TEST_CASE(test_add_overflow64), + TEST_CASES_END() + } +}; + +static int +test_math(void) +{ + return unit_test_suite_runner(&math_test_suite); +} + +REGISTER_FAST_TEST(math_autotest, true, true, test_math); -- 2.47.0.vfs.0.3 ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-01-06 11:58 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-01-02 22:32 [PATCH 0/5] add portable version of __builtin_add_overflow Andre Muezerie 2025-01-02 22:32 ` [PATCH 1/5] maintainers: " Andre Muezerie 2025-01-02 22:32 ` [PATCH 2/5] lib/eal: " Andre Muezerie 2025-01-03 8:19 ` Morten Brørup 2025-01-03 20:38 ` Andre Muezerie 2025-01-02 22:32 ` [PATCH 3/5] doc/api: " Andre Muezerie 2025-01-02 22:32 ` [PATCH 4/5] drivers/net: use " Andre Muezerie 2025-01-02 22:32 ` [PATCH 5/5] app/test: add tests for portable versions " Andre Muezerie 2025-01-02 23:51 ` [PATCH 0/5] add portable version " Stephen Hemminger 2025-01-03 0:15 ` Andre Muezerie 2025-01-03 0:41 ` Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 " Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 1/5] maintainers: " Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 2/5] lib/eal: " Andre Muezerie 2025-01-06 11:07 ` Bruce Richardson 2025-01-06 11:21 ` Morten Brørup 2025-01-06 11:34 ` Bruce Richardson 2025-01-06 11:58 ` Morten Brørup 2025-01-03 20:39 ` [PATCH v2 3/5] doc/api: " Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 4/5] drivers/net: use " Andre Muezerie 2025-01-03 20:39 ` [PATCH v2 5/5] app/test: add tests for " Andre Muezerie
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).