[PATCH 0/2] provide toolchain abstracted __builtin_constant_p

[PATCH 0/2] provide toolchain abstracted __builtin_constant_p

[Tyler Retzlaff]

Decouple direct dependency on GCC built-in __builtin_constant_p provide
a new macro __rte_constant(e) that expands to the built-in for GCC and
to false for MSVC.

Tyler Retzlaff (2):
  eal: provide macro for GCC builtin constant intrinsic
  mempool: use rte constant macro instead of GCC builtin

 lib/eal/include/rte_common.h | 6 ++++++
 lib/mempool/rte_mempool.h    | 7 +++----
 2 files changed, 9 insertions(+), 4 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] eal: provide macro for GCC builtin constant intrinsic

[Tyler Retzlaff]

MSVC does not have a __builtin_constant_p intrinsic so provide
__rte_constant(e) that expands false for MSVC and to the intrinsic for
GCC.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/include/rte_common.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index 298a5c6..d520be6 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -44,6 +44,12 @@
 #endif
 #endif
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_constant(e) 0
+#else
+#define __rte_constant(e) __extension__(__builtin_constant_p(e))
+#endif
+
 /*
  * RTE_TOOLCHAIN_GCC is defined if the target is built with GCC,
  * while a host application (like pmdinfogen) may have another compiler.
-- 
1.8.3.1

RE: [PATCH 1/2] eal: provide macro for GCC builtin constant intrinsic

[Morten Brørup]

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Wednesday, 20 March 2024 22.34
> 
> MSVC does not have a __builtin_constant_p intrinsic so provide
> __rte_constant(e) that expands false for MSVC and to the intrinsic for
> GCC.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---

Reviewed-by: Morten Brørup <mb@smartsharesystems.com>

Re: [PATCH 1/2] eal: provide macro for GCC builtin constant intrinsic

[Stephen Hemminger]

On Wed, 20 Mar 2024 14:33:35 -0700
Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:

> +#ifdef RTE_TOOLCHAIN_MSVC
> +#define __rte_constant(e) 0
> +#else
> +#define __rte_constant(e) __extension__(__builtin_constant_p(e))
> +#endif
> +


I did some looking around and some other project have macros
for expressing constant expression vs constant.

Implementing this with some form of sizeof math is possible.
For example in linux/compiler.h

/*
 * This returns a constant expression while determining if an argument is
 * a constant expression, most importantly without evaluating the argument.
 * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
 *
 * Details:
 * - sizeof() return an integer constant expression, and does not evaluate
 *   the value of its operand; it only examines the type of its operand.
 * - The results of comparing two integer constant expressions is also
 *   an integer constant expression.
 * - The first literal "8" isn't important. It could be any literal value.
 * - The second literal "8" is to avoid warnings about unaligned pointers;
 *   this could otherwise just be "1".
 * - (long)(x) is used to avoid warnings about 64-bit types on 32-bit
 *   architectures.
 * - The C Standard defines "null pointer constant", "(void *)0", as
 *   distinct from other void pointers.
 * - If (x) is an integer constant expression, then the "* 0l" resolves
 *   it into an integer constant expression of value 0. Since it is cast to
 *   "void *", this makes the second operand a null pointer constant.
 * - If (x) is not an integer constant expression, then the second operand
 *   resolves to a void pointer (but not a null pointer constant: the value
 *   is not an integer constant 0).
 * - The conditional operator's third operand, "(int *)8", is an object
 *   pointer (to type "int").
 * - The behavior (including the return type) of the conditional operator
 *   ("operand1 ? operand2 : operand3") depends on the kind of expressions
 *   given for the second and third operands. This is the central mechanism
 *   of the macro:
 *   - When one operand is a null pointer constant (i.e. when x is an integer
 *     constant expression) and the other is an object pointer (i.e. our
 *     third operand), the conditional operator returns the type of the
 *     object pointer operand (i.e. "int *). Here, within the sizeof(), we
 *     would then get:
 *       sizeof(*((int *)(...))  == sizeof(int)  == 4
 *   - When one operand is a void pointer (i.e. when x is not an integer
 *     constant expression) and the other is an object pointer (i.e. our
 *     third operand), the conditional operator returns a "void *" type.
 *     Here, within the sizeof(), we would then get:
 *       sizeof(*((void *)(...)) == sizeof(void) == 1
 * - The equality comparison to "sizeof(int)" therefore depends on (x):
 *     sizeof(int) == sizeof(int)     (x) was a constant expression
 *     sizeof(int) != sizeof(void)    (x) was not a constant expression
 */
