mbox series

[v2,0/3] provide rte_ffs32, rte_ffs64 and __rte_x86_movdiri

Message ID 1733430950-10412-1-git-send-email-andremue@linux.microsoft.com (mailing list archive)
Headers
Series provide rte_ffs32, rte_ffs64 and __rte_x86_movdiri |

Message

Andre Muezerie Dec. 5, 2024, 8:35 p.m. UTC
MSVC does not support inline assembly so use movdiri intrinsic and
provide abstracted rte_ffs{32,64} inline functions instead of directly
using GCC built-ins.

v2:
 * Moved constants to the right side of the comparison
 * Added tests for rte_ffs32 and rte_ffs64 functions

Andre Muezerie (1):
  app/test: add test for rte_ffs32 and rte_ffs64 functions.

Tyler Retzlaff (2):
  eal: provide movdiri for MSVC
  eal: add rte ffs32 and rte ffs64 inline functions

 app/test/test_bitops.c       | 38 ++++++++++++++++++++++++++++++++++++
 lib/eal/include/rte_bitops.h | 34 ++++++++++++++++++++++++++++++++
 lib/eal/x86/include/rte_io.h |  4 ++++
 3 files changed, 76 insertions(+)

--
2.47.0.vfs.0.3
  

Comments

David Marchand Jan. 24, 2025, 2:53 p.m. UTC | #1
On Thu, Dec 5, 2024 at 9:36 PM Andre Muezerie
<andremue@linux.microsoft.com> wrote:
>
> MSVC does not support inline assembly so use movdiri intrinsic and
> provide abstracted rte_ffs{32,64} inline functions instead of directly
> using GCC built-ins.
>
> v2:
>  * Moved constants to the right side of the comparison
>  * Added tests for rte_ffs32 and rte_ffs64 functions
>
> Andre Muezerie (1):
>   app/test: add test for rte_ffs32 and rte_ffs64 functions.
>
> Tyler Retzlaff (2):
>   eal: provide movdiri for MSVC
>   eal: add rte ffs32 and rte ffs64 inline functions
>
>  app/test/test_bitops.c       | 38 ++++++++++++++++++++++++++++++++++++
>  lib/eal/include/rte_bitops.h | 34 ++++++++++++++++++++++++++++++++
>  lib/eal/x86/include/rte_io.h |  4 ++++
>  3 files changed, 76 insertions(+)
>

I see nothing wrong with adding those wrappers to ease MSVC support.
Just, those two ffs helpers should be marked experimental.

And the unit tests for counting/searching bits had been separated in a
dedicated app/test/test_bitcount.c.
  
Andre Muezerie Jan. 24, 2025, 4:16 p.m. UTC | #2
On Fri, Jan 24, 2025 at 03:53:58PM +0100, David Marchand wrote:
> On Thu, Dec 5, 2024 at 9:36 PM Andre Muezerie
> <andremue@linux.microsoft.com> wrote:
> >
> > MSVC does not support inline assembly so use movdiri intrinsic and
> > provide abstracted rte_ffs{32,64} inline functions instead of directly
> > using GCC built-ins.
> >
> > v2:
> >  * Moved constants to the right side of the comparison
> >  * Added tests for rte_ffs32 and rte_ffs64 functions
> >
> > Andre Muezerie (1):
> >   app/test: add test for rte_ffs32 and rte_ffs64 functions.
> >
> > Tyler Retzlaff (2):
> >   eal: provide movdiri for MSVC
> >   eal: add rte ffs32 and rte ffs64 inline functions
> >
> >  app/test/test_bitops.c       | 38 ++++++++++++++++++++++++++++++++++++
> >  lib/eal/include/rte_bitops.h | 34 ++++++++++++++++++++++++++++++++
> >  lib/eal/x86/include/rte_io.h |  4 ++++
> >  3 files changed, 76 insertions(+)
> >
> 
> I see nothing wrong with adding those wrappers to ease MSVC support.
> Just, those two ffs helpers should be marked experimental.
> 
> And the unit tests for counting/searching bits had been separated in a
> dedicated app/test/test_bitcount.c.
> 
> 
> -- 
> David Marchand

Thanks for the review David. I sent a v3 series addressing these issues.