Extending debug ranges

[tvvikram]

Summary

This patch is an early implementation to extend debug ranges and provide multiple location support for debug variables. This work is based on the idea put forward in the mail thread: http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-June/087109.html

A. Debug Range Extension: Currently, the debug ranges in LLVM end prematurely at the end of every basic block. This patch tries to extend them across basic blocks such that when two predecessors of a basic block have the same variable residing in the same location, a new debug value representing the variable and the location is added at the start of the current basic block.

Example: Consider the following partial code containing only DBG_VALUE instructions:
BB#3:
.
DBG_VALUE %EBX, %noreg, !"n", <!17>; line no:8
.
JMP_1 <BB#5>

BB#4:
.
DBG_VALUE %EBX, %noreg, !"n", <!17>; line no:8
.

BB#5: ; preds - BB#3, BB#4
DBG_VALUE %EBX, %noreg, !"n", <!17>; line no:8 <-- newly inserted
.

Here, BB#5 is a successor of both BB#3 and BB#4. Source variable "n" resides in %EBX on both paths. So a new DBG_VALUE instruction will be added at the start of BB#5 for "n" at location %EBX.

B. Multiple locations: Moved to http://reviews.llvm.org/D11986.

[loladiro]

Adding a few more people who I'd like to take a look and comment. For background, this work is being done as part of our effort to improve debug info quality in julia. There is still some work left to do, but I'd like to gather feedback on implementation/design early, as well as make people aware that this is being worked on.

[dblaikie]

+samsonov, who built DbgValueHistoryCalculator

It might be helpful to have a bit of a high level design describing how the
patch achieves this (you could go through & write comments in the code
review to describe the various parts - if they're not worthy of
sufficiently descriptive inline comments in the source itself)

Is it reasonable/possible/helpful to split the two (A and B, in your
original description) features into separate patches to simplify review
(smaller patches are exponentially easier/faster to review)?

[aprantl]

First off, thanks for working on this!

The patch as it is doesn't apply cleanly (see DbgValueHistoryMap::print) and 243841 is a CFE revision...?

[friss]

This patch is an early implementation to extend debug ranges and provide multiple location support for debug variables. This work is based on the idea put forward in the mail thread: http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-June/087109.html

Thank you so much for working on this! I just skimmed through the patch and description. Some questions bellow:

A. Debug Range Extension: Currently, the debug ranges in LLVM end prematurely at the end of every basic block. This patch tries to extend them across basic blocks such that when two predecessors of a basic block have the same variable residing in the same location, a new debug value representing the variable and the location is added at the start of the current basic block.

I think there is an interesting corner case here. I believe that constant values should have a special treatment. When you have code like:

int i = 42;
while (i) i =foo(i)

the DBG_VALUE for i will have a constant on one edge into the loop and a register on the other one. In that scenario, in case a constant competes with a dynamic location at BB entry, I strongly believe the dynamic loc should win.

Also, was it really easier to write a different pass to do the data flow propagation? I saw in the patch that it is because you want to insert new instructions, but couldn't it be done without inserting the instructions, just keeping some temporary data structures up-to-date? I can see some appeal in minimally modifying the current code and reviewing the new pass as a different thing though.

B. Multiple locations: LLVM currently can represent a debug variable only at a single location. This patch provides an infrastructure for earlier passes to express multiple locations for a debug variable by adding a new flag - PreserveDbgValLoc to the DBG_VALUE instruction. The presence of the flag will mean multiple debug ranges for the variable can co-exist together. The patch also infers multiple locations by looking at move instructions.

Could that be split, maybe even multiple times (split out of this patch, then the move inference part)? Why is the flag needed rather than tracking multiple locations unconditionally.
One thing that we discussed with Adrian some time ago is that emitting multiple locations isn't really useful for the debuggers we now, they will just use one location. Thus the generic code shouldn't maybe emit multiple locations as it will cost file size and IO time for nothing.

Example - inferring from move instruction: Consider the following two instructions:

DBG_VALUE %EDI, %noreg, !"n", <!12>
%EBX<def> = MOV32rr %EDI
DBG_VALUE %EBX, %noreg, !"n", <!12>; flags: PreserveDbgValLoc  <-- newly inserted

After the copy instruction, the value “n” will be present in two locations - %EDI and %EBX and the same is represented with the new DBG_VALUE instruction having the flag PreserveDbgValLoc set.

The two ranges continue to exist together until one of the following happens:
Either %EDI or %EBX is clobbered, the two ranges are conservatively ended
A DBG_VALUE instruction for variable “n” without the flag is seen, when the previous ranges are ended.

Test cases: 3 new test cases are added to test the above implementations. There are numerous places in the test cases where debug range extension and addition of PreserveDbgValLoc happens. All test cases under 'ninja check' run, with around 5 miscompare errors. I will correct them once the implementation is okay as the miscompares are volatile with code change.

SVN base revision used: 243841

One thing I'm missing is results. You propose the patch, so I suppose this approach works for you. Could you give some details on what works better, eg does it fix all the issues you were seeing? Also, what's the compile time impact of that change and the effect on binary size?

Fred

[probinson]

In D11933#221911, @friss wrote:

One thing that we discussed with Adrian some time ago is that emitting multiple locations isn't really useful for the debuggers we now, they will just use one location. Thus the generic code shouldn't maybe emit multiple locations as it will cost file size and IO time for nothing.

This is a chicken-and-egg problem. I think the compiler needs to take the lead here, so that debuggers will have something to experiment with and take advantage of. Actual debug-info-size data would be worth having, if it seems excessive this could be put under some kind of flag. The primary size cost would be in recording the addresses to delimit the additional ranges.

[aprantl]

I didn't have a chance to actually play with the code so far (and the patch didn't apply cleanly), but here are some very high level comments and questions:

• Why did you choose to insert DBG_VALUEs into the machine function instead of implementing it directly inside DbgValueHistoryCalculator?
• Having it as a separate pass could actually be good for testability — did you look into using the brand new MIR serialization format for writing MIR->MIR testcases?
• There should also be a test case with an aggregate variable that is split across multiple DW_OP_pieces to make sure they are handled correctly.
• Did you run any benchmarks with the new pass? Does it cause any measurable slowdown?

The MultipleLocations handling should probably be split out into a separate patch and reviewed separately.

thanks,
adrian

lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp

135 ↗

