All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gustavo Sousa <gustavo.sousa@intel.com>
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	intel-xe@lists.freedesktop.org
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
	"Matt Roper" <matthew.d.roper@intel.com>,
	"Violet Monti" <violet.monti@intel.com>,
	"Matthew Brost" <matthew.brost@intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Ashutosh Dixit" <ashutosh.dixit@intel.com>,
	"Mark Brown" <broonie@kernel.org>
Subject: Re: [PATCH] drm/xe/rtp: Fix build error with clang < 21 and non-const initializers
Date: Fri, 5 Jun 2026 10:55:22 -0300	[thread overview]
Message-ID: <87ecilt7lh.fsf@intel.com> (raw)
In-Reply-To: <20260605093305.110598-1-thomas.hellstrom@linux.intel.com>

Thomas Hellström <thomas.hellstrom@linux.intel.com> writes:

> Clang < 21 treats const-qualified compound literals at function scope as
> having static storage duration, which requires all initializer elements
> to be compile-time constants.  When xe_hw_engine.c initializes a local
> struct xe_rtp_table_sr using XE_RTP_TABLE_SR(), the compound literals in
> XE_RTP_TABLE_SR end up containing runtime values (e.g. blit_cctl_val
> derived from gt->mocs.uc_index), triggering:
>
>   xe_hw_engine.c:361: error: initializer element is not a compile-time constant
>   xe_hw_engine.c:416: error: initializer element is not a compile-time constant
>
> ARRAY_SIZE() cannot be used as a replacement because it expands through
> __must_be_array() -> __BUILD_BUG_ON_ZERO_MSG() -> _Static_assert inside
> sizeof(struct{}), which clang < 21 also rejects in the same context.
>
> Replace ARRAY_SIZE() with an open-coded sizeof(arr)/sizeof(elem) in
> XE_RTP_TABLE_SR and XE_RTP_TABLE to avoid both issues.
>
> Fixes: 5ff004fdc737 ("drm/xe/rtp: Add struct types for RTP tables")
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Gustavo Sousa <gustavo.sousa@intel.com>
> Cc: Violet Monti <violet.monti@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Cc: intel-xe@lists.freedesktop.org
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/intel-xe/bfb0dee8-b243-47ba-a89d-71472b0d51c5@sirena.org.uk/
> Assisted-by: GitHub_Copilot:claude-sonnet-4.6
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>  drivers/gpu/drm/xe/xe_rtp.h | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_rtp.h b/drivers/gpu/drm/xe/xe_rtp.h
> index 4e3cfd69f922..2cc65053cd07 100644
> --- a/drivers/gpu/drm/xe/xe_rtp.h
> +++ b/drivers/gpu/drm/xe/xe_rtp.h
> @@ -461,14 +461,22 @@ struct xe_reg_sr;
>  		XE_RTP_PASTE_FOREACH(ACTION_, COMMA, (__VA_ARGS__))	\
>  	}
>  
> +/*
> + * Note: ARRAY_SIZE() cannot be used here because it expands through
> + * __must_be_array() -> __BUILD_BUG_ON_ZERO_MSG() -> _Static_assert inside
> + * sizeof(struct{}), which clang < 21 rejects when the compound literal
> + * contains non-compile-time-constant initializers.
> + */
>  #define XE_RTP_TABLE_SR(...) { \
>  	.entries = (const struct xe_rtp_entry_sr[]){__VA_ARGS__}, \
> -	.n_entries = ARRAY_SIZE(((const struct xe_rtp_entry_sr[]){__VA_ARGS__})), \
> +	.n_entries = sizeof((const struct xe_rtp_entry_sr[]){__VA_ARGS__}) / \
> +		sizeof(struct xe_rtp_entry_sr), \

What would happen if we used

  ARRAY_SIZE(((struct xe_rtp_entry_sr[]){__VA_ARGS__}))

instead?

Anyways, the patch looks correct to me as-is.  Hopefully we'll remember
to go back to use ARRAY_SIZE() when we bump the minimal clang version
required.

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

--
Gustavo Sousa

>  }
>  
>  #define XE_RTP_TABLE(...) { \
>  	.entries = (const struct xe_rtp_entry[]){__VA_ARGS__}, \
> -	.n_entries = ARRAY_SIZE(((const struct xe_rtp_entry[]){__VA_ARGS__})), \
> +	.n_entries = sizeof((const struct xe_rtp_entry[]){__VA_ARGS__}) / \
> +		sizeof(struct xe_rtp_entry), \
>  }
>  
>  #define XE_RTP_PROCESS_CTX_INITIALIZER(arg__) _Generic((arg__),							\
> -- 
> 2.54.0

  parent reply	other threads:[~2026-06-05 13:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-05  9:33 [PATCH] drm/xe/rtp: Fix build error with clang < 21 and non-const initializers Thomas Hellström
2026-06-05 10:49 ` ✗ CI.checkpatch: warning for " Patchwork
2026-06-05 10:50 ` ✓ CI.KUnit: success " Patchwork
2026-06-05 11:29 ` ✓ Xe.CI.BAT: " Patchwork
2026-06-05 13:55 ` Gustavo Sousa [this message]
2026-06-05 14:13   ` [PATCH] " Thomas Hellström
2026-06-05 21:26 ` ✓ Xe.CI.FULL: success for " Patchwork

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=87ecilt7lh.fsf@intel.com \
    --to=gustavo.sousa@intel.com \
    --cc=ashutosh.dixit@intel.com \
    --cc=broonie@kernel.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.brost@intel.com \
    --cc=matthew.d.roper@intel.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=violet.monti@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.