On Fri, 15 Oct 2021 10:30:02 +0100
Vladimir Medvedkin <vladimir.medved...@intel.com> wrote:

> +                     m[i * 8 + j] = (rss_key[i] << j)|
> +                             (uint8_t)((uint16_t)(rss_key[i + 1]) >>
> +                             (8 - j));
> +             }

This ends up being harder than necessary to read. Maybe split into
multiple statements and/or use temporary variable.

> +RTE_INIT(rte_thash_gfni_init)
> +{
> +     rte_thash_gfni_supported = 0;

Not necessary in C globals are initialized to zero by default.

By removing that the constructor can be totally behind #ifdef

> +__rte_internal
> +static inline __m512i
> +__rte_thash_gfni(const uint64_t *mtrx, const uint8_t *tuple,
> +     const uint8_t *secondary_tuple, int len)
> +{
> +     __m512i permute_idx = _mm512_set_epi8(7, 6, 5, 4, 7, 6, 5, 4,
> +                                             6, 5, 4, 3, 6, 5, 4, 3,
> +                                             5, 4, 3, 2, 5, 4, 3, 2,
> +                                             4, 3, 2, 1, 4, 3, 2, 1,
> +                                             3, 2, 1, 0, 3, 2, 1, 0,
> +                                             2, 1, 0, -1, 2, 1, 0, -1,
> +                                             1, 0, -1, -2, 1, 0, -1, -2,
> +                                             0, -1, -2, -3, 0, -1, -2, -3);

NAK

Please don't put the implementation in an inline. This makes it harder
to support (API/ABI) and blocks other architectures from implementing
same thing with different instructions.

Reply via email to