[Mesa-dev] [PATCH 3/3] radeonsi: use #pragma pack to pack si_shader_key

[Nicolai Hähnle]

On 20.06.2017 20:00, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
> 
> sizeof(struct si_shader_key):
>    Before reverting the 2 commits: 120 bytes
>    After reverting the 2 commits: 128 bytes
>    With #pragma pack: 107 bytes
> 
> I'm not sure if memcmp with a byte-aligned size is a good idea.

Does this have a measurable impact? The code is nicer to read with the 
structure after the patches, but I'm not sure it's worth the risk of 
getting misaligned data somewhere.

Nicolai

> ---
>   src/gallium/drivers/radeonsi/si_shader.h | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_shader.h b/src/gallium/drivers/radeonsi/si_shader.h
> index 185bb8d..6432126 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.h
> +++ b/src/gallium/drivers/radeonsi/si_shader.h
> @@ -371,20 +371,25 @@ struct si_shader_selector {
>    * GFX6 - with GS:   ES |     |     | GS | VS | PS
>    *      - with tess: LS | HS  | VS  |    |    | PS
>    *      - with both: LS | HS  | ES  | GS | VS | PS
>    * GFX9 - with GS:   -> |     |     | GS | VS | PS
>    *      - with tess: -> | HS  | VS  |    |    | PS
>    *      - with both: -> | HS  | ->  | GS | VS | PS
>    *
>    * -> = merged with the next stage
>    */
>   
> +/* Use the byte alignment for all following structure members for optimal
> + * shader key memory footprint.
> + */
> +#pragma pack(push, 1)
> +
>   /* Common VS bits between the shader key and the prolog key. */
>   struct si_vs_prolog_bits {
>   	unsigned	instance_divisors[SI_MAX_ATTRIBS];
>   };
>   
>   /* Common TCS bits between the shader key and the epilog key. */
>   struct si_tcs_epilog_bits {
>   	unsigned	prim_mode:3;
>   	unsigned	tes_reads_tess_factors:1;
>   };
> @@ -507,20 +512,23 @@ struct si_shader_key {
>   		/* For shaders where monolithic variants have better code.
>   		 *
>   		 * This is a flag that has no effect on code generation,
>   		 * but forces monolithic shaders to be used as soon as
>   		 * possible, because it's in the "opt" group.
>   		 */
>   		unsigned	prefer_mono:1;
>   	} opt;
>   };
>   
> +/* Restore the pack alignment to default. */
> +#pragma pack(pop)
> +
>   struct si_shader_config {
>   	unsigned			num_sgprs;
>   	unsigned			num_vgprs;
>   	unsigned			spilled_sgprs;
>   	unsigned			spilled_vgprs;
>   	unsigned			private_mem_vgprs;
>   	unsigned			lds_size;
>   	unsigned			spi_ps_input_ena;
>   	unsigned			spi_ps_input_addr;
>   	unsigned			float_mode;
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.