On Wed, Mar 05, 2025 at 09:20:03AM +0000, Bruce Richardson wrote:
> On Tue, Mar 04, 2025 at 01:53:19PM -0800, Andre Muezerie wrote:
> > This test initializes an __m128i data type using the old
> > non-portable way used until now and the more portable way
> > using compiler intrinsics. The test ensures the resulting
> > values after initialization match.
> >
> > Signed-off-by: Andre Muezerie <andre...@linux.microsoft.com>
> > ---
> > app/test/test_thash.c | 34 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/app/test/test_thash.c b/app/test/test_thash.c
> > index 33b0c6adac..5f7081a3ad 100644
> > --- a/app/test/test_thash.c
> > +++ b/app/test/test_thash.c
> > @@ -1029,6 +1029,35 @@ test_keygen(void)
> > return TEST_SUCCESS;
> > }
> >
> > +#ifdef RTE_ARCH_X86
> > +#ifndef RTE_TOOLCHAIN_MSVC
> > +static int
> > +test_init_m128i(void)
> > +{
> > + /* When initializing __m128i with two constant values like below
> > + * MSVC issues warning C4305:
> > + * 'initializing': truncation from 'unsigned __int64' to 'char'
> > + */
> > + static const __m128i a = {
> > + 0x0405060700010203ULL, 0x0C0D0E0F08090A0BULL};
> > +
> > + /* Using compiler intrinsics to initialize __m128i is therefore
> > + * preferred, like below
> > + */
> > + const __m128i b = _mm_set_epi64x(
> > + 0x0C0D0E0F08090A0BULL, 0x0405060700010203ULL);
> > +
> > + if (memcmp(&a, &b, sizeof(a)) != 0) {
> > + printf("Same value was expected when initializing data "
> > + "type using compiler intrinsic\n");
> > + return -1;
> > + }
> > +
> > + return TEST_SUCCESS;
> > +}
> > +#endif
> > +#endif
> > +
> Do we still need this patch? I don't think its necessary.
It was important to give me confidence that I was flipping the arguments
correctly. I
agree that moving forward this test does not add much value and can be removed.
What is the correct process to do that? Should I send a new series without that
patch or can it be simply ignored during merge?
