[Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation

[Ian Romanick]

On 06/23/2015 02:36 PM, Thomas Helland wrote:
> 2015-06-24 23:05 GMT+02:00 Davin McCall <davmac at davmac.org>:
>> Hi - I'm new here.
>>
>> I've recently started poking the Mesa codebase for little reason other than
>> personal interest. In the "help wanted" section of the website it mentions
>> aliasing violations as a target for newcomers to fix, so with that in mind
>> I've attached a patch (against git head) which resolves a few of them, by
>> targeting the linked list implementation (list.h) used in the GLSL
>> compiler/optimizers. This change slightly increases the storage requirements
>> for a list (adds one word) but resolves the blatant aliasing violation that
>> was caused by the trick used to conserve that word in the first place.
>>
>> (I toyed with another approach - using a single sentinel node for both the
>> head and tail of a list - but this was much more invasive, and meant that
>> you could no longer check whether a particular node was a sentinel node
>> unless you had a reference to the list, so I gave up and went with this
>> simpler approach).
>>
>> The most essential change is in the 'exec_list' structure. Three fields
>> 'head', 'tail' and 'tail_pred' are removed, and two separate sentinel nodes
>> are inserted in their place. The old 'head' is replaced by
>> 'head_sentinel.next', 'tail_pred' by 'tail_sentinel.prev', and tail (always
>> NULL) by 'head_sentinel.prev' and 'tail_sentinel.next' (both always NULL).

NAK.  The datastructure is correct as-is.  It has been in common use
since at least 1985.  See the references in the header file.

>> With this patch, I can build a working (though perhaps not 100% bug-free)
>> Mesa without using -fno-strict-aliasing during compilation. Before the
>> patch, applications using Mesa would hang at runtime.
>>
>> I don't really know the process you have in place so if I need to do
>> anything else to get this patch into the codebase please let me know. Any
>> comments and criticisms welcome.
>>
>> Davin
>>
>>
> 
> Hi Davin,
> 
> Welcome aboard the Mesa-boat!
> I haven't really looked at your patch
> (I'll leave that to someone who knows the code),
> but here's a few pointers on how things work around here.
> (I see Ilia already answered you)
> 
> Have a look at http://www.mesa3d.org/devinfo.html.
> This has most of the info you need on how to send patches
> and how your source code should be formatted.
> 
> We try to limit lines to aprox 78 chars (no more than 80).
> Patches also should have a "location" for the change in the subject.
> (In your patch: [PATCH] glsl: Fix strict-aliasing.......)
> And subjects in present tense, but you got that right =)
> 
> Until you get commit access someone else will have to
> push the patches to upstream for you (usually the reviewer).
> 
> That's a few points of the top of my head.
> If there's anything else feel free to ask.
> I'm not the most experienced guy in here, but I guess I'm at
> least able to answer most of your practical questions =)
> 
> -Thomas
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>