(On Diff #31781)

Comment should be above the return.

lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.h

41 ↗

(On Diff #31781)

Just a general remark: Please follow
http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments
and use /// doxygen comments without \brief where possible.

lib/CodeGen/DebugValueFixup.cpp

1 ↗

(On Diff #31781)

Fixup is not a really descriptive name. Really it inserts additional DEBUG_VALUES to extend the live ranges of debug locations. ExtendDebugLocationLiveRanges, DebugValueLiveRangeExtender, ...?

15 ↗

(On Diff #31781)

Please elaborate.

19 ↗

(On Diff #31781)

Should be rephrased as "This is a separate pass from DbgValueHistoryCalculator because....".

[tvvikram]

Sharing a document about design/implementation details: https://docs.google.com/document/d/1noDVWTvTWBdYdweICPBwvwyt8QvX4KHl7j3XKNSg1nE/edit?usp=sharing

[tvvikram]

lib/CodeGen/DebugValueFixup.cpp

1 ↗

(On Diff #31781)

I will rename it to ExtendDebugValueRangeAndLocation.cpp?

[tvvikram]

In D11933#221791, @dblaikie wrote:

+samsonov, who built DbgValueHistoryCalculator

It might be helpful to have a bit of a high level design describing how the
patch achieves this (you could go through & write comments in the code
review to describe the various parts - if they're not worthy of
sufficiently descriptive inline comments in the source itself)

Is it reasonable/possible/helpful to split the two (A and B, in your
original description) features into separate patches to simplify review
(smaller patches are exponentially easier/faster to review)?

I am separating the patches.

[tvvikram]

In D11933#221911, @friss wrote:

This patch is an early implementation to extend debug ranges and provide multiple location support for debug variables. This work is based on the idea put forward in the mail thread: http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-June/087109.html

Thank you so much for working on this! I just skimmed through the patch and description. Some questions bellow:

A. Debug Range Extension: Currently, the debug ranges in LLVM end prematurely at the end of every basic block. This patch tries to extend them across basic blocks such that when two predecessors of a basic block have the same variable residing in the same location, a new debug value representing the variable and the location is added at the start of the current basic block.

I think there is an interesting corner case here. I believe that constant values should have a special treatment. When you have code like:

int i = 42;
while (i) i =foo(i)

the DBG_VALUE for i will have a constant on one edge into the loop and a register on the other one. In that scenario, in case a constant competes with a dynamic location at BB entry, I strongly believe the dynamic loc should win.

Currently, both are treated as separate locations and we do nothing about it.

Also, was it really easier to write a different pass to do the data flow propagation? I saw in the patch that it is because you want to insert new instructions, but couldn't it be done without inserting the instructions, just keeping some temporary data structures up-to-date? I can see some appeal in minimally modifying the current code and reviewing the new pass as a different thing though.

buildLocationLists() need ranges with real instructions. So we had to insert a new instruction.

B. Multiple locations: LLVM currently can represent a debug variable only at a single location. This patch provides an infrastructure for earlier passes to express multiple locations for a debug variable by adding a new flag - PreserveDbgValLoc to the DBG_VALUE instruction. The presence of the flag will mean multiple debug ranges for the variable can co-exist together. The patch also infers multiple locations by looking at move instructions.

Could that be split, maybe even multiple times (split out of this patch, then the move inference part)? Why is the flag needed rather than tracking multiple locations unconditionally.

Flag is provided so that prior passes can set it to convey that a variable exists at a location along with previous locations, if any.

One thing that we discussed with Adrian some time ago is that emitting multiple locations isn't really useful for the debuggers we now, they will just use one location. Thus the generic code shouldn't maybe emit multiple locations as it will cost file size and IO time for nothing.

Example - inferring from move instruction: Consider the following two instructions:

DBG_VALUE %EDI, %noreg, !"n", <!12>
%EBX<def> = MOV32rr %EDI
DBG_VALUE %EBX, %noreg, !"n", <!12>; flags: PreserveDbgValLoc  <-- newly inserted

After the copy instruction, the value “n” will be present in two locations - %EDI and %EBX and the same is represented with the new DBG_VALUE instruction having the flag PreserveDbgValLoc set.

The two ranges continue to exist together until one of the following happens:
Either %EDI or %EBX is clobbered, the two ranges are conservatively ended
A DBG_VALUE instruction for variable “n” without the flag is seen, when the previous ranges are ended.

Test cases: 3 new test cases are added to test the above implementations. There are numerous places in the test cases where debug range extension and addition of PreserveDbgValLoc happens. All test cases under 'ninja check' run, with around 5 miscompare errors. I will correct them once the implementation is okay as the miscompares are volatile with code change.

SVN base revision used: 243841

One thing I'm missing is results. You propose the patch, so I suppose this approach works for you. Could you give some details on what works better, eg does it fix all the issues you were seeing? Also, what's the compile time impact of that change and the effect on binary size?

Fred

[tvvikram]

In D11933#222154, @aprantl wrote:

I didn't have a chance to actually play with the code so far (and the patch didn't apply cleanly), but here are some very high level comments and questions:

• Why did you choose to insert DBG_VALUEs into the machine function instead of implementing it directly inside DbgValueHistoryCalculator?

I have written a brief note about it at start of DebugValueFixup.cpp.

• Having it as a separate pass could actually be good for testability — did you look into using the brand new MIR serialization format for writing MIR->MIR testcases?

I am quite new to MIR serialization. I will try.

• There should also be a test case with an aggregate variable that is split across multiple DW_OP_pieces to make sure they are handled correctly.

Will try to come up with one.

• Did you run any benchmarks with the new pass? Does it cause any measurable slowdown?

I have not tried any benchmarks as of yet.

The MultipleLocations handling should probably be split out into a separate patch and reviewed separately.

Creating 2 new patches.

thanks,
adrian

[tvvikram]

I have created 2 new patches. Should I create a new review or should I update in the current review itself?

[aprantl]

Just update this review with the basic patch and create a new one for the multivalue handling and put a links to the other one into the description.

thanks!

[tvvikram]

This patch contains only Extending Debug Range implementation. The patch on multiple location support for debug variables will be a new review as suggested.

[tvvikram]

In D11933#222508, @tvvikram wrote:

In D11933#222154, @aprantl wrote:

• There should also be a test case with an aggregate variable that is split across multiple DW_OP_pieces to make sure they are handled correctly.

Will try to come up with one.

There were pieces-*.ll tests under test/DebugInfo/ that handled aggregate variables. I have corrected them to match the new correct ranges that get generated.

[aprantl]

Have you had a chance to benchmark the new pass? (Compiling clang with optimizations+debug info would be a good start)

Here's a test case involving constants to play with:

void g(int *);
int f() {
  int x = 23;
  g(&x);
  if (x == 42)
    ++x;
  return x;
}

Compiled with -O1 I get:

define i32 @f() #0 {
entry:
  %x = alloca i32, align 4
  %0 = bitcast i32* %x to i8*, !dbg !14
  call void @llvm.lifetime.start(i64 4, i8* %0) #1, !dbg !14
  tail call void @llvm.dbg.value(metadata i32 23, i64 0, metadata !9, metadata !15), !dbg !16
  store i32 23, i32* %x, align 4, !dbg !16, !tbaa !17
  tail call void @llvm.dbg.value(metadata i32* %x, i64 0, metadata !9, metadata !15), !dbg !16
  call void @g(i32* nonnull %x) #1, !dbg !21
  call void @llvm.dbg.value(metadata i32* %x, i64 0, metadata !9, metadata !15), !dbg !16
  %1 = load i32, i32* %x, align 4, !dbg !22, !tbaa !17
  %cmp = icmp eq i32 %1, 42, !dbg !24
  br i1 %cmp, label %if.then, label %if.end, !dbg !25

if.then:                                          ; preds = %entry
  %inc = add nsw i32 %1, 1, !dbg !26
  call void @llvm.dbg.value(metadata i32 %inc, i64 0, metadata !9, metadata !15), !dbg !16
  store i32 %inc, i32* %x, align 4, !dbg !26, !tbaa !17
  br label %if.end, !dbg !26

if.end:                                           ; preds = %if.then, %entry
  call void @llvm.dbg.value(metadata i32* %x, i64 0, metadata !9, metadata !15), !dbg !16
  %2 = load i32, i32* %x, align 4, !dbg !27, !tbaa !17
  call void @llvm.lifetime.end(i64 4, i8* %0) #1, !dbg !28
  ret i32 %2, !dbg !29
}

and then

# *** IR Dump After DBG_VALUE Fixup Pass ***:
# Machine code for function f: Post SSA
Frame Objects:
  fi#-1: size=8, align=16, fixed, at location [SP-8]
  fi#0: size=4, align=4, at location [SP-12]

BB#0: derived from LLVM BB %entry
    Live Ins: %RBP
	PUSH64r %RBP<kill>, %RSP<imp-def>, %RSP<imp-use>; flags: FrameSetup
	CFI_INSTRUCTION <call frame instruction>
	CFI_INSTRUCTION <call frame instruction>
	%RBP<def> = MOV64rr %RSP; flags: FrameSetup
	CFI_INSTRUCTION <call frame instruction>
	%RSP<def,tied1> = SUB64ri8 %RSP<tied0>, 16, %EFLAGS<imp-def,dead>; flags: FrameSetup dbg:var.c:3:7
	DBG_VALUE 23, 0, !"x", <!15>; line no:3
	MOV32mi %RBP, 1, %noreg, -4, %noreg, 23; mem:ST4[%x](tbaa=!18) dbg:var.c:3:7
	%RDI<def> = LEA64r %RBP, 1, %noreg, -4, %noreg
	DBG_VALUE %RDI, 0, !"x", <!15>; line no:3 indirect
	CALL64pcrel32 <ga:@g>, <regmask>, %RSP<imp-use>, %RDI<imp-use>, %RSP<imp-def>; dbg:var.c:4:3
	%EAX<def> = MOV32rm %RBP, 1, %noreg, -4, %noreg; mem:LD4[%x](tbaa=!18) dbg:var.c:5:7
	CMP32ri8 %EAX, 42, %EFLAGS<imp-def>; dbg:var.c:5:7
	JNE_1 <BB#2>, %EFLAGS<imp-use>
    Successors according to CFG: BB#1(16) BB#2(16)

BB#1: derived from LLVM BB %if.then
    Live Ins: %EAX %RBP
    Predecessors according to CFG: BB#0
	DBG_VALUE %RDI, 0, !"x", <!15>; line no:3 indirect
	DBG_VALUE 23, 0, !"x", <!15>; line no:3
	%EAX<def,tied1> = INC32r %EAX<kill,tied0>, %EFLAGS<imp-def,dead>; dbg:var.c:6:5
	DBG_VALUE %EAX, %noreg, !"x", <!15>; line no:3
	MOV32mr %RBP, 1, %noreg, -4, %noreg, %EAX<kill>; mem:ST4[%x](tbaa=!18) dbg:var.c:6:5
    Successors according to CFG: BB#2

BB#2: derived from LLVM BB %if.end
    Live Ins: %RBP
    Predecessors according to CFG: BB#0 BB#1
	DBG_VALUE 23, 0, !"x", <!15>; line no:3
	%EAX<def> = MOV32rm %RBP, 1, %noreg, -4, %noreg; mem:LD4[%x](tbaa=!18) dbg:var.c:7:10
	%RSP<def,tied1> = ADD64ri8 %RSP<tied0>, 16, %EFLAGS<imp-def,dead>; dbg:var.c:7:3
	%RBP<def> = POP64r %RSP<imp-def>, %RSP<imp-use>; dbg:var.c:7:3
	RETQ %EAX; dbg:var.c:7:3

# End machine code for function f.

Note how the constant value of x is propagated into the last basic block.

[tvvikram]

In D11933#224598, @aprantl wrote:

Have you had a chance to benchmark the new pass? (Compiling clang with optimizations+debug info would be a good start)

I have not done any benchmarking as of now. I will try to add few "STATISTIC" code which will count the number of DBG_VALUE instrs inserted/removed.

[tvvikram]

I compiled your test case with clang -O1 and here is the input to DBG_VALUE fixup pass (output of StackMapLivenessAnalysis).

# *** IR Dump After StackMap Liveness Analysis ***:
# Machine code for function f: Post SSA
Frame Objects:
  fi#0: size=4, align=4, at location [SP-4]

BB#0: derived from LLVM BB %entry
  PUSH64r %RAX<undef>, %RSP<imp-def>, %RSP<imp-use>; flags: FrameSetup dbg:r.c:3:7
  CFI_INSTRUCTION <call frame instruction>
  DBG_VALUE 23, 0, !"x", <!14>; line no:3
  MOV32mi %RSP, 1, %noreg, 4, %noreg, 23; mem:ST4[%x](tbaa=!17) dbg:r.c:3:7
  %RDI<def> = LEA64r %RSP, 1, %noreg, 4, %noreg
  DBG_VALUE %RDI, 0, !"x", <!14>; line no:3 indirect
  CALL64pcrel32 <ga:@g>, <regmask>, %RSP<imp-use>, %RDI<imp-use>, %RSP<imp-def>; dbg:r.c:4:3
  %EAX<def> = MOV32rm %RSP, 1, %noreg, 4, %noreg; mem:LD4[%x](tbaa=!17) dbg:r.c:5:7
  CMP32ri8 %EAX, 42, %EFLAGS<imp-def>; dbg:r.c:5:7
  JNE_1 <BB#2>, %EFLAGS<imp-use>
    Successors according to CFG: BB#1(16) BB#2(16)

BB#1: derived from LLVM BB %if.then
    Live Ins: %EAX
    Predecessors according to CFG: BB#0
  DBG_VALUE 23, 0, !"x", <!14>; line no:3
  %EAX<def,tied1> = INC32r %EAX<kill,tied0>, %EFLAGS<imp-def,dead>; dbg:r.c:6:5
  DBG_VALUE %EAX, %noreg, !"x", <!14>; line no:3
  MOV32mr %RSP, 1, %noreg, 4, %noreg, %EAX<kill>; mem:ST4[%x](tbaa=!17) dbg:r.c:6:5
    Successors according to CFG: BB#2

BB#2: derived from LLVM BB %if.end
    Predecessors according to CFG: BB#0 BB#1
  DBG_VALUE 23, 0, !"x", <!14>; line no:3
  %EAX<def> = MOV32rm %RSP, 1, %noreg, 4, %noreg; mem:LD4[%x](tbaa=!17) dbg:r.c:7:10
  %RDX<def> = POP64r %RSP<imp-def>, %RSP<imp-use>; dbg:r.c:7:3
  RETQ %EAX; dbg:r.c:7:3

# End machine code for function f.

The DBG_VALUE instr for "x" is already present in BB#2 and only "x" in RDI got propagated to BB#1.

With respect to handling variables having constant value, an early thought is to have ranges for them until the variable is moved to register. When the variable is moved to register, it will end all previous ranges of that variable because the move to register will mean that it may be operated on and the value may change. The current patch handles only register locations.

[tvvikram]

lib/CodeGen/DebugValueFixup.cpp

2 ↗

(On Diff #31969)

Is it okay to name the pass as "ExtendDebugValueRangeAndLocation" ?

test/DebugInfo/extend-debug-range.ll

2

I am checking the generated machine instructions after the new pass to check for propagated debug ranges. But llc will also generate the .s file. Now, I tried using -stop-after option to llc, but it still generates the .s file.

[arphaman]

Hi, Sorry about not replying earlier last week,

I hope that you will utilize the new MIR serialization format to test the new pass. I added an inline comment that should explain how you can use the new format to create a test case for this pass.

Cheers,
Alex

test/DebugInfo/extend-debug-range.ll

2

If you would like to test just the individual DebugValueFixup pass, you can use the new 'run-pass' option in llc. The run-pass option expects an MIR file as input. MIR files contain the serialized machine instructions. I'm still working on the documentation for this format, but you can take a look at the current docs at http://llvm.org/docs/MIRLangRef.html.

To generate an input MIR file for this test, you can use the 'stop-after' option with llc. You can pass the current 'extend-debug-range.ll' file directly to it and llc will print the MIR into stdout. You have to specify a name of a pass that runs before the DebugValueFixup for the 'stop-after' option. I see that the DebugValueFixup pass should run after the StackMapLivenessAnalysis pass, so you can obtain the MIR input file by running the following command:

llc -stop-after stackmap-liveness extend-debug-range.ll -o /dev/null > debug-value-fixup.mir

The generated MIR input file will be target dependent, so you will have to place the test in the appropriate target directory in test/CodeGen.

After you've generated an input MIR file, you can transform it into an actual test that tests your pass.
You can use a RUN line similar to this:

RUN: llc -o /dev/null -run-pass=dbgval-fixup -march=??? %s | FileCheck %s

When running this test case, llc will print the produced MIR with the machine instructions that were transformed by your pass directly to stdout.

[llvm-commits]

• msg-22402-242.txt162 BDownload

[tvvikram]

This patch has the following changes over the previous one:

a. Renamed DebugValueFixup pass as ExtendDebugRangeLocation pass.

b. Improved removal of redundant DBG_VALUE instructions. Added a test case for the same - redundant-dbg_val.ll

c. Add "STATISTIC" to count the number of DBG_VALUE instructions inserted/deleted. I am not sure what benchmarks to exactly test.

d. Updated the test case extend-debug-range.ll to compare against assembly instead of machine instructions. I am getting an error in using MIR serialization, I can send a new patch containing the two new MIR test cases once it works.

test/DebugInfo/extend-debug-range.ll

2

I tried the above commands but I keep getting the following error with the second command:

$llc -run-pass=extend-dbg-range-loc -march=x86-64 d.mir -o /dev/null
error: d.mir:172:76: expected an implicit register operand 'implicit-def %eflags'
    dead %eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags, implicit-def %al, debug-location !25

[Please note that the pass has been renamed to extend-dbg-range-loc]

[friss]

c. Add "STATISTIC" to count the number of DBG_VALUE instructions inserted/deleted. I am not sure what benchmarks to exactly test.

The kind of numbers that I (and Adrian I'm pretty sure) wanted to see are more compiler performance numbers. You add a new pass that will have a cost, and we care a lot about -O0 -g compile times. So what is the cost of your new code in terms of compile time?

[tvvikram]

In D11933#226659, @friss wrote:

c. Add "STATISTIC" to count the number of DBG_VALUE instructions inserted/deleted. I am not sure what benchmarks to exactly test.

The kind of numbers that I (and Adrian I'm pretty sure) wanted to see are more compiler performance numbers. You add a new pass that will have a cost, and we care a lot about -O0 -g compile times. So what is the cost of your new code in terms of compile time?

I used clang -O0 -g followed by llc -O0 -time-passes and noticed that the new pass takes around 0.5% - 0.7% of Wall time.

[tvvikram]

Added an .mir test case under test/DebugInfo/MIR/

[llvm-commits]

• msg-26625-658.txt162 BDownload

[arphaman]

Thanks!

test/DebugInfo/MIR/extend-debug-range.mir

1 ↗

(On Diff #32512)

This test is target specific, so you have to add the -march or -mtriple option to llc's parameters.

The test should also be placed in a target specific subdirectory, something like 'DebugInfo/MIR/X86' will work well, but feel free to use another one. This target specific test directory also needs special check in the 'lit.local.cfg' file to make sure that the tests in that directory run only when the target is available. For X86, you can use a condition like this:

if not 'X86' in config.root.targets:
    config.unsupported = True

[arphaman]

test/DebugInfo/MIR/extend-debug-range.mir

249 ↗

(On Diff #32512)

I tweaked the syntax for the memory operands slightly while working on the fix for the problem you reported, so this instruction should be as follows now:

MOV32mr %rip, 1, _, @m, _, %ecx, debug-location !37 :: (store 4 into @m)

[llvm-commits]

• msg-28664-214.txt162 BDownload

[tvvikram]

Updated .mir test cases and added them under test/DebugInfo/MIR/X86

[tvvikram]

Ping!

[aprantl]

In D11933#225701, @tvvikram wrote:

I compiled your test case with clang -O1 and here is the input to DBG_VALUE fixup pass (output of StackMapLivenessAnalysis).

The DBG_VALUE instr for "x" is already present in BB#2 and only "x" in RDI got propagated to BB#1.

Looking into this I found that LiveDebugVariables is the culprit here:

void UserValue::extendDef(SlotIndex Idx, unsigned LocNo,
                          LiveRange *LR, const VNInfo *VNI,
                          SmallVectorImpl<SlotIndex> *Kills,
                          LiveIntervals &LIS, MachineDominatorTree &MDT,
                          UserValueScopes &UVS) {
    ...
    for (unsigned i = 0, e = Children.size(); i != e; ++i) {
      MachineBasicBlock *MBB = Children[i]->getBlock();
      if (UVS.dominates(MBB))
        Todo.push_back(LIS.getMBBStartIdx(MBB));
  }
}

It look like it just unconditionally propagates all DBG_VALUEs down the dominator tree, which happens to work fine if there already is another DBG_VALUE but is wrong if the DBG_VALUE is coming from only one of the predecessors like in this example here.

I filed this as PR24563 (https://llvm.org/bugs/show_bug.cgi?id=24563).

With respect to handling variables having constant value, an early thought is to have ranges for them until the variable is moved to register. When the variable is moved to register, it will end all previous ranges of that variable because the move to register will mean that it may be operated on and the value may change. The current patch handles only register locations.

That sounds reasonable. Memory locations should be treated similarly, too.

[tvvikram]

In D11933#231628, @aprantl wrote:

In D11933#225701, @tvvikram wrote:

I compiled your test case with clang -O1 and here is the input to DBG_VALUE fixup pass (output of StackMapLivenessAnalysis).

The DBG_VALUE instr for "x" is already present in BB#2 and only "x" in RDI got propagated to BB#1.

Looking into this I found that LiveDebugVariables is the culprit here:

void UserValue::extendDef(SlotIndex Idx, unsigned LocNo,
                          LiveRange *LR, const VNInfo *VNI,
                          SmallVectorImpl<SlotIndex> *Kills,
                          LiveIntervals &LIS, MachineDominatorTree &MDT,
                          UserValueScopes &UVS) {
    ...
    for (unsigned i = 0, e = Children.size(); i != e; ++i) {
      MachineBasicBlock *MBB = Children[i]->getBlock();
      if (UVS.dominates(MBB))
        Todo.push_back(LIS.getMBBStartIdx(MBB));
  }
}

It look like it just unconditionally propagates all DBG_VALUEs down the dominator tree, which happens to work fine if there already is another DBG_VALUE but is wrong if the DBG_VALUE is coming from only one of the predecessors like in this example here.

I filed this as PR24563 (https://llvm.org/bugs/show_bug.cgi?id=24563).

Okay!

With respect to handling variables having constant value, an early thought is to have ranges for them until the variable is moved to register. When the variable is moved to register, it will end all previous ranges of that variable because the move to register will mean that it may be operated on and the value may change. The current patch handles only register locations.

That sounds reasonable. Memory locations should be treated similarly, too.

Okay. Is it fine if this implementation is submitted as a separate patch? The current patch handles only register locations.

[aprantl]

The LLVM project generally encourages smaller, incremental patches, as they tend to be easier to review and test.

[aprantl]

I just commented in the PR:

The debug range extension (http://reviews.llvm.org/D11933) is partially redundant with what LiveDebugVariables is trying to do here. The range extension pass is a full data flow analysis — this means we could simplify LiveDebugVariables and have it no longer attempt to extend the liveness of a DEBUG_VALUE beyond the end of a basic block.

It's possible that simplifying LiveDebugVariables could make up some of the runtime that is spent in the debug range extension pass.

[tvvikram]

In D11933#232146, @aprantl wrote:

I just commented in the PR:

The debug range extension (http://reviews.llvm.org/D11933) is partially redundant with what LiveDebugVariables is trying to do here. The range extension pass is a full data flow analysis — this means we could simplify LiveDebugVariables and have it no longer attempt to extend the liveness of a DEBUG_VALUE beyond the end of a basic block.

It's possible that simplifying LiveDebugVariables could make up some of the runtime that is spent in the debug range extension pass.

I added a comment in PR (https://llvm.org/bugs/show_bug.cgi?id=24563#c3)

[tvvikram]

Please let me know if the patch requires any more changes.

[aprantl]

Thanks so far. I added a bunch of more detailed comments inline.

lib/CodeGen/ExtendDebugRangeLocation.cpp

334

Please remove anything multiple locations from this patch it makes reviewing more complicated than necessary.

337

Maybe: Terminate all open ranges at the end of the current basic block.

354

remove these comments.

357

Doxygen (see above)
Also the comment should mention that this joins the analysis results of all incoming edges in a basic block.

360

I don't understand the first sentence.

394

range-based for?

405

drop "currently"

444

transfer

test/DebugInfo/MIR/X86/extend-debug-range.mir

102

We usually drop all non-essential attributes from IR testcases (usually everything in quotes).

[aprantl]

lib/CodeGen/ExtendDebugRangeLocation.cpp

2

Ranges are an implementation detail and probably shouldn't show up in the name.

11

This should ideally be a doxygen comment.

11

Instead of saying "Fixup", better say:
This pass implements a data flow analysis that propagates debug location information by inserting additional DBG_VALUE instructions into the machine instruction stream.

13

"This pass internally builds debug location liveness ranges to determine the points where additional DBG_VALUEs need to be inserted."?

17

Please only document what the code does and don't talk about future extensions.

19

???

22

This is not a sound argument. If this were implement in DbgValueHistoryCalculator we wouldn't need to emit any DBG_VALUE intrinsics, because we could just pass the ranges themselves to the backend.

Let's just say that this is a separate pass to facilitate testing and improve modularity.

58

LLVM build with autobrief, so unless you want more than the first sentence to appear in the brief section, the \brief can/should be omitted from the Doxygen comments.

79

///

81

For better readability it might be worth considering using a struct with self-explaining named members instead of an opaque std::pair.

83

Same here.

140

Full sentences ending with "." are preferred. That said, this comment essentially repeats the function name... what it should say is that it expects DebugValues as inputs.

160

Better: \return the instruction following \c MI.

162

This could be split out into a separate patch and/or tested separately.

180

Could this be written as a ranged-based for?

191

Please run the path through clang-format.

239

missing . at the end

240

range-based for?

251

remove the comment.

263

doxygen

281

doxygen. (the coding guidelines prefer only a single comment on the declaration, so they don't get out of sync).

282

please remove references to multiple locations in this patch

290

Why is this correct?

307

Are constants and memory locations handled in a safe/correct way?

313

The enclosing function is very long. I suggest splitting out certain pieces into separate functions with self-explanatory names so a reader can understand what the function does on a higher level. This looks like a good candidate for that.

[tvvikram]

In D11933#233887, @aprantl wrote:

Thanks so far. I added a bunch of more detailed comments inline.

I will make the suggested changes and submit a new patch.

I am not sure about the name for the new pass. Please suggest.
I have a couple of names though - ImproveDebugValues.cpp, DebugValueDFA.cpp (DFA: DataFlow Analysis). Please confirm.

[aprantl]

In D11933#234164, @tvvikram wrote:

In D11933#233887, @aprantl wrote:

Thanks so far. I added a bunch of more detailed comments inline.

I will make the suggested changes and submit a new patch.

I am not sure about the name for the new pass. Please suggest.
I have a couple of names though - ImproveDebugValues.cpp, DebugValueDFA.cpp (DFA: DataFlow Analysis). Please confirm.

To prevent you from having to rename the file and pass more than once, let's defer this decision until the very end.
Another possible name is "DebugValuePropagation", although "LiveDebugValues", or "DebugValueLiveness" would actually be the most fitting, because we are doing a Liveness analysis. This conflicts with the existing LiveDebugVariables pass that consumes all DBG_VALUEs that describe vregs, builds a side-table that is used during register allocation and writes out new DBG_VALUEs with real registers after regalloc is done. Perhaps we could rename the old LiveDebugVariables to "MaterializeDebugValues", or "RegallocDebugValues?

To recap, my vision is that:

• the old LiveDebugVariables becomes a simple and fast BB-local pass that emits the bare minimum of DBG_VALUEs and
• this new pass then performs the control-flow-aware liveness DFA that extends their range beyond BB boundaries by inserting enough DBG_VALUEs that
• the DWARF backend may again assume that DBG_VALUEs are BB-local (and only performs a very simple merging of adjacent ranges).

This way we will end up with small, understandable, and (via MIR) testable passes with a well-defined scope that do one task well.

[tvvikram]

Made review comment changes.

[tvvikram]

In D11933#234313, @aprantl wrote:

In D11933#234164, @tvvikram wrote:

In D11933#233887, @aprantl wrote:

Thanks so far. I added a bunch of more detailed comments inline.

I will make the suggested changes and submit a new patch.

I am not sure about the name for the new pass. Please suggest.
I have a couple of names though - ImproveDebugValues.cpp, DebugValueDFA.cpp (DFA: DataFlow Analysis). Please confirm.

To prevent you from having to rename the file and pass more than once, let's defer this decision until the very end.

Thanks.

Another possible name is "DebugValuePropagation", although "LiveDebugValues", or "DebugValueLiveness" would actually be the most fitting, because we are doing a Liveness analysis. This conflicts with the existing LiveDebugVariables pass that consumes all DBG_VALUEs that describe vregs, builds a side-table that is used during register allocation and writes out new DBG_VALUEs with real registers after regalloc is done. Perhaps we could rename the old LiveDebugVariables to "MaterializeDebugValues", or "RegallocDebugValues?

The new pass could be a place to do generic code changes related to DBG_VALUE instructions like inferring multiple locations, inserting missing DBG_VALUE instructions if possible, etc. If that is the case, I think we should name the pass with a generic name. Otherwise, I am okay with the name changes you suggested.

To recap, my vision is that:

• the old LiveDebugVariables becomes a simple and fast BB-local pass that emits the bare minimum of DBG_VALUEs and
• this new pass then performs the control-flow-aware liveness DFA that extends their range beyond BB boundaries by inserting enough DBG_VALUEs that
• the DWARF backend may again assume that DBG_VALUEs are BB-local (and only performs a very simple merging of adjacent ranges).

This way we will end up with small, understandable, and (via MIR) testable passes with a well-defined scope that do one task well.

Thanks for the high level view.

lib/CodeGen/ExtendDebugRangeLocation.cpp

17

Removed.

19

Removed. Initially, this file was meant to handle any code changes related to DBG_VALUE instructions.

160

I am not sure about it. MI could already be the last instr in MBB. Anyway, I have removed this routine for now.

162

Removed. I will submit a separate patch for this.

180

I was not updating the iterators after erasing 'u' at line 221. I have rewritten the iterators but could not convert it to 'range-based for'.

290

Unintended comment. Removed.

307

AFAIK, yes as only register locations are handled now.

334

Sorry. I had missed out removing this when I separated out the multiple locations patch. Removed.

360

Removed. I meant that the current implementation support bbs with at most 2 predecessors.

394

The comparison at line 397 prevented me in converting to 'range based for'.

[tvvikram]

Ping!

[aprantl]

Thanks, more comments inline!

Side note, the clang coding standards recommend to write every comment in the code as complete sentences, with a "." at the end.

lib/CodeGen/ExtendDebugRangeLocation.cpp

81

OpenRanges should probably be a local variable in the while (!BBWorklist.empty()) loop in ExtendRanges().

300

This loop is quadratic and only works for two predecessors. Off the top of my head, what about, e.g, having a SmallDenseMap that stores a separate IncomingLocs for each MBB?

IncomingLocs could be a sorted SmallVector or another SmallDenseMap of

struct IncomingLoc {
  DIVariable* Var;
  MachineInstr Loc;
};

A nullptr for Loc (or a missing entry) means that this value has been determined to be unknown.
Additionally each MBB gets a flag that indicates whether it has been visited or not.

At the end of each MBB, go through the list of successors and immediately join() the OutgoingLocs with the IncomingLocs of the successor. In the join function every input wins over the IncomingLocs of a not-yet-visited MBB, after the first visit the MBB is marked as visited. Likewise, a nullptr (or whatever we use to indicate the kill value) also wins against any other value.

360

Ok. See my other comment about generalizing the algorithm.

362

Typically in LLVM transfer functions return bool and this would be written as:

bool Changed = false;
Changed |= transfer(MI)

[tvvikram]

In D11933#240499, @aprantl wrote:

Thanks, more comments inline!

Side note, the clang coding standards recommend to write every comment in the code as complete sentences, with a "." at the end.

Thanks. I will make the changes and submit a new patch.

[aprantl]

Thanks! Any updates?

[tvvikram]

In D11933#270473, @aprantl wrote:

Thanks! Any updates?

Sorry, I couldn't work on this for quite sometime. I will update and submit the patch as soon as possible.

[tvvikram]

First, sorry for submitting this after a long gap.

The following changes have been made:
a. Make OpenRanges usage local to every mbb.
b. Rewrite 'join' routine - generic implementation by looking at all predecessors of an mbb to extend the debug value ranges across mbb by using IncomingLocs as suggested in the review comment. Previously, the implementation was limited to 1 and 2 predecessors.

[tvvikram]

lib/CodeGen/ExtendDebugRangeLocation.cpp

302

I have maintained InLocs for every MBB. These InLocs will also represent the set of DBG_VALUE instructions already inserted. At every 'join' call, the incoming locations are calculated progressively over the predecessors of the MBB (as suggested). Any new DBG_VALUE instruction calculated is inserted at the start of the MBB as well as to the InLocs of this MBB.

[aprantl]

Thanks of the update, this is starting to look really good. I have a couple of comments inline that are mostly about readability. I'll play with the patch and look at the correctness next. Did you make any compile time measurements with the new algorithm? For example, by compiling clang?

lib/CodeGen/ExtendDebugRangeLocation.cpp

68

I found InlinedVariable to be misleading because the Inlined-At is optional. What about just calling it a DebugVariable and promoting it to a struct?

/// A potentially inlined instance of a variable.
struct DebugVar {
  const DILocalVariable &Var;
  const DILocation *InlinedAt;
}

80

This should probably be a (Small)DenseSet (=hash map) instead of a std::map.

88

Instead of having them as members that are cleared for each function, it would be better to make them local variables that are explicitly passed as arguments. It's less error-prone and makes the code easier to read because there is no global state.

200

This entire loop could be replaced by a use of std::remove_if with the condition in a lambda. I believe it would be more readable this way.

223

Again, this can be expressed more compactly with std::remove_if.

257

Since this comparison is duplicated three times why not implement an operator== for VarLoc?

306

With the operator== (see line 256) this pattern can probably also be collapsed to a std::remove_if.

test/DebugInfo/X86/sret.ll

7

I believe, we actually want the dwo_id to be hardcoded in this one testcase, because it forces us to think why the hash changed, when it changed.

[tvvikram]

I have made the suggested changes. In few of the test cases I checked using llc --time-passes, the time spent in this pass was <1%.

[tvvikram]

test/DebugInfo/X86/sret.ll

7

I had noticed that the id had changed in one of previous patches. Fortunately, the id getting generated now is same as in trunk. So I have reverted this change in this patch.

[aprantl]

I just manually verified that this indeed works as advertised on a couple of old bugreports with debug info + ASAN. Totally awesome!
Couple more inline comments and I think the only major thing left to do here is to agree on a name for the new pass :-)

It worries me a bit that none of the test cases exhibit the case with multiple predecessors that the join() function is implementing (nested ifs without else, switch stmts...). It would be good to add one.

lib/CodeGen/ExtendDebugRangeLocation.cpp

183

What if MI is, e.g., constant 1 and V.MI is constant 2? Looks like the function would still return true because isDescribedByReg will return 0 of both. We should also compare the kind (const/reg/...) here.

Regression/Unit test that catches these corner cases?

285

This can now be converted to a range-based for.

340

Why is it necessary to push it into InLocs here? Wouldn't transfer pick it up automatically from the DBG_VALUE that we just inserted?

387

This can now be converted to a range-based for.

[tvvikram]

In D11933#292339, @aprantl wrote:

I just manually verified that this indeed works as advertised on a couple of old bugreports with debug info + ASAN. Totally awesome!

Good to hear that!

Couple more inline comments and I think the only major thing left to do here is to agree on a name for the new pass :-)

I am okay with any of the names that you had suggested before. Please confirm. Also I will submit the current changes along with pass renaming.

It worries me a bit that none of the test cases exhibit the case with multiple predecessors that the join() function is implementing (nested ifs without else, switch stmts...). It would be good to add one.

I have come up with a test case that does join() with 3 predecessors. Will submit along with pass renaming.

lib/CodeGen/ExtendDebugRangeLocation.cpp

183

As the TODO in line 203 says, I am handling DBG_VALUEs that describes a register. OpenRanges when built are guaranteed to describe a register.

340

InLocs also represent the list of already inserted DBG_VALUEs; because we do not want to insert redundant DBG_VALUEs. Only new incoming locs that are calculated during each join() call for the MBB are inserted.

[aprantl]

My vote is to call the pass LiveDbgValues. This is analogous to the well known "live variables" data-flow analysis and thus succinctly describes what this pass is doing (Compute the set of live DBG_VALUEs at each basic block and insert them).

Unless somebody else objects IMO this pass is good to land then.
On the road ahead there are several important things that need to be done in short order, but can/should be done in separate commits:

• Remove the broken propagation in LiveDebugVariables (https://llvm.org/bugs/show_bug.cgi?id=24563) to fix the correctness issue noted in PR24563. This will also reclaim some of the performance we are spending in this new pass.
• DbgValueHistoryCalculator contains a hack (collectChangingRegs()) to identify the frame pointer. With this pass in place, we should be able to remove it and rely on DBG_VALUEs without any loss of functionality.
• Implement the missing location types (constant values, memory locations, fp constants ...)

thanks for all your work,
adrian

[tvvikram]

Renamed the pass as "LiveDebugValues".

[aprantl]

This is ready to land now. Thanks again!

[tvvikram]

Commit rL255096. Thank you!

[jfb]

Hi Vikram,

This seems to break WebAssembly tests:

https://build.chromium.org/p/client.wasm.llvm/builders/linux/builds/726/steps/steps/logs/stdio

Other buildbots also look very sad:

http://lab.llvm.org:8011/console

Could you look into it?

Thanks,

JF

[mehdi_amini]

reverted r255101

[tvvikram]

@joker.eph Thanks for reverting.
@jfb I am looking into it.

[tvvikram]

I got to know that I had built and tested only for certain targets (sorry that I didn't realize about it earlier). I had to make a couple of minor fixes to make the patch work for all targets.

Just to clarify, I am using cmake+Ninja build with the the following command to configure cmake:
cmake -G Ninja -DCMAKE_BUILD_TYPE="Debug" -DLLVM_TARGETS_TO_BUILD="all" -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="WebAssembly" -DLLVM_BINUTILS_INCDIR=/usr/include -DBUILD_SHARED_LIBS=1 -DCMAKE_CXX_FLAGS=" -gdwarf-2 " /path/to/llvmsrc/

and got the following result with "ninja check":
[46/46] Running the LLVM regression tests

• Testing: 15343 tests, 8 threads --

Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
Testing Time: 224.82s

Expected Passes    : 15157
Expected Failures  : 131
Unsupported Tests  : 55

I got similar results with Release build too. Please let me know if the results look fine and if I need to run any more tests before committing.

[mehdi_amini]

This sounds reasonable to me.
We can't test every possible configuration before committing anyway (Windows, Linux, OS X, etc.), this is why we revert easily.

[aprantl]

Please go ahead and re-commit.

[tvvikram]

Recommit: rL255759

include/llvm/CodeGen/Passes.h

/// MachineDominanaceFrontier - This pass is a machine dominators analysis pass.
   /// bundles (created earlier, e.g. during pre-RA scheduling).
   extern char &FinalizeMachineBundlesID;
 
   /// StackMapLiveness - This pass analyses the register live-out set of
   /// stackmap/patchpoint intrinsics and attaches the calculated information to
   /// the intrinsic for later emission to the StackMap.
   extern char &StackMapLivenessID;
 
+  /// ExtendDebugRangeLocation pass
+  extern char &ExtendDebugRangeLocationID;
+
   /// createJumpInstrTables - This pass creates jump-instruction tables.
   ModulePass *createJumpInstrTablesPass();
 
   /// createForwardControlFlowIntegrityPass - This pass adds control-flow
   /// integrity.
   ModulePass *createForwardControlFlowIntegrityPass();
 
   /// InterleavedAccess Pass - This pass identifies and matches interleaved

include/llvm/InitializePasses.h

 void initializeFinalizeMachineBundlesPass(PassRegistry&);
 void initializeLoopAccessAnalysisPass(PassRegistry&);
 void initializeLoopVectorizePass(PassRegistry&);
 void initializeSLPVectorizerPass(PassRegistry&);
 void initializeBBVectorizePass(PassRegistry&);
 void initializeMachineFunctionPrinterPassPass(PassRegistry&);
 void initializeMIRPrintingPassPass(PassRegistry&);
 void initializeStackMapLivenessPass(PassRegistry&);
+void initializeExtendDebugRangeLocationPass(PassRegistry&);
 void initializeMachineCombinerPass(PassRegistry &);
 void initializeLoadCombinePass(PassRegistry&);
 void initializeRewriteSymbolsPass(PassRegistry&);
 void initializeWinEHPreparePass(PassRegistry&);
 void initializePlaceBackedgeSafepointsImplPass(PassRegistry&);
 void initializePlaceSafepointsPass(PassRegistry&);
 void initializeDwarfEHPreparePass(PassRegistry&);
 void initializeFloat2IntPass(PassRegistry&);
 void initializeLoopDistributePass(PassRegistry&);
 void initializeSjLjEHPreparePass(PassRegistry&);
 void initializeDemandedBitsPass(PassRegistry&);
 }
 
 #endif

lib/CodeGen/CMakeLists.txt

add_llvm_library(LLVMCodeGen
   DeadMachineInstructionElim.cpp
   DwarfEHPrepare.cpp
   EarlyIfConversion.cpp
   EdgeBundles.cpp
   ErlangGC.cpp
   ExecutionDepsFix.cpp
   ExpandISelPseudos.cpp
   ExpandPostRAPseudos.cpp
+  ExtendDebugRangeLocation.cpp
   FaultMaps.cpp
   GCMetadata.cpp
   GCMetadataPrinter.cpp
   GCRootLowering.cpp
   GCStrategy.cpp
   GlobalMerge.cpp
   IfConversion.cpp
   ImplicitNullChecks.cpp

lib/CodeGen/CodeGen.cpp

void llvm::initializeCodeGen(PassRegistry &Registry) {
   initializePostMachineSchedulerPass(Registry);
   initializePostRASchedulerPass(Registry);
   initializeProcessImplicitDefsPass(Registry);
   initializeRegisterCoalescerPass(Registry);
   initializeShrinkWrapPass(Registry);
   initializeSlotIndexesPass(Registry);
   initializeStackColoringPass(Registry);
   initializeStackMapLivenessPass(Registry);
+  initializeExtendDebugRangeLocationPass(Registry);
   initializeStackProtectorPass(Registry);
   initializeStackSlotColoringPass(Registry);
   initializeTailDuplicatePassPass(Registry);
   initializeTargetPassConfigPass(Registry);
   initializeTwoAddressInstructionPassPass(Registry);
   initializeUnpackMachineBundlesPass(Registry);
   initializeUnreachableBlockElimPass(Registry);
   initializeUnreachableMachineBlockElimPass(Registry);
   initializeVirtRegMapPass(Registry);
   initializeVirtRegRewriterPass(Registry);
   initializeWinEHPreparePass(Registry);
 }
 
 void LLVMInitializeCodeGen(LLVMPassRegistryRef R) {
   initializeCodeGen(*unwrap(R));
 }

lib/CodeGen/ExtendDebugRangeLocation.cpp

+//===-- ExtendDebugRangeLocation.cpp - Fixup Debug Value MIs --------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// This pass implements a data flow analysis that propagates debug location
+/// information by inserting additional DBG_VALUE instructions into the machine
+/// instruction stream. The pass internally builds debug location liveness
+/// ranges to determine the points where additional DBG_VALUEs need to be
+/// inserted.
+///
+/// This is a separate pass from DbgValueHistoryCalculator to facilitate
+/// testing and improve modularity.
+///
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineInstrBuilder.h"
+#include "llvm/CodeGen/Passes.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetRegisterInfo.h"
+#include "llvm/Target/TargetInstrInfo.h"
+#include "llvm/Target/TargetSubtargetInfo.h"
+#include <list>
+#include <deque>
+
+using namespace llvm;
+
+#define DEBUG_TYPE "extend-dbg-range-loc"
+
+STATISTIC(NumInserted, "Number of DBG_VALUE instructions inserted");
+
+namespace {
+
+class ExtendDebugRangeLocation : public MachineFunctionPass {
+
+public:
+  static char ID;
+
+  /// Default construct and initialize the pass.
+  ExtendDebugRangeLocation();
+
+  /// Tell the pass manager which passes we depend on and what
+  /// information we preserve.
+  void getAnalysisUsage(AnalysisUsage &AU) const override;
+
+  /// Print to ostream with a message
+  void print(const char *msg, raw_ostream &Out) const;
+
+  /// Calculate the liveness information for the given machine function.
+  bool runOnMachineFunction(MachineFunction &MF) override;
+
+private:
+  const TargetRegisterInfo *TRI;
+  const TargetInstrInfo *TII;
+
+  typedef std::pair<const DILocalVariable *, const DILocation *>
+      InlinedVariable;
+
+  /// Member variables and functions for Range Extension across basic blocks
+  struct VarLoc {
+    const MachineBasicBlock *MBB;
+    InlinedVariable Var;
+    const MachineInstr *MI; // MachineInstr should be a DBG_VALUE instr
+  
+    VarLoc(const MachineBasicBlock *_mbb, InlinedVariable _var,
+           const MachineInstr *_mi)
+        : MBB(_mbb), Var(_var), MI(_mi) {}
+  };
+  typedef std::list<VarLoc> VarLocList;
+
+  VarLocList OpenRanges;   // Ranges that are open until end of bb
+  VarLocList OutgoingLocs; // Ranges that exist beyond bb
+
+  bool OLChanged; // OutgoingLocs got changed for this bb
+  bool MBBJoined; // The MBB was joined
+
+  void transferDebugValue(MachineInstr &MI);
+  void transferRegisterDef(MachineInstr &MI);
+  void transferTerminatorInst(MachineInstr &MI);
+  void transfer(MachineInstr &MI);
+
+  void join(MachineBasicBlock &MBB);
+
+  void ExtendRanges(MachineFunction &MF);
+
+  void clearExtendDebugRangeLocationData();
+};
+} // namespace
+
+//===----------------------------------------------------------------------===//
+// Implementation
+//===----------------------------------------------------------------------===//
+
+char ExtendDebugRangeLocation::ID = 0;
+char &llvm::ExtendDebugRangeLocationID = ExtendDebugRangeLocation::ID;
+INITIALIZE_PASS(ExtendDebugRangeLocation, "extend-dbg-range-loc",
+                "Extend DBG_VALUE Range and Location Pass", false, false)
+
+/// Default construct and initialize the pass.
+ExtendDebugRangeLocation::ExtendDebugRangeLocation() : MachineFunctionPass(ID) {
+  initializeExtendDebugRangeLocationPass(*PassRegistry::getPassRegistry());
+}
+
+/// Tell the pass manager which passes we depend on and what information we
+/// preserve.
+void ExtendDebugRangeLocation::getAnalysisUsage(AnalysisUsage &AU) const {
+  MachineFunctionPass::getAnalysisUsage(AU);
+}
+
+void ExtendDebugRangeLocation::clearExtendDebugRangeLocationData() {
+  OLChanged = MBBJoined = false;
+  OpenRanges.clear();
+  OutgoingLocs.clear();
+}
+
+// \brief If @MI is a DBG_VALUE with debug value described by a
+// defined register, returns the number of this register.
+// In the other case, returns 0.
+static unsigned isDescribedByReg(const MachineInstr &MI) {
+  assert(MI.isDebugValue());
+  assert(MI.getNumOperands() == 4);
+  // If location of variable is described using a register (directly or
+  // indirecltly), this register is always a first operand.
+  return MI.getOperand(0).isReg() ? MI.getOperand(0).getReg() : 0;
+}
+
+// \brief This function takes two DBG_VALUE instructions and returns true
+// if their offsets are equal; otherwise returns false.
+static bool areOffsetsEqual(const MachineInstr &MI1, const MachineInstr &MI2) {
+  assert(MI1.isDebugValue());
+  assert(MI1.getNumOperands() == 4);
+
+  assert(MI2.isDebugValue());
+  assert(MI2.getNumOperands() == 4);
+
+  if (!MI1.isIndirectDebugValue() && !MI2.isIndirectDebugValue())
+    return true;
+
+  // Check if both MIs are indirect and they are equal
+  if (MI1.isIndirectDebugValue() && MI2.isIndirectDebugValue())
+    return MI1.getOperand(1).getImm() == MI2.getOperand(1).getImm();
+
+  return false;
+}
+
+//===----------------------------------------------------------------------===//
+// Debug Range Extension Implementation
+//===----------------------------------------------------------------------===//
+
+/// Print OpenRanges and OutgoingLocs
+void ExtendDebugRangeLocation::print(const char *msg, raw_ostream &Out) const {
+  Out << "\nPrint " << msg << "\n";
+  Out << "Printing OpenRanges:\n";
+  for (const auto &OR : OpenRanges) {
+    Out << "MBB: " << OR.MBB->getName() << " Var: " << OR.Var.first->getName();
+    Out << " MI: ";
+    (*OR.MI).dump();
+    Out << "\n";
+  }
+  Out << "Printing OutgoingLocs:\n";
+  for (const auto &OL : OutgoingLocs) {
+    Out << "MBB: " << OL.MBB->getName() << " Var: " << OL.Var.first->getName();
+    Out << " MI: ";
+    (*OL.MI).dump();
+    Out << "\n";
+  }
+  Out << "\n";
+}
+
+/// End all previous ranges related to @MI and start a new range from @MI
+/// if it is a DBG_VALUE instr.
+void ExtendDebugRangeLocation::transferDebugValue(MachineInstr &MI) {
+  if (MI.isDebugValue()) {
+    const DILocalVariable *RawVar = MI.getDebugVariable();
+    assert(RawVar->isValidLocationForIntrinsic(MI.getDebugLoc()) &&
+           "Expected inlined-at fields to agree");
+    InlinedVariable Var(RawVar, MI.getDebugLoc()->getInlinedAt());
+
+    // End all previous ranges of Var
+    VarLocList::iterator ORi = OpenRanges.begin(), ORe = OpenRanges.end();
+    while (ORi != ORe) {
+      VarLocList::iterator ORiSave = ORi;
+      ORi++;
+      if (Var == ORiSave->Var) {
+        OpenRanges.erase(ORiSave);
+        ORi = OpenRanges.begin(), ORe = OpenRanges.end();
+      }
+    }
+    // Add Var to OpenRanges from this DBG_VALUE
+    // TODO: Currently handles DBG_VALUE which has only reg as location
+    if (isDescribedByReg(MI)) {
+      VarLoc V(MI.getParent(), Var, &MI);
+      OpenRanges.push_back(std::move(V));
+    }
+  }
+}
+
+/// A definition of a register may mark the end of a range
+void ExtendDebugRangeLocation::transferRegisterDef(MachineInstr &MI) {
+  for (const MachineOperand &MO : MI.operands()) {
+    // Old register clobbering equivalent code
+    if (MO.isReg() && MO.isDef() && MO.getReg()) {
+      // Remove ranges of all aliased registers
+      for (MCRegAliasIterator RAI(MO.getReg(), TRI, true); RAI.isValid();
+           ++RAI) {
+        VarLocList::iterator ORi = OpenRanges.begin(), ORe = OpenRanges.end();
+        while (ORi != ORe) {
+          VarLocList::iterator ORiSave = ORi;
+          ORi++;
+          if (*RAI == isDescribedByReg(*ORiSave->MI)) {
+            OpenRanges.erase(ORiSave);
+            ORi = OpenRanges.begin(), ORe = OpenRanges.end();
+          }
+        }
+      }
+    }
+  }
+}
+
+/// Terminate all open ranges at the end of the current basic block
+void ExtendDebugRangeLocation::transferTerminatorInst(MachineInstr &MI) {
+  if (MI.isTerminator() || (&MI == &MI.getParent()->instr_back())) {
+    VarLocList::iterator ORi = OpenRanges.begin(), ORe = OpenRanges.end();
+    while (ORi != ORe) {
+      // Copy OpenRanges to OutgoingLocs, if not already present
+      if (std::find_if(OutgoingLocs.begin(), OutgoingLocs.end(),
+                       [&](const VarLoc &O) {
+                         return ((O.MBB == ORi->MBB) && (O.Var == ORi->Var) &&
+                                 (O.MI == ORi->MI));
+                       }) == OutgoingLocs.end()) {
+        assert(ORi->MI->isDebugValue());
+        DEBUG(dbgs() << "Add to OutgoingLocs: "; ORi->MI->dump(););
+        VarLoc V(ORi->MBB, ORi->Var, ORi->MI);
+        OutgoingLocs.push_back(std::move(V));
+        OLChanged = true;
+      }
+
+      // Delete this open range
+      OpenRanges.erase(ORi);
+      ORi = OpenRanges.begin(), ORe = OpenRanges.end();
+    }
+  }
+}
+
+/// This routine creates OpenRanges and OutgoingLocs
+void ExtendDebugRangeLocation::transfer(MachineInstr &MI) {
+
+  DEBUG(dbgs() << "transfer MI: "; MI.dump());
+
+  transferDebugValue(MI);
+  transferRegisterDef(MI);
+  transferTerminatorInst(MI);
+}
+
+/// This routine joins the analysis results of all incoming edges in @MBB by
+/// inserting a new DBG_VALUE instruction at the start of the @MBB - if the same
+/// source variable in all the predecessors of @MBB reside in the same location.
+void ExtendDebugRangeLocation::join(MachineBasicBlock &MBB) {
+  DEBUG(dbgs() << "join MBB: " << MBB.getName() << "\n");
+
+  MBBJoined = false;
+  if (MBB.pred_size() == 0 || MBB.pred_size() > 2)
+    return;
+
+  if (MBB.pred_size() == 1) {
+    // Propagate all OutgoingLocs into the current MBB
+    for (auto &OL : OutgoingLocs) {
+      if (OL.MBB == *MBB.pred_begin()) {
+        const MachineInstr *DMI = OL.MI;
+        MachineInstr *MI =
+            BuildMI(MBB, MBB.instr_begin(), DMI->getDebugLoc(), DMI->getDesc(),
+                    DMI->isIndirectDebugValue(), DMI->getOperand(0).getReg(), 0,
+                    DMI->getDebugVariable(), DMI->getDebugExpression());
+        if (DMI->isIndirectDebugValue())
+          MI->getOperand(1).setImm(DMI->getOperand(1).getImm());
+        DEBUG(dbgs() << "Inserted: "; MI->dump(););
+        ++NumInserted;
+        MBBJoined = true;
+      }
+    }
+    return;
+  }
+
+  MachineBasicBlock *pred1 = *MBB.pred_begin();
+  MachineBasicBlock *pred2 = *(++MBB.pred_begin());
+
+  // Search through OutgoingLocs to match 2 entries: <pred1, var>
+  // and <pred2, var>
+  for (auto OLo = OutgoingLocs.begin(), OLoe = OutgoingLocs.end();
+       OLo != OLoe; OLo++) { // outer loop
+    for (auto OLi = OutgoingLocs.begin(), OLie = OutgoingLocs.end();
+         OLi != OLie; OLi++) { // inner loop
+      // Check for equality of the 2 entries
+      if ((OLo != OLi) && (OLo->MBB == pred1 && OLi->MBB == pred2) &&
+          (OLo->Var == OLi->Var)) {
+        // Now the two locations should be same - reg and offset
+        if ((isDescribedByReg(*OLo->MI) == isDescribedByReg(*OLi->MI)) &&
+            (areOffsetsEqual((*OLo->MI), (*OLi->MI)))) {
+          // A new range is started for the var from the bb's
+          // beginning by inserting a new DBG_VALUE.
+          // transfer() will end this range however appropriate.
+          const MachineInstr *DMI = OLo->MI;
+          MachineInstr *MI = BuildMI(
+              MBB, MBB.instr_begin(), DMI->getDebugLoc(), DMI->getDesc(),
+              DMI->isIndirectDebugValue(), DMI->getOperand(0).getReg(), 0,
+              DMI->getDebugVariable(), DMI->getDebugExpression());
+          if (DMI->isIndirectDebugValue())
+            MI->getOperand(1).setImm(DMI->getOperand(1).getImm());
+          DEBUG(dbgs() << "Inserted: "; MI->dump(); dbgs() << "\n";);
+          ++NumInserted;
+          MBBJoined = true; // Rerun transfer() function
+        }
+      }
+    }
+  }
+}
+
+/// Calculate the liveness information for the given machine function and
+/// extend ranges across basic blocks
+void ExtendDebugRangeLocation::ExtendRanges(MachineFunction &MF) {
+
+  DEBUG(dbgs() << "\nDebug Range Extension\n");
+
+  clearExtendDebugRangeLocationData();
+
+  std::deque<MachineBasicBlock *> BBWorklist;
+
+  // Initialize every mbb with OutgoingLocs
+  for (auto &MBB : MF)
+    for (auto &MI : MBB)
+      transfer(MI);
+  DEBUG(print("After OutgoingLocs Initialize", dbgs()));
+
+  // Construct a worklist of MBBs
+  for (auto &MBB : MF)
+    BBWorklist.push_back(&MBB);
+
+  // Perform join() and transfer() using the worklist until the ranges converge
+  // Ranges have converged when the worklist is empty
+  while (!BBWorklist.empty()) {
+    MachineBasicBlock *MBB = BBWorklist.front();
+    BBWorklist.pop_front();
+
+    join(*MBB);
+
+    if (MBBJoined) {
+      for (auto &MI : *MBB)
+        transfer(MI);
+      DEBUG(print("After propagating joined MBB", dbgs()));
+
+      if (OLChanged) {
+        OLChanged = false;
+        for (MachineBasicBlock::const_succ_iterator si = MBB->succ_begin(),
+                                                    se = MBB->succ_end();
+             si != se; ++si)
+          if (std::find(BBWorklist.begin(), BBWorklist.end(), *si) ==
+              BBWorklist.end()) // add if not already present
+            BBWorklist.push_back(*si);
+      }
+    }
+  }
+  DEBUG(print("Final OutgoingLocs", dbgs()));
+}
+
+bool ExtendDebugRangeLocation::runOnMachineFunction(MachineFunction &MF) {
+  TRI = MF.getSubtarget().getRegisterInfo();
+  TII = MF.getSubtarget().getInstrInfo();
+
+  ExtendRanges(MF);
+
+  return true;
+}

lib/CodeGen/Passes.cpp

void TargetPassConfig::addMachinePasses() {
 
   // Basic block placement.
   if (getOptLevel() != CodeGenOpt::None)
     addBlockPlacement();
 
   addPreEmitPass();
 
   addPass(&StackMapLivenessID, false);
+  addPass(&ExtendDebugRangeLocationID);
 
   AddingMachinePasses = false;
 }
 
 /// Add passes that optimize machine instructions in SSA form.
 void TargetPassConfig::addMachineSSAOptimization() {
   // Pre-ra tail duplication.
   addPass(&EarlyTailDuplicateID);

test/DebugInfo/MIR/X86/extend-debug-range.mir

+# RUN: llc -run-pass=extend-dbg-range-loc -march=x86-64 -o /dev/null %s | FileCheck %s
+
+# Test the extension of debug ranges from predecessors.
+# Generated from the source file ExtendDebugRange.c:
+# #include <stdio.h>
+# int m;
+# extern int inc(int n); 
+# extern int change(int n); 
+# extern int modify(int n); 
+# int main(int argc, char **argv) {
+#   int n;
+#   if (argc != 2)
+#     n = 2;
+#   else
+#     n = atoi(argv[1]);
+#   n = change(n);
+#   if (n > 10) {
+#     m = modify(n);
+#     m = m + n;  // var `m' doesn't has a dbg.value
+#   }   
+#   else
+#     m = inc(n); // var `m' doesn't has a dbg.value
+#   printf("m(main): %d\n", m); 
+#   return 0;
+# }
+# with clang -g -emit-llvm ExtendDebugRange.c -c -o edr.bc
+# then opt -O3 -S -o extend-debug-range.ll edr.bc
+# then llc -stop-after stackmap-liveness extend-debug-range.ll -o /dev/null > extend-debug-range.mir
+# This case will also produce multiple locations but only the debug range
+# extension is tested here. This test case is tested with DWARF information under
+# llvm/test/DebugInfo/extend-debug-range.ll and present here for testing under
+# MIR->MIR serialization.
+
+# DBG_VALUE for variable "n" is extended into BB#5 from its predecessors BB#3
+# and BB#4.
+# CHECK:      bb.5.if.end.7:
+# CHECK:      DBG_VALUE debug-use %rsi, debug-use _, !19, !17, debug-location !20
+# CHECK-NEXT: DBG_VALUE debug-use %ebx, debug-use _, !26, !17, debug-location !27
+
+
+--- |
+  ; ModuleID = 'extend-debug-range.ll'
+  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-unknown-linux-gnu"
+  
+  @m = common global i32 0, align 4
+  @.str = private unnamed_addr constant [13 x i8] c"m(main): %d\0A\00", align 1
+  
+  ; Function Attrs: nounwind uwtable
+  define i32 @main(i32 %argc, i8** nocapture readonly %argv) #0 {
+  entry:
+    tail call void @llvm.dbg.value(metadata i32 %argc, i64 0, metadata !16, metadata !17), !dbg !18
+    tail call void @llvm.dbg.value(metadata i8** %argv, i64 0, metadata !19, metadata !17), !dbg !20
+    %cmp = icmp eq i32 %argc, 2, !dbg !21
+    br i1 %cmp, label %if.else, label %if.end, !dbg !23
+  
+  if.else:                                          ; preds = %entry
+    %arrayidx = getelementptr inbounds i8*, i8** %argv, i64 1, !dbg !24
+    %0 = load i8*, i8** %arrayidx, align 8, !dbg !24
+    %call = tail call i32 (i8*, ...) bitcast (i32 (...)* @atoi to i32 (i8*, ...)*)(i8* %0) #4, !dbg !25
+    tail call void @llvm.dbg.value(metadata i32 %call, i64 0, metadata !26, metadata !17), !dbg !27
+    br label %if.end
+  
+  if.end:                                           ; preds = %if.else, %entry
+    %n.0 = phi i32 [ %call, %if.else ], [ 2, %entry ]
+    %call1 = tail call i32 @change(i32 %n.0) #4, !dbg !28
+    tail call void @llvm.dbg.value(metadata i32 %call1, i64 0, metadata !26, metadata !17), !dbg !27
+    %cmp2 = icmp sgt i32 %call1, 10, !dbg !29
+    br i1 %cmp2, label %if.then.3, label %if.else.5, !dbg !31
+  
+  if.then.3:                                        ; preds = %if.end
+    %call4 = tail call i32 @modify(i32 %call1) #4, !dbg !32
+    %add = add nsw i32 %call4, %call1, !dbg !34
+    br label %if.end.7, !dbg !35
+  
+  if.else.5:                                        ; preds = %if.end
+    %call6 = tail call i32 @inc(i32 %call1) #4, !dbg !36
+    br label %if.end.7
+  
+  if.end.7:                                         ; preds = %if.else.5, %if.then.3
+    %storemerge = phi i32 [ %call6, %if.else.5 ], [ %add, %if.then.3 ]
+    store i32 %storemerge, i32* @m, align 4, !dbg !37
+    %call8 = tail call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([13 x i8], [13 x i8]* @.str, i64 0, i64 0), i32 %storemerge) #4, !dbg !38
+    ret i32 0, !dbg !39
+  }
+  
+  declare i32 @atoi(...) #1
+  
+  declare i32 @change(i32) #1
+  
+  declare i32 @modify(i32) #1
+  
+  declare i32 @inc(i32) #1
+  
+  ; Function Attrs: nounwind
+  declare i32 @printf(i8* nocapture readonly, ...) #2
+  
+  ; Function Attrs: nounwind readnone
+  declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #3
+  
+  attributes #0 = { nounwind uwtable }
+  attributes #2 = { nounwind }
+  attributes #3 = { nounwind readnone }
+  attributes #4 = { nounwind }
+  
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!13, !14}
+  !llvm.ident = !{!15}
+  
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.8.0 (trunk 243841)", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3, globals: !11)
+  !1 = !DIFile(filename: "ExtendDebugRange.c", directory: "/home/vt/julia/test/vikram")
+  !2 = !{}
+  !3 = !{!4}
+  !4 = !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !5, isLocal: false, isDefinition: true, scopeLine: 6, flags: DIFlagPrototyped, isOptimized: false, function: i32 (i32, i8**)* @main, variables: !2)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{!7, !7, !8}
+  !7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+  !8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64, align: 64)
+  !9 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !10, size: 64, align: 64)
+  !10 = !DIBasicType(name: "char", size: 8, align: 8, encoding: DW_ATE_signed_char)
+  !11 = !{!12}
+  !12 = !DIGlobalVariable(name: "m", scope: !0, file: !1, line: 2, type: !7, isLocal: false, isDefinition: true, variable: i32* @m)
+  !13 = !{i32 2, !"Dwarf Version", i32 4}
+  !14 = !{i32 2, !"Debug Info Version", i32 3}
+  !15 = !{!"clang version 3.8.0 (trunk 243841)"}
+  !16 = !DILocalVariable(name: "argc", arg: 1, scope: !4, file: !1, line: 6, type: !7)
+  !17 = !DIExpression()
+  !18 = !DILocation(line: 6, column: 14, scope: !4)
+  !19 = !DILocalVariable(name: "argv", arg: 2, scope: !4, file: !1, line: 6, type: !8)
+  !20 = !DILocation(line: 6, column: 27, scope: !4)
+  !21 = !DILocation(line: 8, column: 12, scope: !22)
+  !22 = distinct !DILexicalBlock(scope: !4, file: !1, line: 8, column: 7)
+  !23 = !DILocation(line: 8, column: 7, scope: !4)
+  !24 = !DILocation(line: 11, column: 14, scope: !22)
+  !25 = !DILocation(line: 11, column: 9, scope: !22)
+  !26 = !DILocalVariable(name: "n", scope: !4, file: !1, line: 7, type: !7)
+  !27 = !DILocation(line: 7, column: 7, scope: !4)
+  !28 = !DILocation(line: 13, column: 7, scope: !4)
+  !29 = !DILocation(line: 14, column: 9, scope: !30)
+  !30 = distinct !DILexicalBlock(scope: !4, file: !1, line: 14, column: 7)
+  !31 = !DILocation(line: 14, column: 7, scope: !4)
+  !32 = !DILocation(line: 15, column: 9, scope: !33)
+  !33 = distinct !DILexicalBlock(scope: !30, file: !1, line: 14, column: 15)
+  !34 = !DILocation(line: 16, column: 11, scope: !33)
+  !35 = !DILocation(line: 17, column: 3, scope: !33)
+  !36 = !DILocation(line: 19, column: 9, scope: !30)
+  !37 = !DILocation(line: 16, column: 7, scope: !33)
+  !38 = !DILocation(line: 20, column: 3, scope: !4)
+  !39 = !DILocation(line: 22, column: 3, scope: !4)
+
+...
+---
+name:            main
+alignment:       4
+exposesReturnsTwice: false
+hasInlineAsm:    false
+isSSA:           false
+tracksRegLiveness: true
+tracksSubRegLiveness: false
+liveins:         
+  - { reg: '%edi' }
+  - { reg: '%rsi' }
+calleeSavedRegisters: [ '%bh', '%bl', '%bp', '%bpl', '%bx', '%ebp', '%ebx', 
+                        '%rbp', '%rbx', '%r12', '%r13', '%r14', '%r15', 
+                        '%r12b', '%r13b', '%r14b', '%r15b', '%r12d', '%r13d', 
+                        '%r14d', '%r15d', '%r12w', '%r13w', '%r14w', '%r15w' ]
+frameInfo:       
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       24
+  offsetAdjustment: -8
+  maxAlignment:    0
+  adjustsStack:    true
+  hasCalls:        true
+  maxCallFrameSize: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+fixedStack:      
+  - { id: 0, type: spill-slot, offset: -24, size: 8, alignment: 8, callee-saved-register: '%rbx' }
+  - { id: 1, type: spill-slot, offset: -16, size: 8, alignment: 16 }
+body:             |
+  bb.0.entry:
+    successors: %bb.1.if.else(16), %bb.2.if.end(16)
+    liveins: %edi, %rsi, %rbx, %rbx, %rbp
+  
+    frame-setup PUSH64r killed %rbp, implicit-def %rsp, implicit %rsp
+    CFI_INSTRUCTION .cfi_def_cfa_offset 16
+    CFI_INSTRUCTION .cfi_offset %rbp, -16
+    %rbp = frame-setup MOV64rr %rsp
+    CFI_INSTRUCTION .cfi_def_cfa_register %rbp
+    frame-setup PUSH64r killed %rbx, implicit-def %rsp, implicit %rsp
+    frame-setup PUSH64r undef %rax, implicit-def %rsp, implicit %rsp
+    CFI_INSTRUCTION .cfi_offset %rbx, -24
+    DBG_VALUE debug-use %edi, debug-use _, !16, !17, debug-location !18
+    DBG_VALUE debug-use %rsi, debug-use _, !19, !17, debug-location !20
+    %eax = MOV32rr %edi
+    DBG_VALUE debug-use %eax, debug-use _, !16, !17, debug-location !18
+    %edi = MOV32ri 2
+    CMP32ri8 killed %eax, 2, implicit-def %eflags, debug-location !23
+    JNE_1 %bb.2.if.end, implicit %eflags
+  
+  bb.1.if.else:
+    successors: %bb.2.if.end
+    liveins: %rsi, %rbp
+  
+    DBG_VALUE debug-use %rsi, debug-use _, !19, !17, debug-location !20
+    %rdi = MOV64rm killed %rsi, 1, _, 8, _, debug-location !24 :: (load 8 from %ir.arrayidx)
+    dead %eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags, implicit-def %al, debug-location !25
+    CALL64pcrel32 @atoi, csr_64, implicit %rsp, implicit %rdi, implicit %al, implicit-def %rsp, implicit-def %eax, debug-location !25
+    %edi = MOV32rr %eax, debug-location !25
+    DBG_VALUE debug-use %edi, debug-use _, !26, !17, debug-location !27
+  
+  bb.2.if.end:
+    successors: %bb.3.if.then.3(16), %bb.4.if.else.5(16)
+    liveins: %edi, %rbp
+  
+    CALL64pcrel32 @change, csr_64, implicit %rsp, implicit %edi, implicit-def %rsp, implicit-def %eax, debug-location !28
+    %ebx = MOV32rr %eax, debug-location !28
+    DBG_VALUE debug-use %ebx, debug-use _, !26, !17, debug-location !27
+    CMP32ri8 %ebx, 11, implicit-def %eflags, debug-location !31
+    JL_1 %bb.4.if.else.5, implicit killed %eflags, debug-location !31
+  
+  bb.3.if.then.3:
+    successors: %bb.5.if.end.7
+    liveins: %ebx, %rbp
+  
+    DBG_VALUE debug-use %ebx, debug-use _, !26, !17, debug-location !27
+    %edi = MOV32rr %ebx, debug-location !32
+    CALL64pcrel32 @modify, csr_64, implicit %rsp, implicit %edi, implicit-def %rsp, implicit-def %eax, debug-location !32
+    %ecx = MOV32rr %eax, debug-location !32
+    %ecx = ADD32rr killed %ecx, killed %ebx, implicit-def dead %eflags, debug-location !34
+    JMP_1 %bb.5.if.end.7
+  
+  bb.4.if.else.5:
+    successors: %bb.5.if.end.7
+    liveins: %ebx, %rbp
+  
+    DBG_VALUE debug-use %ebx, debug-use _, !26, !17, debug-location !27
+    %edi = MOV32rr killed %ebx, debug-location !36
+    CALL64pcrel32 @inc, csr_64, implicit %rsp, implicit %edi, implicit-def %rsp, implicit-def %eax, debug-location !36
+    %ecx = MOV32rr %eax, debug-location !36
+  
+  bb.5.if.end.7:
+    liveins: %ecx, %rbp
+  
+    MOV32mr %rip, 1, _, @m, _, %ecx, debug-location !37 :: (store 4 into @m)
+    dead undef %edi = MOV32ri64 @.str, implicit-def %rdi, debug-location !38
+    dead %eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags, implicit-def %al, debug-location !39
+    %esi = MOV32rr killed %ecx, debug-location !38
+    CALL64pcrel32 @printf, csr_64, implicit %rsp, implicit %rdi, implicit %esi, implicit %al, implicit-def %rsp, implicit-def dead %eax, debug-location !38
+    %eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags, debug-location !39
+    %rsp = ADD64ri8 %rsp, 8, implicit-def dead %eflags, debug-location !39
+    %rbx = POP64r implicit-def %rsp, implicit %rsp, debug-location !39
+    %rbp = POP64r implicit-def %rsp, implicit %rsp, debug-location !39
+    RETQ %eax, debug-location !39
+
+...

test/DebugInfo/MIR/X86/lit.local.cfg

+if not 'X86' in config.root.targets:
+    config.unsupported = True

test/DebugInfo/MIR/lit.local.cfg

+config.suffixes = ['.mir']
+

test/DebugInfo/X86/array.ll

 ;   return array[0];
 ; }
 ;
 ; RUN: llc -filetype=asm %s -o - | FileCheck %s
 ; Test that we only emit register-indirect locations for the array array.
 ; rdar://problem/14874886
 ;
 ; CHECK:     ##DEBUG_VALUE: main:array <- [R{{.*}}+0]
+; CHECK:     ##DEBUG_VALUE: main:array <- [R{{.*}}+0]
+; CHECK:     ##DEBUG_VALUE: main:array <- [R{{.*}}+0]
 ; CHECK-NOT: ##DEBUG_VALUE: main:array <- R{{.*}}
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.9.0"
 
 @main.array = private unnamed_addr constant [4 x i32] [i32 0, i32 1, i32 2, i32 3], align 16
 
 ; Function Attrs: nounwind ssp uwtable
 define void @f(i32* nocapture %p) #0 {

test/DebugInfo/X86/fission-ranges.ll

 ; Don't assume these locations are entirely correct - feel free to update them
 ; if they've changed due to a bugfix, change in register allocation, etc.
 
 ; CHECK: [[A]]: Beginning address index: 2
 ; CHECK-NEXT:                    Length: 169
 ; CHECK-NEXT:      Location description: 11 00
 ; CHECK-NEXT: {{^$}}
 ; CHECK-NEXT:   Beginning address index: 3
-; CHECK-NEXT:                    Length: 21
+; CHECK-NEXT:                    Length: 25
 ; CHECK-NEXT:      Location description: 50 93 04
 ; CHECK: [[E]]: Beginning address index: 4
-; CHECK-NEXT:                    Length: 19
+; CHECK-NEXT:                    Length: 23
 ; CHECK-NEXT:      Location description: 50 93 04
 ; CHECK: [[B]]: Beginning address index: 5
-; CHECK-NEXT:                    Length: 17
+; CHECK-NEXT:                    Length: 21
 ; CHECK-NEXT:      Location description: 50 93 04
 ; CHECK: [[D]]: Beginning address index: 6
-; CHECK-NEXT:                    Length: 17
+; CHECK-NEXT:                    Length: 21
 ; CHECK-NEXT:      Location description: 50 93 04
 
 ; Make sure we don't produce any relocations in any .dwo section (though in particular, debug_info.dwo)
 ; HDR-NOT: .rela.{{.*}}.dwo
 
 ; Make sure we have enough stuff in the debug_addr to cover the address indexes
 ; (6 is the last index in debug_loc.dwo, making 7 entries of 8 bytes each, 7 * 8
 ; == 56 base 10 == 38 base 16)

test/DebugInfo/X86/pieces-3.ll

 ; CHECK: DW_TAG_variable
 ;                                                 rsi, piece 0x00000004
 ; CHECK-NEXT:   DW_AT_location [DW_FORM_block1]       {{.*}} 54 93 04
 ; CHECK-NEXT:   "i1"
 ;
 ; CHECK: .debug_loc
 ; CHECK: [[LOC]]:
 ; CHECK: Beginning address offset: 0x0000000000000000
-; CHECK:    Ending address offset: 0x0000000000000004
+; CHECK:    Ending address offset: 0x0000000000000008
 ; rdi, piece 0x00000008, piece 0x00000004, rsi, piece 0x00000004
 ; CHECK: Location description: 55 93 08 93 04 54 93 04 
 ;
 ; ModuleID = '/Volumes/Data/llvm/test/DebugInfo/X86/sroasplit-2.ll'
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.9.0"
 
 ; Function Attrs: nounwind ssp uwtable

test/DebugInfo/X86/sret.ll

 ; RUN: llc -split-dwarf=Enable -O0 %s -mtriple=x86_64-unknown-linux-gnu -filetype=obj -o %t
 ; RUN: llvm-dwarfdump -debug-dump=all %t | FileCheck %s
 
 ; Based on the debuginfo-tests/sret.cpp code.
 
-; CHECK: DW_AT_GNU_dwo_id [DW_FORM_data8] (0x51ac5644b1937aa1)
-; CHECK: DW_AT_GNU_dwo_id [DW_FORM_data8] (0x51ac5644b1937aa1)
+; CHECK: DW_AT_GNU_dwo_id [DW_FORM_data8] ([[ADDR:0x[0-9a-z]*]])
+; CHECK: DW_AT_GNU_dwo_id [DW_FORM_data8] ([[ADDR]])
 
 %class.A = type { i32 (...)**, i32 }
 %class.B = type { i8 }
 
 @_ZTV1A = linkonce_odr unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*), i8* bitcast (void (%class.A*)* @_ZN1AD2Ev to i8*), i8* bitcast (void (%class.A*)* @_ZN1AD0Ev to i8*)]
 @_ZTVN10__cxxabiv117__class_type_infoE = external global i8*
 @_ZTS1A = linkonce_odr constant [3 x i8] c"1A\00"
 @_ZTI1A = linkonce_odr constant { i8*, i8* } { i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv117__class_type_infoE, i64 2) to i8*), i8* getelementptr inbounds ([3 x i8], [3 x i8]* @_ZTS1A, i32 0, i32 0) }

test/DebugInfo/extend-debug-range.ll

+; RUN: llc -filetype=asm %s -o - | FileCheck %s
+
+; Test the extension of debug ranges from predecessors.
+; Generated from the source file ExtendDebugRange.c:
+; #include <stdio.h>
+; int m;
+; extern int inc(int n); 
+; extern int change(int n); 
+; extern int modify(int n); 
+; int main(int argc, char **argv) {
+;   int n;
+;   if (argc != 2)
+;     n = 2;
+;   else
+;     n = atoi(argv[1]);
+;   n = change(n);
+;   if (n > 10) {
+;     m = modify(n);
+;     m = m + n;  // var `m' doesn't has a dbg.value
+;   }
+;   else
+;     m = inc(n); // var `m' doesn't has a dbg.value
+;   printf("m(main): %d\n", m); 
+;   return 0;
+; }
+; with clang -g -emit-llvm ExtendDebugRange.c -c -o edr.bc
+; then opt -O3 -S -o extend-debug-range.ll edr.bc
+; This case will also produce multiple locations but only the debug range
+; extension is tested here.
+
+; DBG_VALUE for variable "n" is extended into BB#5 from its predecessors BB#3
+; and BB#4.
+; CHECK:       .LBB0_5:
+; CHECK-NEXT:  #DEBUG_VALUE: main:argv <- RSI 
+; CHECK-NEXT:  #DEBUG_VALUE: main:n <- EBX 
+
+
+; ModuleID = 'edr.bc'
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@m = common global i32 0, align 4
+@.str = private unnamed_addr constant [13 x i8] c"m(main): %d\0A\00", align 1
+
+; Function Attrs: nounwind uwtable
+define i32 @main(i32 %argc, i8** nocapture readonly %argv) #0 {
+entry:
+  tail call void @llvm.dbg.value(metadata i32 %argc, i64 0, metadata !16, metadata !17), !dbg !18
+  tail call void @llvm.dbg.value(metadata i8** %argv, i64 0, metadata !19, metadata !17), !dbg !20
+  %cmp = icmp eq i32 %argc, 2, !dbg !21
+  br i1 %cmp, label %if.else, label %if.end, !dbg !23
+
+if.else:                                          ; preds = %entry
+  %arrayidx = getelementptr inbounds i8*, i8** %argv, i64 1, !dbg !24
+  %0 = load i8*, i8** %arrayidx, align 8, !dbg !24
+  %call = tail call i32 (i8*, ...) bitcast (i32 (...)* @atoi to i32 (i8*, ...)*)(i8* %0) #4, !dbg !25
+  tail call void @llvm.dbg.value(metadata i32 %call, i64 0, metadata !26, metadata !17), !dbg !27
+  br label %if.end
+
+if.end:                                           ; preds = %entry, %if.else
+  %n.0 = phi i32 [ %call, %if.else ], [ 2, %entry ]
+  %call1 = tail call i32 @change(i32 %n.0) #4, !dbg !28
+  tail call void @llvm.dbg.value(metadata i32 %call1, i64 0, metadata !26, metadata !17), !dbg !27
+  %cmp2 = icmp sgt i32 %call1, 10, !dbg !29
+  br i1 %cmp2, label %if.then.3, label %if.else.5, !dbg !31
+
+if.then.3:                                        ; preds = %if.end
+  %call4 = tail call i32 @modify(i32 %call1) #4, !dbg !32
+  %add = add nsw i32 %call4, %call1, !dbg !34
+  br label %if.end.7, !dbg !35
+
+if.else.5:                                        ; preds = %if.end
+  %call6 = tail call i32 @inc(i32 %call1) #4, !dbg !36
+  br label %if.end.7
+
+if.end.7:                                         ; preds = %if.else.5, %if.then.3
+  %storemerge = phi i32 [ %call6, %if.else.5 ], [ %add, %if.then.3 ]
+  store i32 %storemerge, i32* @m, align 4, !dbg !37
+  %call8 = tail call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([13 x i8], [13 x i8]* @.str, i64 0, i64 0), i32 %storemerge) #4, !dbg !38
+  ret i32 0, !dbg !39
+}
+
+declare i32 @atoi(...) #1
+
+declare i32 @change(i32) #1
+
+declare i32 @modify(i32) #1
+
+declare i32 @inc(i32) #1
+
+; Function Attrs: nounwind
+declare i32 @printf(i8* nocapture readonly, ...) #2
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #3
+
+attributes #0 = { nounwind uwtable }
+attributes #2 = { nounwind }
+attributes #3 = { nounwind readnone }
+attributes #4 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!13, !14}
+!llvm.ident = !{!15}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.8.0 (trunk 243841)", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3, globals: !11)
+!1 = !DIFile(filename: "ExtendDebugRanges.c", directory: "/home/vt/vikram/newTests")
+!2 = !{}
+!3 = !{!4}
+!4 = !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !5, isLocal: false, isDefinition: true, scopeLine: 6, flags: DIFlagPrototyped, isOptimized: false, function: i32 (i32, i8**)* @main, variables: !2)
+!5 = !DISubroutineType(types: !6)
+!6 = !{!7, !7, !8}
+!7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64, align: 64)
+!9 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !10, size: 64, align: 64)
+!10 = !DIBasicType(name: "char", size: 8, align: 8, encoding: DW_ATE_signed_char)
+!11 = !{!12}
+!12 = !DIGlobalVariable(name: "m", scope: !0, file: !1, line: 2, type: !7, isLocal: false, isDefinition: true, variable: i32* @m)
+!13 = !{i32 2, !"Dwarf Version", i32 4}
+!14 = !{i32 2, !"Debug Info Version", i32 3}
+!15 = !{!"clang version 3.8.0 (trunk 243841)"}
+!16 = !DILocalVariable(name: "argc", arg: 1, scope: !4, file: !1, line: 6, type: !7)
+!17 = !DIExpression()
+!18 = !DILocation(line: 6, column: 14, scope: !4)
+!19 = !DILocalVariable(name: "argv", arg: 2, scope: !4, file: !1, line: 6, type: !8)
+!20 = !DILocation(line: 6, column: 27, scope: !4)
+!21 = !DILocation(line: 8, column: 12, scope: !22)
+!22 = distinct !DILexicalBlock(scope: !4, file: !1, line: 8, column: 7)
+!23 = !DILocation(line: 8, column: 7, scope: !4)
+!24 = !DILocation(line: 11, column: 14, scope: !22)
+!25 = !DILocation(line: 11, column: 9, scope: !22)
+!26 = !DILocalVariable(name: "n", scope: !4, file: !1, line: 7, type: !7)
+!27 = !DILocation(line: 7, column: 7, scope: !4)
+!28 = !DILocation(line: 12, column: 7, scope: !4)
+!29 = !DILocation(line: 13, column: 9, scope: !30)
+!30 = distinct !DILexicalBlock(scope: !4, file: !1, line: 13, column: 7)
+!31 = !DILocation(line: 13, column: 7, scope: !4)
+!32 = !DILocation(line: 14, column: 9, scope: !33)
+!33 = distinct !DILexicalBlock(scope: !30, file: !1, line: 13, column: 15)
+!34 = !DILocation(line: 15, column: 11, scope: !33)
+!35 = !DILocation(line: 16, column: 3, scope: !33)
+!36 = !DILocation(line: 18, column: 9, scope: !30)
+!37 = !DILocation(line: 15, column: 7, scope: !33)
+!38 = !DILocation(line: 19, column: 3, scope: !4)
+!39 = !DILocation(line: 20, column: 3, scope: !4)