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

[Michel Dänzer]

On 24.06.2015 06:35, Matt Turner wrote:
> On Wed, Jun 24, 2015 at 2:05 PM, Davin McCall <davmac at davmac.org> wrote:
> 
>> 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).
>>
>> 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.
> 
> Thanks for the patch. I'm impressed that fixing exec_list/exec_node
> allows the removal of -fno-strict-aliasing (at least, we don't know of
> the rest of the bugs yet :).

Actually, I'm almost 100% certain that there are lots of other strict
aliasing violations in the Mesa code. That's why we've always disabled it.

More generally, IMO it's unrealistic to rely on strict aliasing for
optimization, because very few people really understand it (I'm not one
of them).


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer