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

[Nicolai Hähnle]

On 27.06.2017 17:02, Marek Olšák wrote:
> On Tue, Jun 27, 2017 at 10:02 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>> On 27.06.2017 00:15, Marek Olšák wrote:
>>>
>>> On Thu, Jun 22, 2017 at 9:19 AM, Nicolai Hähnle <nhaehnle at gmail.com>
>>> wrote:
>>>>
>>>> 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.
>>>
>>>
>>> It decreases the time spent in si_update_shaders by 6% when going from
>>> 128 bytes to 106 for the shader key.
>>>
>>> What do you mean by the risk?
>>
>>
>> If we end up accidentally misaligning a member, that could cause a
>> performance hit. But those 6% improvement does seem worth it. I do think
>> Emil's suggestion of adding the attribute directly to the structures is a
>> good idea. With that, the patch is
> 
> I can't add the attributes inside the structures, because every
> structure member would have to have the attribute, and that would be
> too much noise in the code. There are MSVC and gcc attributes. gcc
> supports both kinds. MSVC attributes are the global #pragmas. gcc
> attributes are per-member or on structure types. If the attribute is
> on a structure type, it only specifies alignment for the whole
> structure, not individual members.

My bad, I thought there was an attribute that controls the packing of 
all members. If there isn't, just go ahead with the patch as-is.

Cheers,
Nicolai

> 
> Marek
> 


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