#define __is_constexpr(x) \
	(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))

RE: [PATCH 1/2] eal: provide macro for GCC builtin constant intrinsic

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Monday, 1 April 2024 00.03
> 
> On Wed, 20 Mar 2024 14:33:35 -0700
> Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:
> 
> > +#ifdef RTE_TOOLCHAIN_MSVC
> > +#define __rte_constant(e) 0
> > +#else
> > +#define __rte_constant(e) __extension__(__builtin_constant_p(e))
> > +#endif
> > +
> 
> 
> I did some looking around and some other project have macros
> for expressing constant expression vs constant.
> 
> Implementing this with some form of sizeof math is possible.
> For example in linux/compiler.h
> 
> /*
>  * This returns a constant expression while determining if an argument
> is
>  * a constant expression, most importantly without evaluating the
> argument.
>  * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
>  *
>  * Details:
>  * - sizeof() return an integer constant expression, and does not
> evaluate
>  *   the value of its operand; it only examines the type of its operand.
>  * - The results of comparing two integer constant expressions is also
>  *   an integer constant expression.
>  * - The first literal "8" isn't important. It could be any literal
> value.
>  * - The second literal "8" is to avoid warnings about unaligned
> pointers;
>  *   this could otherwise just be "1".
>  * - (long)(x) is used to avoid warnings about 64-bit types on 32-bit
>  *   architectures.
>  * - The C Standard defines "null pointer constant", "(void *)0", as
>  *   distinct from other void pointers.
>  * - If (x) is an integer constant expression, then the "* 0l" resolves
>  *   it into an integer constant expression of value 0. Since it is cast
> to
>  *   "void *", this makes the second operand a null pointer constant.
>  * - If (x) is not an integer constant expression, then the second
> operand
>  *   resolves to a void pointer (but not a null pointer constant: the
> value
>  *   is not an integer constant 0).
>  * - The conditional operator's third operand, "(int *)8", is an object
>  *   pointer (to type "int").
>  * - The behavior (including the return type) of the conditional
> operator
>  *   ("operand1 ? operand2 : operand3") depends on the kind of
> expressions
>  *   given for the second and third operands. This is the central
> mechanism
>  *   of the macro:
>  *   - When one operand is a null pointer constant (i.e. when x is an
> integer
>  *     constant expression) and the other is an object pointer (i.e. our
>  *     third operand), the conditional operator returns the type of the
>  *     object pointer operand (i.e. "int *). Here, within the sizeof(),
> we
>  *     would then get:
>  *       sizeof(*((int *)(...))  == sizeof(int)  == 4
>  *   - When one operand is a void pointer (i.e. when x is not an integer
>  *     constant expression) and the other is an object pointer (i.e. our
>  *     third operand), the conditional operator returns a "void *" type.
>  *     Here, within the sizeof(), we would then get:
>  *       sizeof(*((void *)(...)) == sizeof(void) == 1
>  * - The equality comparison to "sizeof(int)" therefore depends on (x):
>  *     sizeof(int) == sizeof(int)     (x) was a constant expression
>  *     sizeof(int) != sizeof(void)    (x) was not a constant expression
>  */
> #define __is_constexpr(x) \
> 	(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int
> *)8)))

Nice!
If the author is willing to license it under the BSD license, we can copy it as is.

We might want to add a couple of build time checks to verify that it does what is expected; to catch any changes in compiler behavior.

RE: [PATCH 1/2] eal: provide macro for GCC builtin constant intrinsic

[Morten Brørup]

PING for Review/ACK.

Come on fellow reviewers, it's only 5 lines of code!

The mempool library cannot build with MSVC without this patch series.

