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

[Adrian Prantl]

> On May 30, 2014, at 3:51 PM, Alexey Samsonov <vonosmas at gmail.com> wrote:
> 
> 
> On Fri, May 30, 2014 at 3:44 PM, Adrian Prantl <aprantl at apple.com> wrote:
> 
> > 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.
> 
> The patch should improve this situation by the way:
> <bbX>:
>   DBG_VALUE $rsp, 32 !"var"  // "var is spilled on stack.
> // ... more code ..
> <bbY>:
>   DBG_VALUE $r9, 0, !"var"  // "var" is now loaded to "r9".
> 
> Earlier we used to terminate the location of "var" at the end of bbX, but now we won't do
> this, as we know "rsp" doesn't change in the function.

Yeah, but only if we can detect the Epilogue and ignore the def of $rsp that restores the register at the end of the function.

>  
> 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.
> 
> Yeah, MMI side table doesn't really work for indirect descriptions, when we need to deref the memory stored at reg+offset.
> 
>  
> >
> >>
> >> -- 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
> 
> 
> 
> 
> -- 
> Alexey Samsonov
> vonosmas at gmail.com