DPDK patches and discussions
 help / color / mirror / Atom feed
From: Konstantin Ananyev <konstantin.ananyev@huawei.com>
To: David Marchand <david.marchand@redhat.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: "bluca@debian.org" <bluca@debian.org>,
	"stable@dpdk.org" <stable@dpdk.org>,
	Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>,
	"David Christensen" <drc@linux.ibm.com>,
	Bruce Richardson <bruce.richardson@intel.com>,
	Wathsala Vithanage <wathsala.vithanage@arm.com>
Subject: RE: [PATCH] acl: fix build with GCC 15 on aarch64
Date: Thu, 27 Mar 2025 10:30:39 +0000	[thread overview]
Message-ID: <182c1c9cc7714a649a8e098d7311b74f@huawei.com> (raw)
In-Reply-To: <20250326103928.1189243-1-david.marchand@redhat.com>



> 
> Caught in OBS for Fedora Rawhide on aarch64:
> 
> [  198s] In file included from ../lib/acl/acl_run_neon.h:7,
> [  198s]                  from ../lib/acl/acl_run_neon.c:5:
> [  198s] In function ‘alloc_completion’,
> [  198s]     inlined from ‘acl_start_next_trie’ at
> 	../lib/acl/acl_run.h:140:24,
> [  198s]     inlined from ‘search_neon_4.isra’ at
> 	../lib/acl/acl_run_neon.h:239:20:
> [  198s] ../lib/acl/acl_run.h:93:25: error: ‘cmplt’ may be used
> 	uninitialized [-Werror=maybe-uninitialized]
> [  198s]    93 |                 if (p[n].count == 0) {
> [  198s]       |                     ~~~~^~~~~~
> [  198s] ../lib/acl/acl_run_neon.h: In function ‘search_neon_4.isra’:
> [  198s] ../lib/acl/acl_run_neon.h:230:27: note: ‘cmplt’ declared here
> [  198s]   230 |         struct completion cmplt[4];
> [  198s]       |                           ^~~~~
> 
> The code was resetting sequentially cmpl[].count at the exact index that
> later call to alloc_completion uses.
> While this code seems correct, GCC 15 does not understand this (probably
> when applying some optimisations).
> 
> Instead, reset cmpl[].count all at once in acl_set_flow, and cleanup the
> various vectorized implementations accordingly.
> 
> Bugzilla ID: 1678
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/acl/acl_run.h         | 5 +++++
>  lib/acl/acl_run_altivec.h | 8 ++------
>  lib/acl/acl_run_avx2.h    | 4 +---
>  lib/acl/acl_run_neon.h    | 8 ++------
>  lib/acl/acl_run_scalar.c  | 4 +---
>  lib/acl/acl_run_sse.h     | 8 ++------
>  6 files changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/lib/acl/acl_run.h b/lib/acl/acl_run.h
> index 7f092413cd..9fd3e60021 100644
> --- a/lib/acl/acl_run.h
> +++ b/lib/acl/acl_run.h
> @@ -176,6 +176,8 @@ acl_set_flow(struct acl_flow_data *flows, struct completion *cmplt,
>  	uint32_t cmplt_size, const uint8_t **data, uint32_t *results,
>  	uint32_t data_num, uint32_t categories, const uint64_t *trans)
>  {
> +	unsigned int i;
> +
>  	flows->num_packets = 0;
>  	flows->started = 0;
>  	flows->trie = 0;
> @@ -187,6 +189,9 @@ acl_set_flow(struct acl_flow_data *flows, struct completion *cmplt,
>  	flows->data = data;
>  	flows->results = results;
>  	flows->trans = trans;
> +
> +	for (i = 0; i < cmplt_size; i++)
> +		cmplt[i].count = 0;
>  }
> 
>  typedef void (*resolve_priority_t)
> diff --git a/lib/acl/acl_run_altivec.h b/lib/acl/acl_run_altivec.h
> index 2d398ffded..d5ccdb94f0 100644
> --- a/lib/acl/acl_run_altivec.h
> +++ b/lib/acl/acl_run_altivec.h
> @@ -199,10 +199,8 @@ search_altivec_8(const struct rte_acl_ctx *ctx, const uint8_t **data,
>  	acl_set_flow(&flows, cmplt, RTE_DIM(cmplt), data, results,
>  		total_packets, categories, ctx->trans_table);
> 
> -	for (n = 0; n < MAX_SEARCHES_ALTIVEC8; n++) {
> -		cmplt[n].count = 0;
> +	for (n = 0; n < MAX_SEARCHES_ALTIVEC8; n++)
>  		index_array[n] = acl_start_next_trie(&flows, parms, n, ctx);
> -	}
> 
>  	 /* Check for any matches. */
>  	acl_match_check_x4(0, ctx, parms, &flows, (uint64_t *)&index_array[0]);
> @@ -270,10 +268,8 @@ search_altivec_4(const struct rte_acl_ctx *ctx, const uint8_t **data,
>  	acl_set_flow(&flows, cmplt, RTE_DIM(cmplt), data, results,
>  		total_packets, categories, ctx->trans_table);
> 
> -	for (n = 0; n < MAX_SEARCHES_ALTIVEC4; n++) {
> -		cmplt[n].count = 0;
> +	for (n = 0; n < MAX_SEARCHES_ALTIVEC4; n++)
>  		index_array[n] = acl_start_next_trie(&flows, parms, n, ctx);
> -	}
> 
>  	/* Check for any matches. */
>  	acl_match_check_x4(0, ctx, parms, &flows, index_array);
> diff --git a/lib/acl/acl_run_avx2.h b/lib/acl/acl_run_avx2.h
> index 0b8967f22e..e069fb85b2 100644
> --- a/lib/acl/acl_run_avx2.h
> +++ b/lib/acl/acl_run_avx2.h
> @@ -171,10 +171,8 @@ search_avx2x16(const struct rte_acl_ctx *ctx, const uint8_t **data,
>  	acl_set_flow(&flows, cmplt, RTE_DIM(cmplt), data, results,
>  		total_packets, categories, ctx->trans_table);
> 
> -	for (n = 0; n < RTE_DIM(cmplt); n++) {
> -		cmplt[n].count = 0;
> +	for (n = 0; n < RTE_DIM(cmplt); n++)
>  		index_array[n] = acl_start_next_trie(&flows, parms, n, ctx);
> -	}
> 
>  	t0 = _mm256_set_epi64x(index_array[5], index_array[4],
>  		index_array[1], index_array[0]);
> diff --git a/lib/acl/acl_run_neon.h b/lib/acl/acl_run_neon.h
> index 63074f871d..3b9bd0cc39 100644
> --- a/lib/acl/acl_run_neon.h
> +++ b/lib/acl/acl_run_neon.h
> @@ -172,10 +172,8 @@ search_neon_8(const struct rte_acl_ctx *ctx, const uint8_t **data,
>  	acl_set_flow(&flows, cmplt, RTE_DIM(cmplt), data, results,
>  		     total_packets, categories, ctx->trans_table);
> 
> -	for (n = 0; n < 8; n++) {
> -		cmplt[n].count = 0;
> +	for (n = 0; n < 8; n++)
>  		index_array[n] = acl_start_next_trie(&flows, parms, n, ctx);
> -	}
> 
>  	 /* Check for any matches. */
>  	acl_match_check_x4(0, ctx, parms, &flows, &index_array[0]);
> @@ -234,10 +232,8 @@ search_neon_4(const struct rte_acl_ctx *ctx, const uint8_t **data,
>  	acl_set_flow(&flows, cmplt, RTE_DIM(cmplt), data, results,
>  		     total_packets, categories, ctx->trans_table);
> 
> -	for (n = 0; n < 4; n++) {
> -		cmplt[n].count = 0;
> +	for (n = 0; n < 4; n++)
>  		index_array[n] = acl_start_next_trie(&flows, parms, n, ctx);
> -	}
> 
>  	/* Check for any matches. */
>  	acl_match_check_x4(0, ctx, parms, &flows, index_array);
> diff --git a/lib/acl/acl_run_scalar.c b/lib/acl/acl_run_scalar.c
> index 3d61e79409..a3661b1b6b 100644
> --- a/lib/acl/acl_run_scalar.c
> +++ b/lib/acl/acl_run_scalar.c
> @@ -121,10 +121,8 @@ rte_acl_classify_scalar(const struct rte_acl_ctx *ctx, const uint8_t **data,
>  	acl_set_flow(&flows, cmplt, RTE_DIM(cmplt), data, results, num,
>  		categories, ctx->trans_table);
> 
> -	for (n = 0; n < MAX_SEARCHES_SCALAR; n++) {
> -		cmplt[n].count = 0;
> +	for (n = 0; n < MAX_SEARCHES_SCALAR; n++)
>  		index_array[n] = acl_start_next_trie(&flows, parms, n, ctx);
> -	}
> 
>  	transition0 = index_array[0];
>  	transition1 = index_array[1];
> diff --git a/lib/acl/acl_run_sse.h b/lib/acl/acl_run_sse.h
> index 93286a2c38..4ec819a215 100644
> --- a/lib/acl/acl_run_sse.h
> +++ b/lib/acl/acl_run_sse.h
> @@ -205,10 +205,8 @@ search_sse_8(const struct rte_acl_ctx *ctx, const uint8_t **data,
>  	acl_set_flow(&flows, cmplt, RTE_DIM(cmplt), data, results,
>  		total_packets, categories, ctx->trans_table);
> 
> -	for (n = 0; n < MAX_SEARCHES_SSE8; n++) {
> -		cmplt[n].count = 0;
> +	for (n = 0; n < MAX_SEARCHES_SSE8; n++)
>  		index_array[n] = acl_start_next_trie(&flows, parms, n, ctx);
> -	}
> 
>  	/*
>  	 * indices1 contains index_array[0,1]
> @@ -293,10 +291,8 @@ search_sse_4(const struct rte_acl_ctx *ctx, const uint8_t **data,
>  	acl_set_flow(&flows, cmplt, RTE_DIM(cmplt), data, results,
>  		total_packets, categories, ctx->trans_table);
> 
> -	for (n = 0; n < MAX_SEARCHES_SSE4; n++) {
> -		cmplt[n].count = 0;
> +	for (n = 0; n < MAX_SEARCHES_SSE4; n++)
>  		index_array[n] = acl_start_next_trie(&flows, parms, n, ctx);
> -	}
> 
>  	indices1 = _mm_loadu_si128((xmm_t *) &index_array[0]);
>  	indices2 = _mm_loadu_si128((xmm_t *) &index_array[2]);
> --

LGTM. Also run a quick test on my box - all seems ok. 

Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
Tested-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
 

> 2.48.1


  parent reply	other threads:[~2025-03-27 10:30 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-26 10:39 David Marchand
2025-03-27  8:17 ` David Marchand
2025-03-27  8:55 ` Bruce Richardson
2025-03-27 10:36   ` David Marchand
2025-03-27 10:39     ` Konstantin Ananyev
2025-03-27 10:51       ` Bruce Richardson
2025-03-27 11:17         ` Morten Brørup
2025-03-27 12:10           ` Konstantin Ananyev
2025-03-27 12:24             ` Bruce Richardson
2025-03-27 12:43               ` Konstantin Ananyev
2025-03-27 12:30             ` Morten Brørup
2025-03-27 10:30 ` Konstantin Ananyev [this message]
2025-03-27 18:06 ` Bruce Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=182c1c9cc7714a649a8e098d7311b74f@huawei.com \
    --to=konstantin.ananyev@huawei.com \
    --cc=bluca@debian.org \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=drc@linux.ibm.com \
    --cc=konstantin.v.ananyev@yandex.ru \
    --cc=stable@dpdk.org \
    --cc=wathsala.vithanage@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).