[Mesa-dev] [PATCH v2] glsls: Modify exec_list to avoid strict-aliasing violations

[Eirik Byrkjeflot Anonsen]

Davin McCall <davmac at davmac.org> writes:

> On 26/06/15 17:08, Eirik Byrkjeflot Anonsen wrote:
>> Davin McCall <davmac at davmac.org> writes:
>>
>>> On 26/06/15 14:31, Eirik Byrkjeflot Anonsen wrote:
>>>> Erik Faye-Lund <kusmabite at gmail.com> writes:
>>>>
>>>>> On Fri, Jun 26, 2015 at 1:23 PM, Davin McCall <davmac at davmac.org> wrote:
>>>>>> On 26/06/15 12:03, Davin McCall wrote:
>>>>>>> ... The stored value of 'n' is not accessed by any other type than the
>>>>>>> type of n itself. This value is then cast to a different pointer type. You
>>>>>>> are mistaken if you think that the cast accesses the stored value of n. The
>>>>>>> other "stored value" access that it occurs in that expression is to the
>>>>>>> object pointed at by the result of the cast. [...]:
>>>>>> I'm sorry, I think that was phrased somewhat abrasively, which I did not
>>>>>> intend. Let me try this part again. If we by break up the expression in
>>>>>> order of evaluation:
>>>>>>
>>>>>> From:
>>>>>>      return ((const struct exec_node **)n)[0]
>>>>>>
>>>>>> In order of evaluation:
>>>>>>
>>>>>> n
>>>>>> - which accesses the stored value of n, i.e. a value of type 'struct exec
>>>>>> node *', via n, which is obviously of that type.
>>>>>>
>>>>>> (const struct exec_node **)n
>>>>>>    - which casts that value, after it has been retrieved, to another type. If
>>>>>> this were an aliasing violation, then casting any pointer variable to
>>>>>> another type would be an aliasing violation; this is clearly not the case.
>>>>>>
>>>>>> ((const struct exec_node **)n)[0]
>>>>>> - which de-references the result of the above cast, thereby accessing a
>>>>>> stored value of type 'exec node *' using a glvalue of type 'exec node *'.
>>>>> I think breaking this up is a mistake, because the strict-aliasing
>>>>> rules is explicitly about the *combination* of these two things.
>>>>>
>>>>> You *are* accessing the underlying memory of 'n' through a different
>>>>> type, and this is what strict aliasing is all about. But it takes two
>>>>> steps, a single step isn't enough to do so.
>>>>>
>>>>> Those other spec-quotes doesn't undo the strict-aliasing definitions;
>>>>> knowing how things are laid out in memory doesn't mean the compiler
>>>>> cannot assume two differently typed variables doesn't overlap.
>>>> So basically, you're saying that e.g.:
>>>>
>>>>      p->next = a;
>>>>      q = exec_node_get_next_const(p);
>>>>
>>>> is equivalent to:
>>>>
>>>>      exec_node * p1 = p;
>>>>      exec_node ** p2 = (exec_node**)p;
>>>>      p1->next = a;
>>>>      q = p2[0];
>>>> [...]
>>>>    Thus the two
>>>> assignments can be "safely" reordered. Sounds plausible to me.
>>> The assignments are to 'p1->next' and 'p2[0]', which *are* the same type
>>> (struct exec_node *) and therefore the assignments *cannot* be
>>> reordered. It is exactly this that I rely on in my patch to resolve the
>>> aliasing issue with the current code.
>> But, on the other hand, if *p1 and *p2 are known to be non-overlapping,
>> it would mean that p1->next and p2[0] are non-overlapping. Thus the two
>> statements can be reordered. Though I'd actually have to carefully read
>> the spec to check whether a compiler would be allowed to do that.
>
> In the example you've give *p1 and *p2 can still overlap, because *p2 is 
> of type 'struct exec_node *' and *p1 is of type 'struct exec_node' which 
> contains members of type 'struct exec_node *'.
>
> But, yes, if it could be shown that they didn't overlap, the two 
> statements could be re-ordered. It's just that in this case, that cannot 
> be shown.
>
>> [...]
>>
>> However, I'm assuming the intent of the rule is that two valid pointers
>> of different types can never point to the same address. In other words,
>> when the compiler sees two pointers of different types, it can assume
>> they point to different memory locations.
>
> There is one exception however (which I referred to above) - if one 
> points at an aggregate type which contains a member whose type is the 
> same as what the other points to.
>
>>
>> Thus, if the compiler thinks:
>>
>>     p->next = a  becomes  "write a to p+0"
>>     b = q[0]     becomes  "read b from q+0"
>>
>> With p and q being known different values (due to having different
>> types), it is clear that the read statement can not be affected by the
>> write statement.
>>
>> Again, I'm not sure that's what the spec actually says, but I suspect
>> strongly that this was the intention of the strict aliasing rules. And
>> that precisely these optimizations is what compilers do when they use
>> the strict aliasing rules.
>
> Right, if you can eliminate the "membership" exception. :)
>
> Davin

Right. That's the loophole I realized would break a lot of stuff after I
sent this example :)

eirik