Other patches are also being held back, waiting for this MSVC compatible DPDK macro for __builtin_constant_p().

The macro for MSVC can be improved as suggested by Stephen later.

> From: Morten Brørup [mailto:mb@smartsharesystems.com]
> Sent: Monday, 1 April 2024 10.35
> 
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Monday, 1 April 2024 00.03
> >
> > On Wed, 20 Mar 2024 14:33:35 -0700
> > Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:
> >
> > > +#ifdef RTE_TOOLCHAIN_MSVC
> > > +#define __rte_constant(e) 0
> > > +#else
> > > +#define __rte_constant(e) __extension__(__builtin_constant_p(e))
> > > +#endif
> > > +
> >
> >
> > I did some looking around and some other project have macros
> > for expressing constant expression vs constant.
> >
> > Implementing this with some form of sizeof math is possible.
> > For example in linux/compiler.h
> >
> > /*
> >  * This returns a constant expression while determining if an argument
> > is
> >  * a constant expression, most importantly without evaluating the
> > argument.
> >  * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
> >  *
> >  * Details:
> >  * - sizeof() return an integer constant expression, and does not
> > evaluate
> >  *   the value of its operand; it only examines the type of its operand.
> >  * - The results of comparing two integer constant expressions is also
> >  *   an integer constant expression.
> >  * - The first literal "8" isn't important. It could be any literal
> > value.
> >  * - The second literal "8" is to avoid warnings about unaligned
> > pointers;
> >  *   this could otherwise just be "1".
> >  * - (long)(x) is used to avoid warnings about 64-bit types on 32-bit
> >  *   architectures.
> >  * - The C Standard defines "null pointer constant", "(void *)0", as
> >  *   distinct from other void pointers.
> >  * - If (x) is an integer constant expression, then the "* 0l" resolves
> >  *   it into an integer constant expression of value 0. Since it is cast
> > to
> >  *   "void *", this makes the second operand a null pointer constant.
> >  * - If (x) is not an integer constant expression, then the second
> > operand
> >  *   resolves to a void pointer (but not a null pointer constant: the
> > value
> >  *   is not an integer constant 0).
> >  * - The conditional operator's third operand, "(int *)8", is an object
> >  *   pointer (to type "int").
> >  * - The behavior (including the return type) of the conditional
> > operator
> >  *   ("operand1 ? operand2 : operand3") depends on the kind of
> > expressions
> >  *   given for the second and third operands. This is the central
> > mechanism
> >  *   of the macro:
> >  *   - When one operand is a null pointer constant (i.e. when x is an
> > integer
> >  *     constant expression) and the other is an object pointer (i.e. our
> >  *     third operand), the conditional operator returns the type of the
> >  *     object pointer operand (i.e. "int *). Here, within the sizeof(),
> > we
> >  *     would then get:
> >  *       sizeof(*((int *)(...))  == sizeof(int)  == 4
> >  *   - When one operand is a void pointer (i.e. when x is not an integer
> >  *     constant expression) and the other is an object pointer (i.e. our
> >  *     third operand), the conditional operator returns a "void *" type.
> >  *     Here, within the sizeof(), we would then get:
> >  *       sizeof(*((void *)(...)) == sizeof(void) == 1
> >  * - The equality comparison to "sizeof(int)" therefore depends on (x):
> >  *     sizeof(int) == sizeof(int)     (x) was a constant expression
> >  *     sizeof(int) != sizeof(void)    (x) was not a constant expression
> >  */
> > #define __is_constexpr(x) \
> > 	(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int
> > *)8)))
> 
> Nice!
> If the author is willing to license it under the BSD license, we can copy it
> as is.
> 
> We might want to add a couple of build time checks to verify that it does what
> is expected; to catch any changes in compiler behavior.

Re: [PATCH 1/2] eal: provide macro for GCC builtin constant intrinsic

[Andrew Rybchenko]

