From 134f965b88967f8aef470c89b0c042695cfb7b38 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Wed, 8 Jan 2025 09:33:18 +0100 Subject: [PATCH] panvk: Fix an alignment issue on x86 On x86-32, long long are aligned on 4-bytes only, which breaks the assumption we had about our sysvals struct layouts. Define an aligned_u64 embedding the alignment attribute to keep the alignment sane. While at it, enforce this alignment with an alignment attribute on the struct itself. This fixes the build on x86-32, and should do what we expect, though it's not been tested in practice. Fixes: ae76a6a04596 ("panvk: Pack push constants") Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/12429 Signed-off-by: Boris Brezillon Tested-by: Mikhail Gavrilov Reviewed-by: Erik Faye-Lund Part-of: --- src/panfrost/vulkan/panvk_shader.h | 44 ++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/panfrost/vulkan/panvk_shader.h b/src/panfrost/vulkan/panvk_shader.h index a734f044224..6df60626234 100644 --- a/src/panfrost/vulkan/panvk_shader.h +++ b/src/panfrost/vulkan/panvk_shader.h @@ -51,6 +51,10 @@ enum panvk_desc_table_id { }; #endif +#define FAU_WORD_SIZE sizeof(uint64_t) + +#define aligned_u64 __attribute__((aligned(sizeof(uint64_t)))) uint64_t + struct panvk_graphics_sysvals { struct { float constants[4]; @@ -71,7 +75,7 @@ struct panvk_graphics_sysvals { uint32_t noperspective_varyings; } vs; - uint64_t push_consts; + aligned_u64 push_consts; #if PAN_ARCH <= 7 /* gl_Layer on Bifrost is a bit of hack. We have to issue one draw per @@ -80,10 +84,21 @@ struct panvk_graphics_sysvals { int32_t layer_id; struct { - uint64_t sets[PANVK_DESC_TABLE_GFX_COUNT]; + aligned_u64 sets[PANVK_DESC_TABLE_GFX_COUNT]; } desc; #endif -}; +} __attribute__((aligned(FAU_WORD_SIZE))); + +static_assert((sizeof(struct panvk_graphics_sysvals) % FAU_WORD_SIZE) == 0, + "struct panvk_graphics_sysvals must be 8-byte aligned"); +static_assert((offsetof(struct panvk_graphics_sysvals, push_consts) % + FAU_WORD_SIZE) == 0, + "panvk_graphics_sysvals::push_consts must be 8-byte aligned"); +#if PAN_ARCH <= 7 +static_assert((offsetof(struct panvk_graphics_sysvals, desc) % FAU_WORD_SIZE) == + 0, + "panvk_graphics_sysvals::desc must be 8-byte aligned"); +#endif struct panvk_compute_sysvals { struct { @@ -96,26 +111,31 @@ struct panvk_compute_sysvals { uint32_t x, y, z; } local_group_size; - uint64_t push_consts; + aligned_u64 push_consts; #if PAN_ARCH <= 7 struct { - uint64_t sets[PANVK_DESC_TABLE_COMPUTE_COUNT]; + aligned_u64 sets[PANVK_DESC_TABLE_COMPUTE_COUNT]; } desc; #endif -}; +} __attribute__((aligned(FAU_WORD_SIZE))); + +static_assert((sizeof(struct panvk_compute_sysvals) % FAU_WORD_SIZE) == 0, + "struct panvk_compute_sysvals must be 8-byte aligned"); +static_assert((offsetof(struct panvk_compute_sysvals, push_consts) % + FAU_WORD_SIZE) == 0, + "panvk_compute_sysvals::push_consts must be 8-byte aligned"); +#if PAN_ARCH <= 7 +static_assert((offsetof(struct panvk_compute_sysvals, desc) % FAU_WORD_SIZE) == + 0, + "panvk_compute_sysvals::desc must be 8-byte aligned"); +#endif /* This is not the final offset in the push constant buffer (AKA FAU), but * just a magic offset we use before packing push constants so we can easily * identify the type of push constant (driver sysvals vs user push constants). */ #define SYSVALS_PUSH_CONST_BASE MAX_PUSH_CONSTANTS_SIZE -#define FAU_WORD_SIZE sizeof(uint64_t) - -static_assert((sizeof(struct panvk_compute_sysvals) % FAU_WORD_SIZE) == 0, - "struct panvk_compute_sysvals must be 8-byte aligned"); -static_assert((sizeof(struct panvk_graphics_sysvals) % FAU_WORD_SIZE) == 0, - "struct panvk_graphics_sysvals must be 8-byte aligned"); #define sysval_size(__ptype, __name) \ sizeof(((struct panvk_##__ptype##_sysvals *)NULL)->__name)