diff mbox series

[v2,1/2] lib/hash: initialize __m128i data type in a portable way

Message ID 1741125199-1217-1-git-send-email-andremue@linux.microsoft.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series [v2,1/2] lib/hash: initialize __m128i data type in a portable way |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Andre Muezerie March 4, 2025, 9:53 p.m. UTC 
The mechanism used to initialize an __m128i data type in rte_thash.h is
non-portable and MSVC does not like it. It clearly is not doing what
is desired:

..\lib\hash\rte_thash.h(38): warning C4305: 'initializing':
    truncation from 'unsigned __int64' to 'char'
..\lib\hash\rte_thash.h(38): warning C4305: 'initializing':
    truncation from 'unsigned __int64' to 'char'

A more portable approach is to use compiler intrinsics to perform the
initialization. This patch uses a single compiler intrinsic to
initialize the data.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 lib/hash/rte_thash.h | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Comments

Bruce Richardson March 5, 2025, 9:13 a.m. UTC | #1 
On Tue, Mar 04, 2025 at 01:53:18PM -0800, Andre Muezerie wrote:
> The mechanism used to initialize an __m128i data type in rte_thash.h is
> non-portable and MSVC does not like it. It clearly is not doing what
> is desired:
> 
> ..\lib\hash\rte_thash.h(38): warning C4305: 'initializing':
>     truncation from 'unsigned __int64' to 'char'
> ..\lib\hash\rte_thash.h(38): warning C4305: 'initializing':
>     truncation from 'unsigned __int64' to 'char'
> 
> A more portable approach is to use compiler intrinsics to perform the
> initialization. This patch uses a single compiler intrinsic to
> initialize the data.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> ---
>  lib/hash/rte_thash.h | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
Much simpler fix, thanks.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
David Marchand March 6, 2025, 1:32 p.m. UTC | #2 
On Tue, Mar 4, 2025 at 10:53 PM Andre Muezerie
<andremue@linux.microsoft.com> wrote:
>
> The mechanism used to initialize an __m128i data type in rte_thash.h is
> non-portable and MSVC does not like it. It clearly is not doing what
> is desired:
>
> ..\lib\hash\rte_thash.h(38): warning C4305: 'initializing':
>     truncation from 'unsigned __int64' to 'char'
> ..\lib\hash\rte_thash.h(38): warning C4305: 'initializing':
>     truncation from 'unsigned __int64' to 'char'
>
> A more portable approach is to use compiler intrinsics to perform the
> initialization. This patch uses a single compiler intrinsic to
> initialize the data.
>
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

No need to send a new revision.
I applied just this patch, and dropped the second patch in patchwork.

Applied, thanks.
diff mbox series

Patch

diff --git a/lib/hash/rte_thash.h b/lib/hash/rte_thash.h
index 04f9f1875c..8bca430663 100644
--- a/lib/hash/rte_thash.h
+++ b/lib/hash/rte_thash.h
@@ -30,14 +30,6 @@ 
 extern "C" {
 #endif
 
-#ifdef RTE_ARCH_X86
-/* Byte swap mask used for converting IPv6 address
- * 4-byte chunks to CPU byte order
- */
-static const __m128i rte_thash_ipv6_bswap_mask = {
-		0x0405060700010203ULL, 0x0C0D0E0F08090A0BULL};
-#endif
-
 /**
  * length in dwords of input tuple to
  * calculate hash of ipv4 header only
@@ -159,6 +151,11 @@  rte_thash_load_v6_addrs(const struct rte_ipv6_hdr *orig,
 			union rte_thash_tuple *targ)
 {
 #ifdef RTE_ARCH_X86
+	/* Byte swap mask used for converting IPv6 address
+	 * 4-byte chunks to CPU byte order
+	 */
+	const __m128i rte_thash_ipv6_bswap_mask = _mm_set_epi64x(
+		0x0C0D0E0F08090A0BULL, 0x0405060700010203ULL);
 	__m128i ipv6 = _mm_loadu_si128((const __m128i *)&orig->src_addr);
 	*(__m128i *)&targ->v6.src_addr =
 			_mm_shuffle_epi8(ipv6, rte_thash_ipv6_bswap_mask);