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

[Robinson, Paul via llvm-commits]

@dblaikie does that answer the question?  Sorry I should have pinged this a while ago.
--paulr

From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf Of Robinson, Paul via llvm-commits
Sent: Tuesday, September 26, 2017 2:05 PM
To: David Blaikie; 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.

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.
--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<mailto: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/20171017/9ad1c282/attachment.html>