[PATCH] Generate better location ranges for some register-described variables.

[Adrian Prantl]

> On May 30, 2014, at 3:40 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> On Fri, May 30, 2014 at 3:37 PM, Adrian Prantl <aprantl at apple.com> wrote:
>> 
>>> On May 30, 2014, at 3:31 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>> 
>>> On Fri, May 30, 2014 at 3:12 PM, Eric Christopher <echristo at gmail.com> wrote:
>>>> On Fri, May 30, 2014 at 3:03 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>>> On Fri, May 30, 2014 at 3:00 PM, Alexey Samsonov <vonosmas at gmail.com> wrote:
>>>>>> 
>>>>>> On Fri, May 30, 2014 at 12:45 PM, Eric Christopher <echristo at gmail.com>
>>>>>> wrote:
>>>>>>> 
>>>>>>> On Fri, May 30, 2014 at 10:26 AM, Adrian Prantl <aprantl at apple.com> wrote:
>>>>>>>> 
>>>>>>>>> On May 29, 2014, at 10:03 PM, Alexey Samsonov <vonosmas at gmail.com>
>>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>> Yeah, well, we can add "FrameTeardown" machine instruction flag in
>>>>>>>>> addition to "FrameSetup", and annotate machine instructions added in frame
>>>>>>>>> lowering code. But do we expect many users of this flag (assuming that this
>>>>>>>>> usage is also somewhat questionable)?
>>>>>>>> 
>>>>>>>> The FrameSetup flag is also only used by the debug info, so at least it
>>>>>>>> wouldn’t be any more questionable than that :-)
>>>>>> 
>>>>>> 
>>>>>> OK. Why don't we land the patch as-is now (don't terminate the register at
>>>>>> the end of MBB if this register is only modified in frame setup and the last
>>>>>> MBB - it should be safe), and then replace "the last MBB" condition with
>>>>>> checking a "FrameTeardown" flag, attached to certain machine instructions?
>>>>>> In this way we won't have to special-case certain registers and make
>>>>>> assumptions about their constant-ness.
>>>>> 
>>>>> Given the limited nature of this fix (both in the fix's possible scope
>>>>> and intent/purpose) I'm personally sort of inclined to do this by
>>>>> whitelisting certain registers instead, if that were/is possible...
>>>>> seems simpler and still sufficient for the required purpose here.
>>>>> 
>>>>> Can we easily just test whether a register is the stack/frame pointer?
>>>>> (I actually know so little about this that the difference between
>>>>> those two things isn't straight in my head and I'm not sure which, or
>>>>> both, we need for the common/intended case here)
>>>>> 
>>>> 
>>>> If we need to stack realign we might not be able to do this as easily.
>>>> I'm not a huge fan of either method, but I think that the assumption
>>>> that Alexey has might make more sense than whitelisting.
>>> 
>>> Fair enough.
>>> 
>>>> My logic:
>>>> Whitelisting involves assuming that various registers won't be used in
>>>> ways that surprise us and I can think of a few ways where it could
>>>> happen: stack realignment, inline asm, a smarter stack coloring
>>>> algorithm.
>>> 
>>> Can inline asm clobber the frame pointer & we actually tolerate that,
>>> find the right variables, etc, when that happens? (how?)
>>> 
>>> Can't quite picture the stack coloring you have in mind & Alexey
>>> commented on the stack realignment issue.
>>> 
>>> The only other idea I'd float would be another simpler solution,
>>> though a bit more expressive: just a check to see that the particular
>>> register(s) (stack/frame pointers, whichever things it is we actually
>>> use to describe the location of stack variables) don't change beyond
>>> the prologue, and if not, describe any variables that are relative to
>>> that register as having that location for the whole program.
>> 
>> That won’t work for values that are spilled to the stack, because they only live on the stack for part of the function.
> 
> Do we get those right currently?

Partially. We get them right until the end of the current basic block.
But there is no code that would insert DBG_VALUES after they are reloaded again.

> 
>> [And for (-O0) allocas that are described in the MMI table, we already do what you described].
> 
> Right - but the problem Alexey is trying to address is what ASan does,
> where it makes one big alloca and describes variables as being within
> that alloca - so the MMI sidetable handling doesn't fire…

Awright.
> 
>> 
>> -- adrian
>> 
>>> 
>>> But, yeah, that's not /lots/ easier than finding any registers that
>>> happen to meet that property.
>>> 
>>> - David
>>> 
>>>> 
>>>> -eric
>>>> 
>>>>>> 
>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> :)
>>>>>>> 
>>>>>>>>> I can also make this patch even less general and just special-case the
>>>>>>>>> frame pointer and the stack pointer registers, so that the code wouldn't
>>>>>>>>> pretend to solve a general problem we're facing.
>>>>>>>> 
>>>>>>>> The frame pointer should be stable over the entire function, special
>>>>>>>> casing it seems to be appropriate. I don’t know whether, e.g, stack coloring
>>>>>>>> would cause the stack pointer to be modified in the middle of a function.
>>>>>>>> 
>>>>>>> 
>>>>>>> FWIW it may not _now_ but there's nothing from stopping it either. :\
>>>>>>> 
>>>>>>> -eric
>>>>>> 
>>>>>> 
>>>>>> --
>>>>>> Alexey Samsonov
>>>>>> vonosmas at gmail.com