On 4/1/24 11:34, Morten Brørup wrote:
>> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>> Sent: Monday, 1 April 2024 00.03
>>
>> On Wed, 20 Mar 2024 14:33:35 -0700
>> Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:
>>
>>> +#ifdef RTE_TOOLCHAIN_MSVC
>>> +#define __rte_constant(e) 0
>>> +#else
>>> +#define __rte_constant(e) __extension__(__builtin_constant_p(e))
>>> +#endif
>>> +
>>
>>
>> I did some looking around and some other project have macros
>> for expressing constant expression vs constant.
>>
>> Implementing this with some form of sizeof math is possible.
>> For example in linux/compiler.h
>>
>> /*
>>   * This returns a constant expression while determining if an argument
>> is
>>   * a constant expression, most importantly without evaluating the
>> argument.
>>   * Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de>
>>   *
>>   * Details:
>>   * - sizeof() return an integer constant expression, and does not
>> evaluate
>>   *   the value of its operand; it only examines the type of its operand.
>>   * - The results of comparing two integer constant expressions is also
>>   *   an integer constant expression.
>>   * - The first literal "8" isn't important. It could be any literal
>> value.
>>   * - The second literal "8" is to avoid warnings about unaligned
>> pointers;
>>   *   this could otherwise just be "1".
>>   * - (long)(x) is used to avoid warnings about 64-bit types on 32-bit
>>   *   architectures.
>>   * - The C Standard defines "null pointer constant", "(void *)0", as
>>   *   distinct from other void pointers.
>>   * - If (x) is an integer constant expression, then the "* 0l" resolves
>>   *   it into an integer constant expression of value 0. Since it is cast
>> to
>>   *   "void *", this makes the second operand a null pointer constant.
>>   * - If (x) is not an integer constant expression, then the second
>> operand
>>   *   resolves to a void pointer (but not a null pointer constant: the
>> value
>>   *   is not an integer constant 0).
>>   * - The conditional operator's third operand, "(int *)8", is an object
>>   *   pointer (to type "int").
>>   * - The behavior (including the return type) of the conditional
>> operator
>>   *   ("operand1 ? operand2 : operand3") depends on the kind of
>> expressions
>>   *   given for the second and third operands. This is the central
>> mechanism
>>   *   of the macro:
>>   *   - When one operand is a null pointer constant (i.e. when x is an
>> integer
>>   *     constant expression) and the other is an object pointer (i.e. our
>>   *     third operand), the conditional operator returns the type of the
>>   *     object pointer operand (i.e. "int *). Here, within the sizeof(),
>> we
>>   *     would then get:
>>   *       sizeof(*((int *)(...))  == sizeof(int)  == 4
>>   *   - When one operand is a void pointer (i.e. when x is not an integer
>>   *     constant expression) and the other is an object pointer (i.e. our
>>   *     third operand), the conditional operator returns a "void *" type.
>>   *     Here, within the sizeof(), we would then get:
>>   *       sizeof(*((void *)(...)) == sizeof(void) == 1
>>   * - The equality comparison to "sizeof(int)" therefore depends on (x):
>>   *     sizeof(int) == sizeof(int)     (x) was a constant expression
>>   *     sizeof(int) != sizeof(void)    (x) was not a constant expression
>>   */
>> #define __is_constexpr(x) \
>> 	(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int
>> *)8)))
> 
> Nice!
> If the author is willing to license it under the BSD license, we can copy it as is.
> 
> We might want to add a couple of build time checks to verify that it does what is expected; to catch any changes in compiler behavior.
> 

LGTM too, but meanwhile we can continue without it just to unblock build 
on MSVC

Re: [PATCH 1/2] eal: provide macro for GCC builtin constant intrinsic

[Bruce Richardson]

On Wed, Mar 20, 2024 at 02:33:35PM -0700, Tyler Retzlaff wrote:
> MSVC does not have a __builtin_constant_p intrinsic so provide
> __rte_constant(e) that expands false for MSVC and to the intrinsic for
> GCC.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  lib/eal/include/rte_common.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> index 298a5c6..d520be6 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -44,6 +44,12 @@
>  #endif
>  #endif
>  
> +#ifdef RTE_TOOLCHAIN_MSVC
> +#define __rte_constant(e) 0
> +#else
> +#define __rte_constant(e) __extension__(__builtin_constant_p(e))
> +#endif
> +

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH 1/2] eal: provide macro for GCC builtin constant intrinsic

