[llvm-dev] [RFC] Removing debug locations from ConstantSDNodes

[Xinliang David Li via llvm-dev]

+Dehao Chen <dehao at google.com>  to comment on the potential impact. In
general, I prefer the debug info is kept when sample PGO is on.

David


On Tue, Jun 19, 2018 at 5:46 PM Vedant Kumar <vsk at apple.com> wrote:

> Some time ago I saw a report of strange single-stepping behavior given
> Swift code that looks like this:
>
> 1|  var input = [16, 14, 10, 9, 8, 7, 4, 3, 2, 1] //< Only number 1 is
> relevant.
> 2|  print("start")
>
> TL;DR: The debugger steps from line 2 -> 1 -> 2 in this code example. One
> clean way to fix this bug is to remove debug locations from
> ConstantSDNodes, and I'm asking if there are any objections to doing that.
>
> In more detail:
>
> The problem appears to be that we assign a debug location to the
> ConstantSDNode for "1". When this constant is used again (as it happens to
> be in the lowered version of the call to print()) the debugger steps back
> to the first use of the constant. Here's a snippet of MIR output that
> illustrates the problem:
>
> %19:gr64 = ADD64ri8 %18, 8, implicit-def dead %eflags; GR64:%19,%18
> dbg:destroy-after-foreach.swift:2:7
> %20:gr32 = MOV32ri64 1; GR32:%20 dbg:destroy-after-foreach.swift:1:44
> %21:gr64 = SUBREG_TO_REG 0, killed %20, sub_32bit; GR64:%21 GR32:%20
> dbg:destroy-after-foreach.swift:1:44
> %rdi = COPY %21; GR64:%21 dbg:destroy-after-foreach.swift:2:7
>
> The out-of-order stepping behavior is confusing and unexpected to users.
> ISTM that the simplest way to fix this bug is to not attach debug locations
> to constant SDNodes. This matches the model used by IR (Constants don't
> have DebugLocs). As an added benefit, it would let us delete a fair amount
> of code which traffics in SDLocs. I'm not sure what (if any) impact this
> would have on sampling-based profilers. SamplePGO, at least, has smoothing
> methods specifically designed to paper over this sort of "out-of-place"
> debug location issue (see
> https://storage.googleapis.com/pub-tools-public-publication-data/pdf/45290.pdf,
> Section 4.1.4, "Sources of Profile Inaccuracies").
>
> I brought all of this up a while ago on IRC: some feedback was positive,
> but I didn't get a clear +1 or -1 to proceed.
>
> Please let me know what you think, if there are cleaner alternatives to
> consider, etc. Any comments/feedback would be appreciated.
>
> Someone (Reid?) mentioned that we could try sinking constants to their
> point of first use as an alternative, and (IIUC) create new nodes with
> distinct DebugLocs for each use of a constant. I don't recall the details
> of that alternative clearly. Based on my (likely incorrect) understanding
> of it, dropping locations from constants outright might be simpler.
>
> thanks,
> vedant
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180619/a3131a07/attachment.html>