Re: PATCH: tree-ssa-sink breaks stack layout

[Daniel Berlin <dberlin at dberlin dot org>]

On Tue, Mar 31, 2009 at 3:27 PM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
> Daniel Berlin wrote:
>>
>> Absolutely none of the optimizations in the middle end ?care about
>> lexical block boundaries, and it the fact that we have the info at all
>> is not meant to be used to block or stop optimizations.
>> In fact, the very first thing GIMPLE lowering does is remove lexical
>> scopes!
>>
>> "/* The differences between High GIMPLE and Low GIMPLE are the
>> ? following:
>>
>> ? 1- Lexical scopes are removed (i.e., GIMPLE_BIND disappears).
>> "
>>
>> ISTM the bug here is in trying to use lexical scope data, after we've
>> destroyed it, to make decisions about aliasing.
>
> Yes, I also considered this. ?I was confused because the current
> lexical-block-based analysis in cfgexpand long predates the new gimple
> optimizations and data structures, so I didn't know which was correct, or if
> the lexical block information still being available and used at that point
> meant it was supposed to be preserved and valid throughout the tree
> optimizations even if it was no longer the primary representation.
Sadly, it's not.

> Plus,
> the stack space sharing optimization is an important one for some
> applications, whereas we'll have another chance to do code motion after we
> convert to RTL; so given a choice between disabling one or the other as a
> quick fix, it seemed better to attack it at the tree-ssa-sink end.


Code motion on RTL has never done, well, much of anything anything but
simple computations.
Also, off the top of my head, loop store motion, loop invariant
motion, PRE, and copy prop can all cause the same problem you are
experiencing fairly easily.

The easiest way out of this i can think of would be to fix
add_alias_set_conflicts.

This calls objects_must_conflict_p, which must be returning 1 in your
case because type based aliasing is off.
The reason it returns 1 is simple:

It calls
int
alias_sets_must_conflict_p (alias_set_type set1, alias_set_type set2)
{

which does

  if (set1 == 0 || set2 == 0 || set1 == set2)
    return 1;

The == 0 part is incorrect, at least for this usage.

If type based aliasing is off (IE at O1), the sets will be 0, which in
this case doesn't mean the sets "must conflict", it means we know
*nothing*, which makes it  "may-conflict".

returning 1 makes it believe these two variables must-alias, which
means they can share stack space, which is what causes your bug.
:)

--Dan