[Andrew Rybchenko]

On 5/27/24 15:00, Bruce Richardson wrote:
> On Wed, Mar 20, 2024 at 02:33:35PM -0700, Tyler Retzlaff wrote:
>> MSVC does not have a __builtin_constant_p intrinsic so provide
>> __rte_constant(e) that expands false for MSVC and to the intrinsic for
>> GCC.
>>
>> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>> ---
>>   lib/eal/include/rte_common.h | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
>> index 298a5c6..d520be6 100644
>> --- a/lib/eal/include/rte_common.h
>> +++ b/lib/eal/include/rte_common.h
>> @@ -44,6 +44,12 @@
>>   #endif
>>   #endif
>>   
>> +#ifdef RTE_TOOLCHAIN_MSVC
>> +#define __rte_constant(e) 0
>> +#else
>> +#define __rte_constant(e) __extension__(__builtin_constant_p(e))
>> +#endif
>> +
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

[PATCH 2/2] mempool: use rte constant macro instead of GCC builtin

[Tyler Retzlaff]

Use newly introduced __rte_constant(e) macro instead of directly using
__builtin_constant_p() allowing mempool to be built by MSVC.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/mempool/rte_mempool.h | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 23fd5c8..a3564fb 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -1521,7 +1521,7 @@ struct rte_mempool_cache *
 	/* The cache is a stack, so copy will be in reverse order. */
 	cache_objs = &cache->objs[cache->len];
 
-	if (__extension__(__builtin_constant_p(n)) && n <= cache->len) {
+	if (__rte_constant(n) && n <= cache->len) {
 		/*
 		 * The request size is known at build time, and
 		 * the entire request can be satisfied from the cache,
@@ -1542,8 +1542,7 @@ struct rte_mempool_cache *
 	 * If the request size 'n' is known at build time, the above comparison
 	 * ensures that n > cache->len here, so omit RTE_MIN().
 	 */
-	len = __extension__(__builtin_constant_p(n)) ? cache->len :
-			RTE_MIN(n, cache->len);
+	len = __rte_constant(n) ? cache->len : RTE_MIN(n, cache->len);
 	cache->len -= len;
 	remaining = n - len;
 	for (index = 0; index < len; index++)
@@ -1554,7 +1553,7 @@ struct rte_mempool_cache *
 	 * where the entire request can be satisfied from the cache
 	 * has already been handled above, so omit handling it here.
 	 */
-	if (!__extension__(__builtin_constant_p(n)) && remaining == 0) {
+	if (!__rte_constant(n) && remaining == 0) {
 		/* The entire request is satisfied from the cache. */
 
 		RTE_MEMPOOL_CACHE_STAT_ADD(cache, get_success_bulk, 1);
-- 
1.8.3.1

RE: [PATCH 2/2] mempool: use rte constant macro instead of GCC builtin

[Morten Brørup]

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Wednesday, 20 March 2024 22.34
> 
> Use newly introduced __rte_constant(e) macro instead of directly using
> __builtin_constant_p() allowing mempool to be built by MSVC.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---

Reviewed-by: Morten Brørup <mb@smartsharesystems.com>

Re: [PATCH 2/2] mempool: use rte constant macro instead of GCC builtin

[Andrew Rybchenko]

On 3/26/24 12:57, Morten Brørup wrote:
>> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
>> Sent: Wednesday, 20 March 2024 22.34
>>
>> Use newly introduced __rte_constant(e) macro instead of directly using
>> __builtin_constant_p() allowing mempool to be built by MSVC.
>>
>> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>> ---
> 
> Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
> 

Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Re: [PATCH 2/2] mempool: use rte constant macro instead of GCC builtin

[Thomas Monjalon]

20/03/2024 22:33, Tyler Retzlaff:
> Use newly introduced __rte_constant(e) macro instead of directly using
> __builtin_constant_p() allowing mempool to be built by MSVC.

Does it mean we should enable mempool build?
If yes, please send a v2.