[PATCH] D38088: Fix out-of-order stepping behavior in programs with hoisted constants.

[David Blaikie via llvm-commits]

On Tue, Sep 26, 2017 at 2:05 PM Robinson, Paul <paul.robinson at sony.com>
wrote:

> Doesn't need to be merged with the insertion point, I don't think? Does
> this merge across basic blocks? or just raise the uses of a constant to the
> beginning of a single basic block?
>
>
>
> I read the pass as looking for a dominating insertion point across
> multiple basic blocks.  Doesn't mean the insertion point necessarily is in
> a different block, but I *think* it could be.
>

Yeah, still seems like it might be nice to keep the location if it doesn't
cross a basic block boundary.

Adrian - any ideas/thoughts on all this?


> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> *Sent:* Monday, September 25, 2017 5:06 PM
> *To:* Robinson, Paul;
> reviews+D38088+public+4bc8f3de9212eeb7 at reviews.llvm.org; Voss, Matthew;
> craig.topper at gmail.com; echristo at gmail.com
>
>
> *Cc:* llvm-commits at lists.llvm.org
> *Subject:* Re: [PATCH] D38088: Fix out-of-order stepping behavior in
> programs with hoisted constants.
>
>
>
>
>
> On Fri, Sep 22, 2017 at 12:22 PM Robinson, Paul <paul.robinson at sony.com>
> wrote:
>
> (re-adding llvm-commits)
>
> If this is motivated by SPGO issues - should it be using
> mergeDebugLocations, then (& thus only changing the location if they come
> from different basic blocks)?
>
>
>
> Well, you could consider it a merge of all the uses, but then it should
> also be merged with the insertion point, which is chosen to dominate the
> uses.
>
>
>
> Doesn't need to be merged with the insertion point, I don't think? Does
> this merge across basic blocks? or just raise the uses of a constant to the
> beginning of a single basic block?
>
> If it does work across basic block, then yeah, merging the location may be
> insufficient (since the location could be the same but still be merging
> across basic blocks... - not sure what the best way to detect/handle that
> is)
>
>
>
>   Intuitively it seemed unlikely this would ever produce a real debug
> location.  (Also, in a sense this is an artificially generated instruction
> and from that perspective having no source location is also appropriate.)
>
> Do you want us to pursue the extra bookkeeping?  I'm having a hard time
> imagining this coming up in the wild (i.e., not convinced it's worth the
> effort, but you're a lot better at that than I am).
>
>
> I'd guess at a macro that uses the same constant a few times (in which
> case they'll have the same location) or maybe a single line statement (with
> column info disabled) using the constant a few times/in a few places.
>
> I'd think it'd be worth checking how this works for some simple cases with
> a single basic block, etc. Maybe seeing how often that such a merge would
> fire on a real codebase maybe, etc.
>
> - Dave
>
>
> --paulr
>
>
>
> https://reviews.llvm.org/D38088
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171024/f9bd40cc/attachment.html>