<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Fri, Nov 14, 2025 at 9:19 PM Bruce Richardson <<a href="mailto:bruce.richardson@intel.com">bruce.richardson@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sat, Oct 11, 2025 at 04:59:34PM +0530, Shreesh Adiga wrote:<br>
> Replace the clearing of lower 32 bits of XMM register with blend of<br>
> zero register.<br>
> Remove the clearing of upper 64 bits of tmp1 as it is redundant.<br>
> tmp1 after clearing upper bits was being xor with tmp2 before the<br>
> bits 96:65 from tmp2 were returned. The xor operation of bits 96:65<br>
> remains unchanged due to tmp1 having bits 96:64 cleared to 0.<br>
> After removing the xor operation, the clearing of upper 64 bits of tmp1<br>
> becomes redundant and hence can be removed.<br>
> Clang is able to optimize away the AND + memory operand with the<br>
> above sequence, however GCC is still emitting the code for AND with<br>
> memory operands which is being explicitly eliminated here.<br>
> <br>
> Additionally replace the 48 byte crc_xmm_shift_tab with the contents of<br>
> shf_table which is 32 bytes, achieving the same functionality.<br>
> <br>
> Signed-off-by: Shreesh Adiga <<a href="mailto:16567adigashreesh@gmail.com" target="_blank">16567adigashreesh@gmail.com</a>><br>
> ---<br>
<br>
Sorry for delay in getting back to look at the second version of this. The<br>
explanation, given in reponse to questions of v1, of the second set of<br>
changes in this makes sense.<br>
<br>
Acked-by: Bruce Richardson <<a href="mailto:bruce.richardson@intel.com" target="_blank">bruce.richardson@intel.com</a>><br>
<br>
Ideally, this patch should have been sent in reponse to v1 to keep the<br>
thread together. Also, I think this would be better split into two patches,<br>
one for the reduce64_to_32 change and another for the shift table change.<br>
That way, you could include the fuller explanation of the second change in<br>
the commit log to make easier review.<br></blockquote><div><br></div><div>Sure I will send an updated patch after splitting into two patches. </div><div>Since I am not familiar with email based patch submissions, it ended up being</div><div>a new thread, sorry about that. I will try to update this thread with the new revision soon.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Changes since v1:<br>
> Reversed the operands in the blend operation for readability.<br>
> Removed tmp1 operations that are not affecting the result and hence<br>
> avoid clearing the upper 64 bits for tmp1.<br>
> <br>
>  lib/net/net_crc_sse.c | 30 ++++++------------------------<br>
>  1 file changed, 6 insertions(+), 24 deletions(-)<br>
> <br>
> diff --git a/lib/net/net_crc_sse.c b/lib/net/net_crc_sse.c<br>
> index 112dc94ac1..e590aeb5ac 100644<br>
> --- a/lib/net/net_crc_sse.c<br>
> +++ b/lib/net/net_crc_sse.c<br>
> @@ -96,35 +96,24 @@ crcr32_reduce_128_to_64(__m128i data128, __m128i precomp)<br>
>  static __rte_always_inline uint32_t<br>
>  crcr32_reduce_64_to_32(__m128i data64, __m128i precomp)<br>
>  {<br>
> -     static const alignas(16) uint32_t mask1[4] = {<br>
> -             0xffffffff, 0xffffffff, 0x00000000, 0x00000000<br>
> -     };<br>
> -<br>
> -     static const alignas(16) uint32_t mask2[4] = {<br>
> -             0x00000000, 0xffffffff, 0xffffffff, 0xffffffff<br>
> -     };<br>
>       __m128i tmp0, tmp1, tmp2;<br>
> <br>
> -     tmp0 = _mm_and_si128(data64, _mm_load_si128((const __m128i *)mask2));<br>
> +     tmp0 = _mm_blend_epi16(data64, _mm_setzero_si128(), 0x3);<br>
> <br>
>       tmp1 = _mm_clmulepi64_si128(tmp0, precomp, 0x00);<br>
>       tmp1 = _mm_xor_si128(tmp1, tmp0);<br>
> -     tmp1 = _mm_and_si128(tmp1, _mm_load_si128((const __m128i *)mask1));<br>
> <br>
>       tmp2 = _mm_clmulepi64_si128(tmp1, precomp, 0x10);<br>
> -     tmp2 = _mm_xor_si128(tmp2, tmp1);<br>
>       tmp2 = _mm_xor_si128(tmp2, tmp0);<br>
> <br>
>       return _mm_extract_epi32(tmp2, 2);<br>
>  }<br>
> <br>
> -static const alignas(16) uint8_t crc_xmm_shift_tab[48] = {<br>
> -     0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,<br>
> -     0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,<br>
> +static const alignas(16) uint8_t crc_xmm_shift_tab[32] = {<br>
> +     0x00, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87,<br>
> +     0x88, 0x89, 0x8a, 0x8b, 0x8c, 0x8d, 0x8e, 0x8f,<br>
>       0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,<br>
> -     0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,<br>
> -     0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,<br>
> -     0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff<br>
> +     0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f<br>
>  };<br>
> <br>
>  /**<br>
> @@ -216,19 +205,12 @@ crc32_eth_calc_pclmulqdq(<br>
>                       0x80808080, 0x80808080, 0x80808080, 0x80808080<br>
>               };<br>
> <br>
> -             const alignas(16) uint8_t shf_table[32] = {<br>
> -                     0x00, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87,<br>
> -                     0x88, 0x89, 0x8a, 0x8b, 0x8c, 0x8d, 0x8e, 0x8f,<br>
> -                     0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,<br>
> -                     0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f<br>
> -             };<br>
> -<br>
>               __m128i last16, a, b;<br>
> <br>
>               last16 = _mm_loadu_si128((const __m128i *)&data[data_len - 16]);<br>
> <br>
>               temp = _mm_loadu_si128((const __m128i *)<br>
> -                     &shf_table[data_len & 15]);<br>
> +                     &crc_xmm_shift_tab[data_len & 15]);<br>
>               a = _mm_shuffle_epi8(fold, temp);<br>
> <br>
>               temp = _mm_xor_si128(temp,<br>
> --<br>
> 2.49.1<br>
> <br>
</blockquote></div></div>