This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Wed, Apr 1, 2009 at 6:00 PM, Daniel Berlin <dberlin@dberlin.org> wrote:
> On Wed, Apr 1, 2009 at 11:52 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Wed, Apr 1, 2009 at 5:40 PM, Daniel Berlin <dberlin@dberlin.org> wrote:
>>> On Wed, Apr 1, 2009 at 10:08 AM, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> On Wed, Apr 1, 2009 at 3:58 PM, Daniel Berlin <dberlin@dberlin.org> wrote:
>>>>> On Wed, Apr 1, 2009 at 5:12 AM, Richard Guenther
>>>>> <richard.guenther@gmail.com> wrote:
>>>>>> On Wed, Apr 1, 2009 at 2:27 AM, Daniel Berlin <dberlin@dberlin.org> wrote:
>>>>>>> On Tue, Mar 31, 2009 at 6:04 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>>>>>>>> Well, no, it is a *real* bug that it claims the two objects
>>>>>>>>> must-conflict and decides they can share a space when we have no
>>>>>>>>> informatin available to substantiate this
>>>>>>>>
>>>>>>>> We're at -O1 so it's true that the objects must-conflict in the alias.c sense.
>>>>>>>
>>>>>>> alias.c uses conflict as a synonym for the word "alias" in almost all
>>>>>>> places, and there is no documentation elsewhere, so uh, claiming i'm
>>>>>>> "changing the definition" seems a bit much, that said ...
>>>>>>>
>>>>>>>> The code thinks that if the blocks are different and the objects must-conflict
>>>>>>>> then it's enough to conclude that the slots can be shared. ?That's wrong.
>>>>>>>>
>>>>>>>> You seem to be proposing to change the definition of must-conflict;
>>>>>>>> ?it seems
>>>>>>>> to me (and that was the conclusion of the discussion in PR middle-end/32327)
>>>>>>>> that the problem is rather in the "if the blocks are different".
>>>>>>> This is independent of the fact that it's clearly missing an aliasing
>>>>>>> conflict above.
>>>>>>> Then again, I have no dog in this fight, I can happily wait till you
>>>>>>> fix this with live range info and then discover it's still borked
>>>>>>> because it is using alias information wrong :)
>>>>>>
>>>>>> This alias sets are conflicting check was added for the very same reason - to
>>>>>> paper over bugs that may appear otherwise due to RTL scheduling of loads/stores
>>>>>> which may make life-ranges of stack variables which share their stack slot
>>>>>> overlapping. ?The theory is that the scheduler won't do such thing if the
>>>>>> alias sets conflict (which is of course not exactly true if you can disambiguate
>>>>>> using offsets, so you can still get overlapping lifetime of a whole struct just
>>>>>> not individual parts ... if that will break anything is another question).
>>>>>>
>>>>>
>>>>> All true, but look at this way:
>>>>>
>>>>> At O1, objects_must_conflict is returning 1 (causing it to not add an
>>>>> aliasing conflict and share the slots)
>>>>> At O2, objects_must_conflict is returning 0 (causing it to add an
>>>>> aliasing conflict and not share the slots)
>>>>>
>>>>> Either the objects conflict or they don't. Conservatively, it needs to
>>>>> return the answer that *doesn't* allow sharing, which it clearly is
>>>>> *not* doing.
>>>>
>>>> The naming may be completely bogus, but this code only adds
>>>> "conflicts" in the sense that two variables that conflict may not share
>>>> stack slots.
>>>
>>> Yes, so when it doesn't have type-based info to the contrary, it
>>> should be saying they may not share stack slots, instead, it says they
>>> can.
>>
>> Well, it also doesn't have type-based info to later break in the face
>> of stack slot sharing, so it says "safe!". ?No longer true if we start to
>> use PTA info or any other clever non-TBAA disambiguation of course.
>
> I completely agree that nothing later than stack slot sharing should
> be breaking this sharing in the face of the aliasing info it has.
> However, my point is that stack slot sharing *itself* is breaking it
> by saying it can share them ?based on this aliasing info :)

It doesn't break because it bases it on this aliasing info but because
it bases it on outdated scope block information.

Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]