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.
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.