[DebugInfo] More precise variable range for stack locations

[NikolaPrica]

Summary

Variable's stack location can stretch longer than they should. If variable is placed at stack in some nested basic block its range can be calculated to be up to next occurrence of variable's DBG_VALUE or up to the end of function thus covering basic blocks that should not be covered by location range. This is because DbgEntityHistoryCalculator ends register locations at the end of basic block only if register has been changed through course of function which is not case for register used to reference stack objects.
This patch also tries to produce single value location if location list builder managed to merge all locations into one.

[dstenb]

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1359 ↗

(On Diff #198278)

It seems that this can give incorrect single value locations for cases where we have produced location list entries with empty ranges.

For example:

frame-setup PUSH64r undef $rax, implicit-def $rsp, implicit $rsp
DBG_VALUE 123, $noreg, !12, !DIExpression(), debug-location !13 
DBG_VALUE 456, $noreg, !12, !DIExpression(), debug-location !13 
CALL64pcrel32 @foo, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, debug-location !14 
$eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, debug-location !15                                                                   
$rcx = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !15 
RETQ killed $eax, debug-location !15

Here, the first debug value will result in a location list entry with an empty range, since it is immediately followed by the second, and such entries are omitted from the location list, resulting in a single entry in Entries for the constant value 456. However, MInsn is the first debug value, so we will create a single location description with the wrong value:

0x00000043:     DW_TAG_variable
                  DW_AT_const_value	(123)

I'm not sure what the check should be to avoid these sort of cases. Perhaps check that all debug value entries in HistoryMapEntries are described by the same location, and have the same expression?

[NikolaPrica]

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1359 ↗

(On Diff #198278)

Such check could be return value of buildLocationList? Or we can just take the last DebugInstr from HistoryMapEntries which could be the last entry or the entry before the last one?

[dstenb]

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1359 ↗

(On Diff #198278)

Or we can just take the last DebugInstr from HistoryMapEntries which could be the last entry or the entry before the last one?

That is perhaps possible. I tried to think if we can land in a situation where the last DebugInstr in HistoryMapEntries produces a location list entry with an empty range. For example something like this:

DBG_VALUE 123, $noreg, !12, !DIExpression(), debug-location !13 
[...]
DBG_VALUE 123, $noreg, !12, !DIExpression(), debug-location !13
DBG_VALUE 456, $noreg, !12, !DIExpression(), debug-location !13 
[end of function]

However, AFAIK we can't insert debug values after terminators currently (the machine verifier will complain about that), and with a presence of a terminator in the last block then the range will not be empty, so perhaps this is guaranteed? I'm not completely sure.

[NikolaPrica]

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1359 ↗

(On Diff #198278)

I'm not sure either. But I suppose that such situation can appear for local variable that ends at some basic block? Maybe it is safer perform such verification in buildLocationList and return it as result?

[dstenb]

(Adding Jeremy as reviewer as he has also been working in this area recently.)

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

375 ↗

(On Diff #198278)

Perhaps this must not be done in this patch, but I wonder if we should close all register-described values here?

For example:

volatile int g;

int baz(int p) {
  int local = 123;

  if (g > 1) {
    local = p;
    g += p;
  }

  return 3;
}

when compiling the above with clang -O1 -g --target=x86_64-unknown-linux-gnu we land in the following MIR after livedebugvalues:

bb.0.entry:
  successors: %bb.1(0x40000000), %bb.2(0x40000000)
  liveins: $edi

  DBG_VALUE $edi, $noreg, !16, !DIExpression(), debug-location !18
  DBG_VALUE $edi, $noreg, !16, !DIExpression(), debug-location !18
  DBG_VALUE 123, $noreg, !17, !DIExpression(), debug-location !19
  renamable $eax = MOV32rm $rip, 1, $noreg, @g, $noreg, debug-location !20 :: (volatile dereferenceable load 4 from @g, !tbaa !22)
  CMP32ri8 killed renamable $eax, 2, implicit-def $eflags, debug-location !26
  JCC_1 %bb.2, 12, implicit $eflags, debug-location !27

bb.1.if.then:
  successors: %bb.2(0x80000000)
  liveins: $edi

  DBG_VALUE 123, $noreg, !17, !DIExpression(), debug-location !19
  DBG_VALUE $edi, $noreg, !16, !DIExpression(), debug-location !18
  DBG_VALUE $edi, $noreg, !17, !DIExpression(), debug-location !19
  ADD32mr $rip, 1, $noreg, @g, $noreg, killed renamable $edi, implicit-def dead $eflags, debug-location !28 :: (volatile store 4 into @g, !tbaa !22), (volatile dereferenceable load 4 from @g, !tbaa !22)

bb.2.if.end:
  DBG_VALUE $edi, $noreg, !16, !DIExpression(), debug-location !18
  $eax = MOV32ri 3, debug-location !31
  RETQ $eax, debug-location !31

As seen, in the g > 1 branch the local variable is described by edi. As edi is not modified, it will not be contained in ChangingRegs.

As we don't close the range here, this means that we'll incorrectly describe local using edi for the rest of the function:

DW_AT_location	(0x00000000
   [0x0000000000000000,  0x000000000000000b): DW_OP_consts +123, DW_OP_stack_value
   [0x000000000000000b,  0x0000000000000017): DW_OP_reg5 RDI)

0000000000000000 baz:
       0: 8b 05 00 00 00 00            	movl	(%rip), %eax
       6: 83 f8 02                     	cmpl	$2, %eax
       9: 7c 06                        	jl	6 <baz+0x11>
       b: 01 3d 00 00 00 00            	addl	%edi, (%rip)
      11: b8 03 00 00 00               	movl	$3, %eax
      16: c3                           	retq

[jmorse]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

375 ↗

(On Diff #198278)

Perhaps this must not be done in this patch, but I wonder if we should close all register-described values here?

I'd say definite yes -- when I dug into the history [0] for PR41600 (which this patch fixes I think) it sounded like ChangingRegs was a mild hack to work around variable locations being unreliable ~5 years ago. IMHO (YMMV) we might be able to delete it in another patch as a now redundant feature. (Plus as you demonstrate, it can be broken).

[0] https://reviews.llvm.org/D3933

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1359 ↗

(On Diff #198278)

Great question @ empty ranges; would those be filtered out by this clause [0]?

I believe there are certain circumstances where LLVM won't have a (Machine IR) terminator after a function call declared attribute((noreturn)), often at the end of a function. A quick experiment shows that during IR optimisation another part of the compiler places an 'unreachable' inst after the noreturn call and drops any dbg.values, so I doubt this scenario is accessible from source languages.

[0] https://github.com/llvm/llvm-project/blob/da82ce99b7460164ffb841619aadb44f397d2106/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp#L1189

test/DebugInfo/MIR/X86/dbg-stack-value-range.mir

3–8 ↗

(On Diff #198278)

IMHO the second sentence should describe the desired behaviour ("DbgEntityHistoryCalculator should close the variable's range at the end of the basic block..."), then afterwards describe the error that this test is for. It's not obvious right now that the second sentence is about the error case, until you read the third sentence.

72 ↗

(On Diff #198278)

I understand attributes are a pain to maintain in tests; can they be dropped?

105 ↗

(On Diff #198278)

Nit - most of the default MIR data can be omitted, pretty much every attribute that's false. (--simplify-mir to llc will drop it automatically).

[NikolaPrica]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

375 ↗

(On Diff #198278)

This is similar problem that I am targeting. It would be solved by closing register-described values at the end of block. The only difference is that local is dead after the branch, but the idea is the same: Having BB1 that branches to BB2 and BB3, different DBG_VALUE from BB2 takes range across BB3 due to lack of variable's DBG_VALUE in it (variable might be dead or it just does lack location at that block).

I stumbled on a case when frame register is not in ChangingRegs but as you shown it can happen for other registers. I needed to modify DwarfDebug part in order to preserve single value variable locations because of partitioning of variable's range. Current condition for recognizing single value locations would not be recognized as single value location).

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1359 ↗

(On Diff #198278)

Great question @ empty ranges; would those be filtered out by this clause [0]?

They would, but the result location will end up as a DebugLocEntry::Value. The catch is that initialization of DbgVariable takes DBG_VALUE instruction. Maybe we should refactor DbgVariable to have DebugLocEntry::Value instead of DBG_VALUE instruction?

test/DebugInfo/MIR/X86/dbg-stack-value-range.mir

3–8 ↗

(On Diff #198278)

Sure. I will rewrite it!

72 ↗

(On Diff #198278)

I usually drop them from .ll but now I needed "no-frame-pointer" for test case so I missed to remove them from .mir. I will drop it.

[NikolaPrica]

I've posted new approach for fixing this problem at https://reviews.llvm.org/D61890. If that approach turns to be better I will abandon this change.

[jmorse]

Just to note: the patch this is based on (r359426) was reverted in r360301 due to compile time issues. I've got a patch incoming (tomorrow) that relies on this patch to fix it: they'll need to be un-reverted / committed in a certain order and possibly all together. (We can work through that when this is approved though).

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

375 ↗

(On Diff #198278)

NB: I think the ChangingRegs situation can be addressed separately. and this patch is fine to go ahead with the other tweaks.

[aprantl]

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1355 ↗

(On Diff #198278)

Tiny nit: Can we use the actual class name instead of paraphrasing it as "location list builder"?

[NikolaPrica]

-Take last HistoryMapEntries as a representative single value location.
-Update test explanation.
-Strip uneccessary parts of test.

[jmorse]

LGTM (I think I can approve?). The source IR needing no-frame-pointer-elim seems like enough reason to keep that attribute around.

As mentioned above, r360301 needs to be reverted before this is committed, and then D61940 is needed to fix the compile time regression that will cause. Best to wait for D61940 to be approved before committing, so that we can do that all at once.

[jmorse]

Nikola mentioned in D61890 that the changes to fission-ranges.ll result in reduced variable location ranges, which is unfortunate. IMO, eyeballing the test, I think the input IR we have at the moment is wrong, and it was only the bug that this patch fixes that caused us to get the output right.

In fission-ranges.ll the dbg.value for !18 / the "e" variable, has a !DILocation !39, which corresonds to the lexical block where 'b' is iterated over. This causes LiveDebugValues to not propagate the variable location out of the block modifying "e", because it believes "e" isn't valid outside of that lexical block. In the source code at the top of the file though we can see that "e" has a function-wide scope. Thus the IR in the test is wrong, the dbg.value should have a different DILocation! We only covered the whole function before because DbgEntityHistoryCalculator was extending variable locations too far.

We should check whether clang still generates this faulty IR, but I've run out of time for today.

[NikolaPrica]

For the sample code in fisson-range.ll we now generate slightly different IR (few more llvm.dbg.values). With such IR, LLVM generates one block larger range but nevertheless the problem there is different nature and it could be used as a test case for further improvement of LiveDebugValues (value from stack gets loaded into a callee clobberabble register and at the entry of a block we have two variable locations and we choose the one which is inserted later - see LiveDebugvalues::transferDebugValue). I will take a look into that.

I suppose that this is ready to land?

[NikolaPrica]

Actually I've just produce fix for fisson-ranges.ll that will have better ranges. I'll post it as child patch of this one.

[jmorse]

In D61600#1509763, @NikolaPrica wrote:

I suppose that this is ready to land?

Indeed, although as mentioned that requires one patch to be unreverted, and D61940 to be ready to fix the performance regression in it :/. There shouldn't be a serious delay, hopefully.

[NikolaPrica]

-Rebase
-Change referenced scope to function scope for variable "b" in fisson-ranges.ll

[dstenb]

Adding a comment about another hand-written example.

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

1369–1370 ↗

(On Diff #200507)

If we have input like this:

!13 = !DILocalVariable(name: "local", scope: !7, file: !1, line: 5, type: !14)
!14 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
[...]
  bb.0.entry:
    DBG_VALUE 123, $noreg, !13, !DIExpression(), debug-location !16
    DBG_VALUE 456, $noreg, !13, !DIExpression(DW_OP_LLVM_fragment, 0, 16), debug-location !18
    DBG_VALUE 789, $noreg, !13, !DIExpression(DW_OP_LLVM_fragment, 16, 16), debug-location !18
    CALL64pcrel32 @bar, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, debug-location !17
    RETQ debug-location !18

buildLocationList() will produce a single entry (a composite location description containing the 456 and 789 fragment). Then MInsn will be:

DBG_VALUE 123, $noreg, !13, !DIExpression(), debug-location !16

and DbgMI:

DBG_VALUE 789, $noreg, !13, !DIExpression(DW_OP_LLVM_fragment, 16, 16), debug-location !18

and End null. As MInsn covers the whole scope, we will produce a single location description, but with DbgMI, which is only one of the two fragmented debug values which the variable is described by:

DW_AT_location	(DW_OP_piece 0x2, DW_OP_constu 0x315, DW_OP_stack_value, DW_OP_piece 0x2)

That input is hand-modified, and perhaps we can never get such debug information here in DwarfDebug when running through the normal pass chain. Still, it feels a bit iffy that a size optimization may result in us dropping information. My feeling is that this framework code should be able to handle that type of silly input.

Maybe we should be conservative and only try to produce single location descriptions in the most obvious cases. The most conservative approach I guess would be to only produce single location descriptions when all debug values are the same. Another is perhaps only doing this if all debug values are non-fragmented?

[NikolaPrica]

• Represent DbgVariable's single value representation by DebugLocEntry::Value insted of representing it with MachineInstr. Such representation will allow us to produce fragment single value representation (for now it is not supported).
• Update DwarfDebug::buildLocationList to check if produced list has single entry that is valid throughout whole variable's scope.

If this seems as a proper way, I suppose that we can make DebugLoc::Value now as a separate class ValueLoc or ValueLocation? Such refactor could go as a separate NFC patch if it is fine?

[aprantl]

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

610 ↗

(On Diff #201236)

typo: Check and has a single

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

247 ↗

(On Diff #201236)

///

270 ↗

(On Diff #201236)

per coding style this comment should be on the decl inside the class not on the implementation

1177 ↗

(On Diff #201236)

If the resulting list

[NikolaPrica]

• Update comments.

[dstenb]

Thanks a lot! This "location list -> single location description" detection seems more robust, and should help us moving towards supporting rewrites of single-entry location lists containing fragments also.

Nit: Perhaps we should move the Value class out of DebugLocEntry now that we use it for DbgVariable also? However, I don't have any good ideas on in where to place it (e.g. which header file). I think we can land this without doing such a refactoring though.

lib/CodeGen/AsmPrinter/DwarfDebug.h

114 ↗

(On Diff #201268)

Nit: Perhaps change single instruction -> single location?

[dstenb]

In D61600#1518010, @dstenb wrote:

Nit: Perhaps we should move the Value class out of DebugLocEntry now that we use it for DbgVariable also? However, I don't have any good ideas on in where to place it (e.g. which header file). I think we can land this without doing such a refactoring though.

Sorry, I saw now that you had already added a comment about that.

[NikolaPrica]

• Updated comment (single instruction -> single location)

@dstenb Nevermind. Thanks for the review anyway! Do you think that this is ready for landing now?

[dstenb]

In D61600#1518157, @NikolaPrica wrote:

Do you think that this is ready for landing now?

LGTM, but I think it would be good to see if @aprantl has any more comments on this.

[aprantl]

LGTM. Have you tested the performance when compiling Clang with ASAN enabled to make sure we're not accidentally regressing because of the more thorough single-location check?

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

364 ↗

(On Diff #201528)

personally I'd write this as CurElem->first == FrameReg not a big deal, though

lib/CodeGen/AsmPrinter/DwarfDebug.h

119 ↗

(On Diff #201528)

NFC commit for later, make this Optional<unsigned>

[NikolaPrica]

• Rebase.

[NikolaPrica]

Have you tested the performance when compiling Clang with ASAN enabled to make sure we're not accidentally regressing because of the more thorough single-location check?

I've built LLVM with '-DLLVM_USE_SANITIZER=Address' using downloaded clang binary to produce private clang binary. I've run tests and compiled latest version of gdb with new private binary and I didn't see any regression there. Is that what you meant?

[dstenb]

In D61600#1527114, @NikolaPrica wrote:

Have you tested the performance when compiling Clang with ASAN enabled to make sure we're not accidentally regressing because of the more thorough single-location check?

I've built LLVM with '-DLLVM_USE_SANITIZER=Address' using downloaded clang binary to produce private clang binary. I've run tests and compiled latest version of gdb with new private binary and I didn't see any regression there. Is that what you meant?

I think that the idea was to see if this patch increases the compilation time when building such a Clang binary.

(Just make sure to use the RelWithDebInfo build type also.)

[aprantl]

In D61600#1527182, @dstenb wrote:

In D61600#1527114, @NikolaPrica wrote:

Have you tested the performance when compiling Clang with ASAN enabled to make sure we're not accidentally regressing because of the more thorough single-location check?

I've built LLVM with '-DLLVM_USE_SANITIZER=Address' using downloaded clang binary to produce private clang binary. I've run tests and compiled latest version of gdb with new private binary and I didn't see any regression there. Is that what you meant?

I think that the idea was to see if this patch increases the compilation time when building such a Clang binary.

(Just make sure to use the RelWithDebInfo build type also.)

Yes. Enabling ASAN on a large C++ code base such as Clang is a great way to see compiler performance problems.
Can you post the output of "ninja clean && time ninja clang" before and after applying your patch to the *host* compiler?

[NikolaPrica]

In D61600#1527581, @aprantl wrote:

In D61600#1527182, @dstenb wrote:

In D61600#1527114, @NikolaPrica wrote:

Have you tested the performance when compiling Clang with ASAN enabled to make sure we're not accidentally regressing because of the more thorough single-location check?

I've built LLVM with '-DLLVM_USE_SANITIZER=Address' using downloaded clang binary to produce private clang binary. I've run tests and compiled latest version of gdb with new private binary and I didn't see any regression there. Is that what you meant?

I think that the idea was to see if this patch increases the compilation time when building such a Clang binary.

(Just make sure to use the RelWithDebInfo build type also.)

Yes. Enabling ASAN on a large C++ code base such as Clang is a great way to see compiler performance problems.
Can you post the output of "ninja clean && time ninja clang" before and after applying your patch to the *host* compiler?

Thanks for clarification!

before

real 46m29,670s
user 325m27,488s
sys 6m17,255s

after

real 46m41,562s
user 326m16,497s
sys 6m18,981s

Considering user + sys there is 0.25% increment with patch.

[aprantl]

Thanks, that looks like an acceptable performance/accuracy tradeoff.

[NikolaPrica]

Thanks for the review and patience!

llvm/trunk/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

void llvm::calculateDbgEntityHistory(const MachineFunction *MF,
                                      const TargetRegisterInfo *TRI,
                                      DbgValueHistoryMap &DbgValues,
                                      DbgLabelInstrMap &DbgLabels) {
   BitVector ChangingRegs(TRI->getNumRegs());
   collectChangingRegs(MF, TRI, ChangingRegs);
 
   const TargetLowering *TLI = MF->getSubtarget().getTargetLowering();
   unsigned SP = TLI->getStackPointerRegisterToSaveRestore();
+  unsigned FrameReg = TRI->getFrameRegister(*MF);
   RegDescribedVarsMap RegVars;
   DbgValueEntriesMap LiveEntries;
   for (const auto &MBB : *MF) {
     for (const auto &MI : MBB) {
       if (!MI.isDebugInstr()) {
         // Not a DBG_VALUE instruction. It may clobber registers which describe
         // some variables.
         for (const MachineOperand &MO : MI.operands()) {
for (const auto &MBB : *MF) {
 
     // Make sure locations for register-described variables are valid only
     // until the end of the basic block (unless it's the last basic block, in
     // which case let their liveness run off to the end of the function).
     if (!MBB.empty() && &MBB != &MF->back()) {
       for (auto I = RegVars.begin(), E = RegVars.end(); I != E;) {
         auto CurElem = I++; // CurElem can be erased below.
         if (TRI->isVirtualRegister(CurElem->first) ||
-            ChangingRegs.test(CurElem->first))
+            ChangingRegs.test(CurElem->first) ||
+            CurElem->first == FrameReg)
           clobberRegisterUses(RegVars, CurElem, DbgValues, LiveEntries,
                               MBB.back());
       }
     }
   }
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

DIE *DwarfCompileUnit::constructVariableDIEImpl(const DbgVariable &DV,
   // Add variable address.
 
   unsigned Offset = DV.getDebugLocListIndex();
   if (Offset != ~0U) {
     addLocationList(*VariableDie, dwarf::DW_AT_location, Offset);
     return VariableDie;
   }
 
-  // Check if variable is described by a DBG_VALUE instruction.
-  if (const MachineInstr *DVInsn = DV.getMInsn()) {
-    assert(DVInsn->getNumOperands() == 4);
-    if (DVInsn->getOperand(0).isReg()) {
-      auto RegOp = DVInsn->getOperand(0);
-      auto Op1 = DVInsn->getOperand(1);
-      // If the second operand is an immediate, this is an indirect value.
-      assert((!Op1.isImm() || (Op1.getImm() == 0)) && "unexpected offset");
-      MachineLocation Location(RegOp.getReg(), Op1.isImm());
-      addVariableAddress(DV, *VariableDie, Location);
-    } else if (DVInsn->getOperand(0).isImm()) {
-      // This variable is described by a single constant.
-      // Check whether it has a DIExpression.
+  // Check if variable has a single location description.
+  if (auto *DVal = DV.getValueLoc()) {
+    if (DVal->isLocation())
+      addVariableAddress(DV, *VariableDie, DVal->getLoc());
+    else if (DVal->isInt()) {
       auto *Expr = DV.getSingleExpression();
       if (Expr && Expr->getNumElements()) {
         DIELoc *Loc = new (DIEValueAllocator) DIELoc;
         DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
         // If there is an expression, emit raw unsigned bytes.
         DwarfExpr.addFragmentOffset(Expr);
-        DwarfExpr.addUnsignedConstant(DVInsn->getOperand(0).getImm());
+        DwarfExpr.addUnsignedConstant(DVal->getInt());
         DwarfExpr.addExpression(Expr);
         addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
       } else
-        addConstantValue(*VariableDie, DVInsn->getOperand(0), DV.getType());
-    } else if (DVInsn->getOperand(0).isFPImm())
-      addConstantFPValue(*VariableDie, DVInsn->getOperand(0));
-    else if (DVInsn->getOperand(0).isCImm())
-      addConstantValue(*VariableDie, DVInsn->getOperand(0).getCImm(),
-                       DV.getType());
-
+        addConstantValue(*VariableDie, DVal->getInt(), DV.getType());
+    } else if (DVal->isConstantFP()) {
+      addConstantFPValue(*VariableDie, DVal->getConstantFP());
+    } else if (DVal->isConstantInt()) {
+      addConstantValue(*VariableDie, DVal->getConstantInt(), DV.getType());
+    }
     return VariableDie;
   }
 
   // .. else use frame index.
   if (!DV.hasFrameIndexExprs())
     return VariableDie;
 
   Optional<unsigned> NVPTXAddressSpace;

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_CODEGEN_ASMPRINTER_DWARFDEBUG_H
 #define LLVM_LIB_CODEGEN_ASMPRINTER_DWARFDEBUG_H
 
 #include "AddressPool.h"
 #include "DebugLocStream.h"
+#include "DebugLocEntry.h"
 #include "DwarfFile.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"

 /// This class is used to track local variable information.
 ///
 /// Variables can be created from allocas, in which case they're generated from
 /// the MMI table.  Such variables can have multiple expressions and frame
 /// indices.
 ///
 /// Variables can be created from \c DBG_VALUE instructions.  Those whose
 /// location changes over time use \a DebugLocListIndex, while those with a
-/// single instruction use \a MInsn and (optionally) a single entry of \a Expr.
+/// single location use \a ValueLoc and (optionally) a single entry of \a Expr.
 ///
 /// Variables that have been optimized out use none of these fields.
 class DbgVariable : public DbgEntity {
-  unsigned DebugLocListIndex = ~0u;          /// Offset in DebugLocs.
-  const MachineInstr *MInsn = nullptr;       /// DBG_VALUE instruction.
+  /// Offset in DebugLocs.
+  unsigned DebugLocListIndex = ~0u;
+  /// Single value location description.
+  std::unique_ptr<DebugLocEntry::Value> ValueLoc = nullptr;
 
   struct FrameIndexExpr {
     int FI;
     const DIExpression *Expr;
   };
   mutable SmallVector<FrameIndexExpr, 1>
       FrameIndexExprs; /// Frame index + expression.
 
 public:
   /// Construct a DbgVariable.
   ///
   /// Creates a variable without any DW_AT_location.  Call \a initializeMMI()
   /// for MMI entries, or \a initializeDbgValue() for DBG_VALUE instructions.
   DbgVariable(const DILocalVariable *V, const DILocation *IA)
       : DbgEntity(V, IA, DbgVariableKind) {}
 
   /// Initialize from the MMI table.
   void initializeMMI(const DIExpression *E, int FI) {
     assert(FrameIndexExprs.empty() && "Already initialized?");
-    assert(!MInsn && "Already initialized?");
+    assert(!ValueLoc.get() && "Already initialized?");
 
     assert((!E || E->isValid()) && "Expected valid expression");
     assert(FI != std::numeric_limits<int>::max() && "Expected valid index");
 
     FrameIndexExprs.push_back({FI, E});
   }
 
-  /// Initialize from a DBG_VALUE instruction.
-  void initializeDbgValue(const MachineInstr *DbgValue) {
+  // Initialize variable's location.
+  void initializeDbgValue(DebugLocEntry::Value Value) {
     assert(FrameIndexExprs.empty() && "Already initialized?");
-    assert(!MInsn && "Already initialized?");
-
-    assert(getVariable() == DbgValue->getDebugVariable() && "Wrong variable");
-    assert(getInlinedAt() == DbgValue->getDebugLoc()->getInlinedAt() &&
-           "Wrong inlined-at");
+    assert(!ValueLoc && "Already initialized?");
+    assert(!Value.getExpression()->isFragment() && "Fragments not supported.");
 
-    MInsn = DbgValue;
-    if (auto *E = DbgValue->getDebugExpression())
+    ValueLoc = llvm::make_unique<DebugLocEntry::Value>(Value);
+    if (auto *E = ValueLoc->getExpression())
       if (E->getNumElements())
         FrameIndexExprs.push_back({0, E});
   }
 
+  /// Initialize from a DBG_VALUE instruction.
+  void initializeDbgValue(const MachineInstr *DbgValue);
+
   // Accessors.
   const DILocalVariable *getVariable() const {
     return cast<DILocalVariable>(getEntity());
   }
 
   const DIExpression *getSingleExpression() const {
-    assert(MInsn && FrameIndexExprs.size() <= 1);
+    assert(ValueLoc.get() && FrameIndexExprs.size() <= 1);
     return FrameIndexExprs.size() ? FrameIndexExprs[0].Expr : nullptr;
   }
 
   void setDebugLocListIndex(unsigned O) { DebugLocListIndex = O; }
   unsigned getDebugLocListIndex() const { return DebugLocListIndex; }
   StringRef getName() const { return getVariable()->getName(); }
-  const MachineInstr *getMInsn() const { return MInsn; }
+  const DebugLocEntry::Value *getValueLoc() const { return ValueLoc.get(); }
   /// Get the FI entries, sorted by fragment offset.
   ArrayRef<FrameIndexExpr> getFrameIndexExprs() const;
   bool hasFrameIndexExprs() const { return !FrameIndexExprs.empty(); }
   void addMMIEntry(const DbgVariable &V);
 
   // Translate tag to proper Dwarf tag.
   dwarf::Tag getTag() const {
     // FIXME: Why don't we just infer this tag and store it all along?
bool isObjectPointer() const {
     if (getVariable()->isObjectPointer())
       return true;
     if (getType()->isObjectPointer())
       return true;
     return false;
   }
 
   bool hasComplexAddress() const {
-    assert(MInsn && "Expected DBG_VALUE, not MMI variable");
+    assert(ValueLoc.get() && "Expected DBG_VALUE, not MMI variable");
     assert((FrameIndexExprs.empty() ||
             (FrameIndexExprs.size() == 1 &&
              FrameIndexExprs[0].Expr->getNumElements())) &&
            "Invalid Expr for DBG_VALUE");
     return !FrameIndexExprs.empty();
   }
 
   bool isBlockByrefVariable() const;
class DwarfDebug : public DebugHandlerBase {
   void recordSourceLine(unsigned Line, unsigned Col, const MDNode *Scope,
                         unsigned Flags);
 
   /// Populate LexicalScope entries with variables' info.
   void collectEntityInfo(DwarfCompileUnit &TheCU, const DISubprogram *SP,
                          DenseSet<InlinedEntity> &ProcessedVars);
 
   /// Build the location list for all DBG_VALUEs in the
-  /// function that describe the same variable.
-  void buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
+  /// function that describe the same variable. If the resulting 
+  /// list has only one entry that is valid for entire variable's
+  /// scope return true.
+  bool buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
                          const DbgValueHistoryMap::Entries &Entries);
 
   /// Collect variable information from the side table maintained by MF.
   void collectVariableInfoFromMFTable(DwarfCompileUnit &TheCU,
                                       DenseSet<InlinedEntity> &P);
 
   /// Emit the reference to the section.
   void emitSectionReference(const DwarfCompileUnit &CU);

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

for (unsigned i = 0, N = Elements.size(); i < N; ++i) {
       auto *DT = cast<DIDerivedType>(Elements[i]);
       if (getName() == DT->getName())
         return DT->getBaseType();
     }
   }
   return Ty;
 }
 
+/// Get .debug_loc entry for the instruction range starting at MI.
+static DebugLocEntry::Value getDebugLocValue(const MachineInstr *MI) {
+  const DIExpression *Expr = MI->getDebugExpression();
+  assert(MI->getNumOperands() == 4);
+  if (MI->getOperand(0).isReg()) {
+    auto RegOp = MI->getOperand(0);
+    auto Op1 = MI->getOperand(1);
+    // If the second operand is an immediate, this is a
+    // register-indirect address.
+    assert((!Op1.isImm() || (Op1.getImm() == 0)) && "unexpected offset");
+    MachineLocation MLoc(RegOp.getReg(), Op1.isImm());
+    return DebugLocEntry::Value(Expr, MLoc);
+  }
+  if (MI->getOperand(0).isImm())
+    return DebugLocEntry::Value(Expr, MI->getOperand(0).getImm());
+  if (MI->getOperand(0).isFPImm())
+    return DebugLocEntry::Value(Expr, MI->getOperand(0).getFPImm());
+  if (MI->getOperand(0).isCImm())
+    return DebugLocEntry::Value(Expr, MI->getOperand(0).getCImm());
+
+  llvm_unreachable("Unexpected 4-operand DBG_VALUE instruction!");
+}
+
+void DbgVariable::initializeDbgValue(const MachineInstr *DbgValue) {
+  assert(FrameIndexExprs.empty() && "Already initialized?");
+  assert(!ValueLoc.get() && "Already initialized?");
+
+  assert(getVariable() == DbgValue->getDebugVariable() && "Wrong variable");
+  assert(getInlinedAt() == DbgValue->getDebugLoc()->getInlinedAt() &&
+         "Wrong inlined-at");
+
+  ValueLoc = llvm::make_unique<DebugLocEntry::Value>(getDebugLocValue(DbgValue));
+  if (auto *E = DbgValue->getDebugExpression())
+    if (E->getNumElements())
+      FrameIndexExprs.push_back({0, E});
+}
+
 ArrayRef<DbgVariable::FrameIndexExpr> DbgVariable::getFrameIndexExprs() const {
   if (FrameIndexExprs.size() == 1)
     return FrameIndexExprs;
 
   assert(llvm::all_of(FrameIndexExprs,
                       [](const FrameIndexExpr &A) {
                         return A.Expr->isFragment();
                       }) &&
          "multiple FI expressions without DW_OP_LLVM_fragment");
   llvm::sort(FrameIndexExprs,
              [](const FrameIndexExpr &A, const FrameIndexExpr &B) -> bool {
                return A.Expr->getFragmentInfo()->OffsetInBits <
                       B.Expr->getFragmentInfo()->OffsetInBits;
              });
 
   return FrameIndexExprs;
 }
 
 void DbgVariable::addMMIEntry(const DbgVariable &V) {
-  assert(DebugLocListIndex == ~0U && !MInsn && "not an MMI entry");
-  assert(V.DebugLocListIndex == ~0U && !V.MInsn && "not an MMI entry");
+  assert(DebugLocListIndex == ~0U && !ValueLoc.get() && "not an MMI entry");
+  assert(V.DebugLocListIndex == ~0U && !V.ValueLoc.get() && "not an MMI entry");
   assert(V.getVariable() == getVariable() && "conflicting variable");
   assert(V.getInlinedAt() == getInlinedAt() && "conflicting inlined-at location");
 
   assert(!FrameIndexExprs.empty() && "Expected an MMI entry");
   assert(!V.FrameIndexExprs.empty() && "Expected an MMI entry");
 
   // FIXME: This logic should not be necessary anymore, as we now have proper
   // deduplication. However, without it, we currently run into the assertion
if (DbgVariable *DbgVar = MFVars.lookup(Var))
       DbgVar->addMMIEntry(*RegVar);
     else if (InfoHolder.addScopeVariable(Scope, RegVar.get())) {
       MFVars.insert({Var, RegVar.get()});
       ConcreteEntities.push_back(std::move(RegVar));
     }
   }
 }
 
-// Get .debug_loc entry for the instruction range starting at MI.
-static DebugLocEntry::Value getDebugLocValue(const MachineInstr *MI) {
-  const DIExpression *Expr = MI->getDebugExpression();
-  assert(MI->getNumOperands() == 4);
-  if (MI->getOperand(0).isReg()) {
-    auto RegOp = MI->getOperand(0);
-    auto Op1 = MI->getOperand(1);
-    // If the second operand is an immediate, this is a
-    // register-indirect address.
-    assert((!Op1.isImm() || (Op1.getImm() == 0)) && "unexpected offset");
-    MachineLocation MLoc(RegOp.getReg(), Op1.isImm());
-    return DebugLocEntry::Value(Expr, MLoc);
+/// Determine whether a *singular* DBG_VALUE is valid for the entirety of its
+/// enclosing lexical scope. The check ensures there are no other instructions
+/// in the same lexical scope preceding the DBG_VALUE and that its range is
+/// either open or otherwise rolls off the end of the scope.
+static bool validThroughout(LexicalScopes &LScopes,
+                            const MachineInstr *DbgValue,
+                            const MachineInstr *RangeEnd) {
+  assert(DbgValue->getDebugLoc() && "DBG_VALUE without a debug location");
+  auto MBB = DbgValue->getParent();
+  auto DL = DbgValue->getDebugLoc();
+  auto *LScope = LScopes.findLexicalScope(DL);
+  // Scope doesn't exist; this is a dead DBG_VALUE.
+  if (!LScope)
+    return false;
+  auto &LSRange = LScope->getRanges();
+  if (LSRange.size() == 0)
+    return false;
+
+  // Determine if the DBG_VALUE is valid at the beginning of its lexical block.
+  const MachineInstr *LScopeBegin = LSRange.front().first;
+  // Early exit if the lexical scope begins outside of the current block.
+  if (LScopeBegin->getParent() != MBB)
+    return false;
+  MachineBasicBlock::const_reverse_iterator Pred(DbgValue);
+  for (++Pred; Pred != MBB->rend(); ++Pred) {
+    if (Pred->getFlag(MachineInstr::FrameSetup))
+      break;
+    auto PredDL = Pred->getDebugLoc();
+    if (!PredDL || Pred->isMetaInstruction())
+      continue;
+    // Check whether the instruction preceding the DBG_VALUE is in the same
+    // (sub)scope as the DBG_VALUE.
+    if (DL->getScope() == PredDL->getScope())
+      return false;
+    auto *PredScope = LScopes.findLexicalScope(PredDL);
+    if (!PredScope || LScope->dominates(PredScope))
+      return false;
   }
-  if (MI->getOperand(0).isImm())
-    return DebugLocEntry::Value(Expr, MI->getOperand(0).getImm());
-  if (MI->getOperand(0).isFPImm())
-    return DebugLocEntry::Value(Expr, MI->getOperand(0).getFPImm());
-  if (MI->getOperand(0).isCImm())
-    return DebugLocEntry::Value(Expr, MI->getOperand(0).getCImm());
 
-  llvm_unreachable("Unexpected 4-operand DBG_VALUE instruction!");
+  // If the range of the DBG_VALUE is open-ended, report success.
+  if (!RangeEnd)
+    return true;
+
+  // Fail if there are instructions belonging to our scope in another block.
+  const MachineInstr *LScopeEnd = LSRange.back().second;
+  if (LScopeEnd->getParent() != MBB)
+    return false;
+
+  // Single, constant DBG_VALUEs in the prologue are promoted to be live
+  // throughout the function. This is a hack, presumably for DWARF v2 and not
+  // necessarily correct. It would be much better to use a dbg.declare instead
+  // if we know the constant is live throughout the scope.
+  if (DbgValue->getOperand(0).isImm() && MBB->pred_empty())
+    return true;
+
+  return false;
 }
 
 /// Build the location list for all DBG_VALUEs in the function that
 /// describe the same variable. The resulting DebugLocEntries will have
 /// strict monotonically increasing begin addresses and will never
-/// overlap.
+/// overlap. If the resulting list has only one entry that is valid
+/// throughout variable's scope return true.
 //
 // See the definition of DbgValueHistoryMap::Entry for an explanation of the
 // different kinds of history map entries. One thing to be aware of is that if
 // a debug value is ended by another entry (rather than being valid until the
 // end of the function), that entry's instruction may or may not be included in
 // the range, depending on if the entry is a clobbering entry (it has an
 // instruction that clobbers one or more preceding locations), or if it is an
 // (overlapping) debug value entry. This distinction can be seen in the example

 // 4        [DbgValue, ~0, DBG_VALUE @g, [...] (fragment 0, 96)]
 //
 // Output [start, end) [Value...]:
 //
 // [0-1)    [(reg0, fragment 0, 32)]
 // [1-3)    [(reg0, fragment 0, 32), (reg1, fragment 32, 32)]
 // [3-4)    [(reg1, fragment 32, 32), (123, fragment 64, 32)]
 // [4-)     [(@g, fragment 0, 96)]
-void DwarfDebug::buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
+bool DwarfDebug::buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
                                    const DbgValueHistoryMap::Entries &Entries) {
   using OpenRange =
       std::pair<DbgValueHistoryMap::EntryIndex, DebugLocEntry::Value>;
   SmallVector<OpenRange, 4> OpenRanges;
+  bool isSafeForSingleLocation = true;
+  const MachineInstr *StartDebugMI = nullptr;
+  const MachineInstr *EndMI = nullptr;
 
   for (auto EB = Entries.begin(), EI = EB, EE = Entries.end(); EI != EE; ++EI) {
     const MachineInstr *Instr = EI->getInstr();
 
     // Remove all values that are no longer live.
     size_t Index = std::distance(EB, EI);
     auto Last =
         remove_if(OpenRanges, [&](OpenRange &R) { return R.first <= Index; });
     OpenRanges.erase(Last, OpenRanges.end());
 
     // If we are dealing with a clobbering entry, this iteration will result in
     // a location list entry starting after the clobbering instruction.
     const MCSymbol *StartLabel =
         EI->isClobber() ? getLabelAfterInsn(Instr) : getLabelBeforeInsn(Instr);
     assert(StartLabel &&
            "Forgot label before/after instruction starting a range!");
 
     const MCSymbol *EndLabel;
-    if (std::next(EI) == Entries.end())
+    if (std::next(EI) == Entries.end()) {
       EndLabel = Asm->getFunctionEnd();
+      if (EI->isClobber())
+        EndMI = EI->getInstr();
+    }
     else if (std::next(EI)->isClobber())
       EndLabel = getLabelAfterInsn(std::next(EI)->getInstr());
     else
       EndLabel = getLabelBeforeInsn(std::next(EI)->getInstr());
     assert(EndLabel && "Forgot label after instruction ending a range!");
 
     if (EI->isDbgValue())
       LLVM_DEBUG(dbgs() << "DotDebugLoc: " << *Instr << "\n");
 
     // If this history map entry has a debug value, add that to the list of
-    // open ranges.
+    // open ranges and check if its location is valid for a single value
+    // location.
     if (EI->isDbgValue()) {
       // Do not add undef debug values, as they are redundant information in
       // the location list entries. An undef debug results in an empty location
       // description. If there are any non-undef fragments then padding pieces
       // with empty location descriptions will automatically be inserted, and if
       // all fragments are undef then the whole location list entry is
       // redundant.
       if (!Instr->isUndefDebugValue()) {
         auto Value = getDebugLocValue(Instr);
         OpenRanges.emplace_back(EI->getEndIndex(), Value);
+
+        // TODO: Add support for single value fragment locations.
+        if (Instr->getDebugExpression()->isFragment())
+          isSafeForSingleLocation = false;
+
+        if (!StartDebugMI)
+          StartDebugMI = Instr;
+      } else {
+        isSafeForSingleLocation = false;
       }
     }
 
     // Location list entries with empty location descriptions are redundant
     // information in DWARF, so do not emit those.
     if (OpenRanges.empty())
       continue;
 
LLVM_DEBUG({
         Value.dump();
       dbgs() << "-----\n";
     });
 
     auto PrevEntry = std::next(CurEntry);
     if (PrevEntry != DebugLoc.rend() && PrevEntry->MergeRanges(*CurEntry))
       DebugLoc.pop_back();
   }
+
+  return DebugLoc.size() == 1 && isSafeForSingleLocation &&
+         validThroughout(LScopes, StartDebugMI, EndMI);
 }
 
 DbgEntity *DwarfDebug::createConcreteEntity(DwarfCompileUnit &TheCU,
                                             LexicalScope &Scope,
                                             const DINode *Node,
                                             const DILocation *Location,
                                             const MCSymbol *Sym) {
   ensureAbstractEntityIsCreatedIfScoped(TheCU, Node, Scope.getScopeNode());
   if (isa<const DILocalVariable>(Node)) {
     ConcreteEntities.push_back(
         llvm::make_unique<DbgVariable>(cast<const DILocalVariable>(Node),
                                        Location));
     InfoHolder.addScopeVariable(&Scope,
         cast<DbgVariable>(ConcreteEntities.back().get()));
   } else if (isa<const DILabel>(Node)) {
     ConcreteEntities.push_back(
         llvm::make_unique<DbgLabel>(cast<const DILabel>(Node),
                                     Location, Sym));
     InfoHolder.addScopeLabel(&Scope,
         cast<DbgLabel>(ConcreteEntities.back().get()));
   }
   return ConcreteEntities.back().get();
 }
 
-/// Determine whether a *singular* DBG_VALUE is valid for the entirety of its
-/// enclosing lexical scope. The check ensures there are no other instructions
-/// in the same lexical scope preceding the DBG_VALUE and that its range is
-/// either open or otherwise rolls off the end of the scope.
-static bool validThroughout(LexicalScopes &LScopes,
-                            const MachineInstr *DbgValue,
-                            const MachineInstr *RangeEnd) {
-  assert(DbgValue->getDebugLoc() && "DBG_VALUE without a debug location");
-  auto MBB = DbgValue->getParent();
-  auto DL = DbgValue->getDebugLoc();
-  auto *LScope = LScopes.findLexicalScope(DL);
-  // Scope doesn't exist; this is a dead DBG_VALUE.
-  if (!LScope)
-    return false;
-  auto &LSRange = LScope->getRanges();
-  if (LSRange.size() == 0)
-    return false;
-
-  // Determine if the DBG_VALUE is valid at the beginning of its lexical block.
-  const MachineInstr *LScopeBegin = LSRange.front().first;
-  // Early exit if the lexical scope begins outside of the current block.
-  if (LScopeBegin->getParent() != MBB)
-    return false;
-  MachineBasicBlock::const_reverse_iterator Pred(DbgValue);
-  for (++Pred; Pred != MBB->rend(); ++Pred) {
-    if (Pred->getFlag(MachineInstr::FrameSetup))
-      break;
-    auto PredDL = Pred->getDebugLoc();
-    if (!PredDL || Pred->isMetaInstruction())
-      continue;
-    // Check whether the instruction preceding the DBG_VALUE is in the same
-    // (sub)scope as the DBG_VALUE.
-    if (DL->getScope() == PredDL->getScope())
-      return false;
-    auto *PredScope = LScopes.findLexicalScope(PredDL);
-    if (!PredScope || LScope->dominates(PredScope))
-      return false;
-  }
-
-  // If the range of the DBG_VALUE is open-ended, report success.
-  if (!RangeEnd)
-    return true;
-
-  // Fail if there are instructions belonging to our scope in another block.
-  const MachineInstr *LScopeEnd = LSRange.back().second;
-  if (LScopeEnd->getParent() != MBB)
-    return false;
-
-  // Single, constant DBG_VALUEs in the prologue are promoted to be live
-  // throughout the function. This is a hack, presumably for DWARF v2 and not
-  // necessarily correct. It would be much better to use a dbg.declare instead
-  // if we know the constant is live throughout the scope.
-  if (DbgValue->getOperand(0).isImm() && MBB->pred_empty())
-    return true;
-
-  return false;
-}
-
 // Find variables for each lexical scope.
 void DwarfDebug::collectEntityInfo(DwarfCompileUnit &TheCU,
                                    const DISubprogram *SP,
                                    DenseSet<InlinedEntity> &Processed) {
   // Grab the variable info that was squirreled away in the MMI side-table.
   collectVariableInfoFromMFTable(TheCU, Processed);
 
   for (const auto &I : DbgValues) {
DbgVariable *RegVar = cast<DbgVariable>(createConcreteEntity(TheCU,
                                             *Scope, LocalVar, IV.second));
 
     const MachineInstr *MInsn = HistoryMapEntries.front().getInstr();
     assert(MInsn->isDebugValue() && "History must begin with debug value");
 
     // Check if there is a single DBG_VALUE, valid throughout the var's scope.
     // If the history map contains a single debug value, there may be an
     // additional entry which clobbers the debug value.
+    size_t HistSize = HistoryMapEntries.size();
     bool SingleValueWithClobber =
-        HistoryMapEntries.size() == 2 && HistoryMapEntries[1].isClobber();
-    if (HistoryMapEntries.size() == 1 || SingleValueWithClobber) {
+        HistSize == 2 && HistoryMapEntries[1].isClobber();
+    if (HistSize == 1 || SingleValueWithClobber) {
       const auto *End =
           SingleValueWithClobber ? HistoryMapEntries[1].getInstr() : nullptr;
       if (validThroughout(LScopes, MInsn, End)) {
         RegVar->initializeDbgValue(MInsn);
         continue;
       }
     }
 
     // Do not emit location lists if .debug_loc secton is disabled.
     if (!useLocSection())
       continue;
 
     // Handle multiple DBG_VALUE instructions describing one variable.
     DebugLocStream::ListBuilder List(DebugLocs, TheCU, *Asm, *RegVar, *MInsn);
 
     // Build the location list for this variable.
     SmallVector<DebugLocEntry, 8> Entries;
-    buildLocationList(Entries, HistoryMapEntries);
+    bool isValidSingleLocation = buildLocationList(Entries, HistoryMapEntries);
+
+    // Check whether buildLocationList managed to merge all locations to one
+    // that is valid throughout the variable's scope. If so, produce single
+    // value location.
+    if (isValidSingleLocation) {
+      RegVar->initializeDbgValue(Entries[0].getValues()[0]);
+      continue;
+    }
 
     // If the variable has a DIBasicType, extract it.  Basic types cannot have
     // unique identifiers, so don't bother resolving the type with the
     // identifier map.
     const DIBasicType *BT = dyn_cast<DIBasicType>(
         static_cast<const Metadata *>(LocalVar->getType()));
 
     // Finalize the entry by lowering it into a DWARF bytestream.
if (useSegmentedStringOffsetsTable()) {
     emitStringOffsetsTableHeader();
     StringOffsetsSection = Asm->getObjFileLowering().getDwarfStrOffSection();
   }
   DwarfFile &Holder = useSplitDwarf() ? SkeletonHolder : InfoHolder;
   Holder.emitStrings(Asm->getObjFileLowering().getDwarfStrSection(),
                      StringOffsetsSection, /* UseRelativeOffsets = */ true);
 }
 
-void DwarfDebug::emitDebugLocEntry(ByteStreamer &Streamer, const
-                                   DebugLocStream::Entry &Entry,
+void DwarfDebug::emitDebugLocEntry(ByteStreamer &Streamer,
+                                   const DebugLocStream::Entry &Entry,
                                    const DwarfCompileUnit *CU) {
   auto &&Comments = DebugLocs.getComments(Entry);
   auto Comment = Comments.begin();
   auto End = Comments.end();
 
   // The expressions are inserted into a byte stream rather early (see
   // DwarfExpression::addExpression) so for those ops (e.g. DW_OP_convert) that
   // need to reference a base_type DIE the offset of that DIE is not yet known.

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h

public:
   void addSourceLine(DIE &Die, const DIType *Ty);
   void addSourceLine(DIE &Die, const DIObjCProperty *Ty);
 
   /// Add constant value entry in variable DIE.
   void addConstantValue(DIE &Die, const MachineOperand &MO, const DIType *Ty);
   void addConstantValue(DIE &Die, const ConstantInt *CI, const DIType *Ty);
   void addConstantValue(DIE &Die, const APInt &Val, const DIType *Ty);
   void addConstantValue(DIE &Die, const APInt &Val, bool Unsigned);
+  void addConstantValue(DIE &Die, uint64_t Val, const DIType *Ty);
   void addConstantValue(DIE &Die, bool Unsigned, uint64_t Val);
 
   /// Add constant value entry in variable DIE.
   void addConstantFPValue(DIE &Die, const MachineOperand &MO);
   void addConstantFPValue(DIE &Die, const ConstantFP *CFP);
 
   /// Add a linkage name, if it isn't empty.
   void addLinkageName(DIE &Die, StringRef LinkageName);

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

 
 void DwarfUnit::addConstantValue(DIE &Die, const MachineOperand &MO,
                                  const DIType *Ty) {
   assert(MO.isImm() && "Invalid machine operand!");
 
   addConstantValue(Die, isUnsignedDIType(DD, Ty), MO.getImm());
 }
 
+void DwarfUnit::addConstantValue(DIE &Die, uint64_t Val, const DIType *Ty) {
+  addConstantValue(Die, isUnsignedDIType(DD, Ty), Val);
+}
+
 void DwarfUnit::addConstantValue(DIE &Die, bool Unsigned, uint64_t Val) {
   // FIXME: This is a bit conservative/simple - it emits negative values always
   // sign extended to 64 bits rather than minimizing the number of bytes.
   addUInt(Die, dwarf::DW_AT_const_value,
           Unsigned ? dwarf::DW_FORM_udata : dwarf::DW_FORM_sdata, Val);
 }
 
 void DwarfUnit::addConstantValue(DIE &Die, const APInt &Val, const DIType *Ty) {

llvm/trunk/test/DebugInfo/MIR/X86/dbg-stack-value-range.mir

+# RUN: llc -start-before=livedebugvalues %s -filetype=asm -o -| FileCheck %s
+#
+# DbgEntityHistoryCalculator should close variable's range at the end of
+# the basic block when variable is referenced through non-changing frame
+# pointer register. Having BB1 that branches to BB2 and BB3, where
+# BB2 falls through to BB3, variable can have different locations at BB1 and BB2.
+# Since input locations at BB3 for same variable are different LiveDebugValues
+# wont generate DBG_VALUE for BB3. If last variable location at BB2 is
+# non-changing register, DbgEntityHistoryCalculator will extend range of
+# DBG_VALUE from BB2 to whole BB3 and thus produce incorrect range for
+# case when we took branch BB3 from BB1.
+#
+# Verifies that "local1" stack location is ended at the end of the block (.Ltmp6).
+#
+# CHECK: .Ltmp4:
+# CHECK-NEXT:   #DEBUG_VALUE: foo:local1 <- [DW_OP_constu 4, DW_OP_minus, DW_OP_deref] $rbp
+# CHECK:        jmp     .LBB0_2
+# CHECK-NEXT: .Ltmp6:
+# CHECK: .Lfunc_end0:
+#
+# CHECK: .Linfo_string7:
+# CHECK-NEXT: .asciz  "local1"
+#
+# CHECK: .Ldebug_loc2:
+# CHECK-NEXT: .quad   .Ltmp1-.Lfunc_begin0
+# CHECK-NEXT: .quad   .Ltmp4-.Lfunc_begin0
+# CHECK-NEXT: .short  1                       # Loc expr size
+# CHECK-NEXT: .byte   94                      # super-register DW_OP_reg14
+# CHECK-NEXT: .quad   .Ltmp4-.Lfunc_begin0
+# CHECK-NEXT: .quad   .Ltmp6-.Lfunc_begin0
+# CHECK-NEXT: .short  2                       # Loc expr size
+# CHECK-NEXT: .byte   118                     # DW_OP_breg6
+# CHECK-NEXT: .byte   124                     # -4
+#
+# CHECK: .long   .Ldebug_loc2            # DW_AT_location
+# CHECK-NEXT: .long   .Linfo_string7          # DW_AT_name
+
+
+--- |
+  ; ModuleID = 'dbg-stack-value-range.ll'
+  source_filename = "dbg-stack-value-range.c"
+  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-unknown-linux-gnu"
+  
+  ; Function Attrs: nounwind uwtable
+  define dso_local i32 @foo(i32 %X) local_unnamed_addr #0 !dbg !7 {
+  entry:
+    %local1 = alloca i32, align 4
+    call void @llvm.dbg.value(metadata i32 %X, metadata !12, metadata !DIExpression()), !dbg !15
+    %0 = bitcast i32* %local1 to i8*, !dbg !15
+    call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %0), !dbg !15
+    call void @llvm.dbg.value(metadata i32 5, metadata !14, metadata !DIExpression()), !dbg !15
+    %call = tail call i32 (...) @check(), !dbg !15
+    %tobool = icmp eq i32 %call, 0, !dbg !15
+    br i1 %tobool, label %if.else, label %if.then, !dbg !15
+  
+  if.then:                                          ; preds = %entry
+    call void @llvm.dbg.value(metadata i32 4, metadata !13, metadata !DIExpression()), !dbg !15
+    store i32 4, i32* %local1, align 4, !dbg !15, !tbaa !16
+    call void @llvm.dbg.value(metadata i32* %local1, metadata !13, metadata !DIExpression(DW_OP_deref)), !dbg !15
+    %call1 = call i32 @init(i32* nonnull %local1), !dbg !15
+    call void @llvm.dbg.value(metadata i32 undef, metadata !14, metadata !DIExpression()), !dbg !15
+    br label %if.end, !dbg !15
+  
+  if.else:                                          ; preds = %entry
+    call void @llvm.dbg.value(metadata i32 5, metadata !13, metadata !DIExpression()), !dbg !15
+    store i32 5, i32* %local1, align 4, !dbg !15, !tbaa !16
+    br label %if.end
+  
+  if.end:                                           ; preds = %if.else, %if.then
+    %1 = bitcast i32* %local1 to i8*, !dbg !15
+    call void @llvm.dbg.value(metadata i32 undef, metadata !14, metadata !DIExpression()), !dbg !15
+    %call2 = call i32 (...) @init2(), !dbg !15
+    call void @llvm.dbg.value(metadata i32 undef, metadata !14, metadata !DIExpression()), !dbg !15
+    %2 = load i32, i32* %local1, align 4, !dbg !15, !tbaa !16
+    call void @llvm.dbg.value(metadata i32 %2, metadata !13, metadata !DIExpression()), !dbg !15
+    call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %1), !dbg !15
+    ret i32 %2, !dbg !15
+  }
+  
+  ; Function Attrs: argmemonly nounwind
+  declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+  
+  declare dso_local i32 @check(...) local_unnamed_addr
+  
+  declare dso_local i32 @init(i32*) local_unnamed_addr
+  
+  declare dso_local i32 @init2(...) local_unnamed_addr
+  
+  ; Function Attrs: argmemonly nounwind
+  declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+  
+  ; Function Attrs: nounwind readnone speculatable
+  declare void @llvm.dbg.value(metadata, metadata, metadata)
+  
+  ; Function Attrs: nounwind
+  declare void @llvm.stackprotector(i8*, i8**)
+
+  attributes #0 = { nounwind uwtable "no-frame-pointer-elim"="false" "no-frame-pointer-elim-non-leaf" }
+  
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3, !4, !5}
+  !llvm.ident = !{!6}
+  
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+  !1 = !DIFile(filename: "dbg-stack-value-range.c", directory: "/")
+  !2 = !{}
+  !3 = !{i32 2, !"Dwarf Version", i32 4}
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = !{i32 1, !"wchar_size", i32 4}
+  !6 = !{!"clang version 9.0.0"}
+  !7 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 11, type: !8, scopeLine: 11, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
+  !8 = !DISubroutineType(types: !9)
+  !9 = !{!10, !10}
+  !10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !11 = !{!12, !13, !14}
+  !12 = !DILocalVariable(name: "X", arg: 1, scope: !7, file: !1, line: 11, type: !10)
+  !13 = !DILocalVariable(name: "local1", scope: !7, file: !1, line: 12, type: !10)
+  !14 = !DILocalVariable(name: "local2", scope: !7, file: !1, line: 12, type: !10)
+  !15 = !DILocation(line: 11, column: 13, scope: !7)
+  !16 = !{!17, !17, i64 0}
+  !17 = !{!"int", !18, i64 0}
+  !18 = !{!"omnipotent char", !19, i64 0}
+  !19 = !{!"Simple C/C++ TBAA"}
+
+...
+---
+name:            foo
+alignment:       4
+frameInfo:       
+  stackSize:       24
+  offsetAdjustment: -24
+  maxAlignment:    4
+  adjustsStack:    true
+  hasCalls:        true
+fixedStack:      
+  - { id: 0, type: spill-slot, offset: -16, size: 8, alignment: 16, stack-id: 0, 
+      callee-saved-register: '$r14d', callee-saved-restored: true, debug-info-variable: '', 
+      debug-info-expression: '', debug-info-location: '' }
+stack:           
+  - { id: 0, name: local1, type: default, offset: -20, size: 4, alignment: 4, 
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true, 
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+body:             |
+  bb.0.entry:
+    successors: %bb.3(0x30000000), %bb.1(0x50000000)
+  
+    DBG_VALUE $edi, $noreg, !12, !DIExpression(), debug-location !15
+    frame-setup PUSH64r killed $rbp, implicit-def $rsp, implicit $rsp
+    CFI_INSTRUCTION def_cfa_offset 16
+    CFI_INSTRUCTION offset $rbp, -16
+    $rbp = frame-setup MOV64rr $rsp
+    CFI_INSTRUCTION def_cfa_register $rbp
+    $rsp = frame-setup SUB64ri8 $rsp, 16, implicit-def dead $eflags
+    DBG_VALUE 5, $noreg, !14, !DIExpression(), debug-location !15
+    $r14d = MOV32ri 4, implicit-def $r14
+    DBG_VALUE $r14d, $noreg, !13, !DIExpression(), debug-location !15
+    dead $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $al, debug-location !15
+    CALL64pcrel32 @check, csr_64, implicit $rsp, implicit $ssp, implicit $al, implicit-def $rsp, implicit-def $ssp, implicit-def $eax, debug-location !15
+    TEST32rr killed renamable $eax, renamable $eax, implicit-def $eflags, debug-location !15
+    JCC_1 %bb.3, 4, implicit killed $eflags, debug-location !15
+  
+  bb.1.if.then:
+    successors: %bb.3(0x80000000)
+  
+    MOV32mr $rbp, 1, $noreg, -4, $noreg, $r14d, debug-location !15 :: (store 4 into %ir.local1, !tbaa !16)
+    DBG_VALUE $rbp, $noreg, !13, !DIExpression(DW_OP_constu, 4, DW_OP_minus, DW_OP_deref), debug-location !15
+    renamable $rdi = LEA64r $rbp, 1, $noreg, -4, $noreg
+    CALL64pcrel32 @init, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp, implicit-def dead $eax, debug-location !15
+    DBG_VALUE $noreg, $noreg, !14, !DIExpression(), debug-location !15
+    JMP_1 %bb.3
+  
+  bb.3.if.end:
+    DBG_VALUE $noreg, $noreg, !14, !DIExpression(), debug-location !15
+    dead $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $al, debug-location !15
+    CALL64pcrel32 @init2, csr_64, implicit $rsp, implicit $ssp, implicit $al, implicit-def $rsp, implicit-def $ssp, implicit-def dead $eax, debug-location !15
+    DBG_VALUE $noreg, $noreg, !14, !DIExpression(), debug-location !15
+    renamable $eax = MOV32rm $rbp, 1, $noreg, -4, $noreg, debug-location !15 :: (dereferenceable load 4 from %ir.local1, !tbaa !16)
+    $rsp = frame-destroy ADD64ri8 $rsp, 16, implicit-def dead $eflags, debug-location !15
+    $rbp = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !15
+    CFI_INSTRUCTION def_cfa $rsp, 8, debug-location !15
+    RETQ $eax, debug-location !15
+
+...

llvm/trunk/test/DebugInfo/X86/fission-ranges.ll

 for.inc10:                                        ; preds = %for.body9
   %inc11 = add nsw i32 %b.03, 1, !dbg !38
   tail call void @llvm.dbg.value(metadata i32 %inc11, metadata !15, metadata !DIExpression()), !dbg !42
   %exitcond11 = icmp eq i32 %inc11, 30, !dbg !38
   br i1 %exitcond11, label %for.inc13, label %for.cond7.preheader, !dbg !38
 
 for.inc13:                                        ; preds = %for.inc10
   %inc14 = add i32 %d.06, 1, !dbg !37
-  tail call void @llvm.dbg.value(metadata i32 %inc14, metadata !16, metadata !DIExpression()), !dbg !37
+  tail call void @llvm.dbg.value(metadata i32 %inc14, metadata !16, metadata !DIExpression()), !dbg !42
   %exitcond12 = icmp eq i32 %inc14, 30, !dbg !37
   br i1 %exitcond12, label %for.inc16, label %for.cond4.preheader, !dbg !37
 
 for.inc16:                                        ; preds = %for.inc13
   %inc17 = add nsw i32 %a.08, 1, !dbg !31
   tail call void @llvm.dbg.value(metadata i32 %inc17, metadata !14, metadata !DIExpression()), !dbg !31
   %exitcond13 = icmp eq i32 %inc17, 30, !dbg !31
   br i1 %exitcond13, label %for.end18, label %for.cond1.preheader, !dbg !31