[DebugInfo] Don't always extend variable locations when the reg location is unchanging

[jmorse]

Summary

This patch removes the "ChangingRegs" facility in DbgEntityHistoryCalculator, as its overapproximate nature can produce incorrect variable locations (see [0]). The original intention (D3933) seems to have been to increase the number of scenarios where single-location variable records are used in the DWARF output.

The patch first kills off everything that calculates the ChangingRegs vector. Previously ChangingRegs spotted epilogues and marked the frame register as unchanging if it wasn't modified outside the epilogue. With this patch, we perform the same test by only clobbering the frame register if it is modified by instructions not marked FrameDestroy (line 256). The logic for clobbering regmasks becomes slightly more complex because we can now only iterate over RegsVars, and so need to preserve it's iteration order while scanning or regmask-clobbered registers.

The logic for terminating variable locations at the end of a basic block now becomes much more enjoyably simple: we just terminate them all.

Test changes: I need to state an assumption about correctness first. My understanding is that we deliberately don't terminate stack-based variable locations during frame destruction, because the debugger should understand that those become invalid as the frame is destroyed. (CC @dblaikie who's expressed an interest in expression simplicity in the past). This would certainly match what "gcc-7.4 test.c -g -O2 -fno-omit-frame-pointer -m32" does for the following, where a single location of DW_OP_fbreg +0 is used for the 'sandal' variable throughout the function:

int
main()
{
  int sandal;
  f(&sandal);
  return sandal;
}

(A location that is not valid in the prologue and epilogue).

With that in mind, here are what the test changes look like:

• struct_by_value.ll doesn't get a single-location variable record any more because that single location gets clobbered (my understanding of aarch64 is w0 aliases x0 in the code below). IMO this makes sense: the location isn't on the stack, so the debugger must be told where the variable location ends.
  
  ldr w0, [x0]
  
  ret
• inlined-argument.ll: Everything in this code except the final "ret" is from an inlined function, and is in multiple blocks. My understanding is that the single-location detection algorithm [1] doesn't actually support detecting this scenario: we just get it right because ChangingRegs causes the locations to spill over into the return block, covering the whole function. This should be fixed in a future patch rather than now, IMO.
• pieces.ll: we now don't terminate a spilt stack location when the frame gets destroyed (by addl %rsp, $40), and so the location changes range. This doesn't generate a new single-location entry, alas.
• dbg-addr.ll: This location should have been a single-location entry in the first place, it's not clear why ChangingRegs didn't catch it.
• reference-argument.ll: This test contains a call inst, immediately followed by frame destruction. The new logic recognises that the (register) variable location should terminate after the call, which clobbers the register.
• stack-value-piece.ll: A register location fragment gets clobbered when the return value is written to $xmm0, and so a new location without that fragment is created in this test.

To summarise, we now generate a few more single-location records for stack variables, and several circumstances where registers are clobbered in the final block of the function now get terminated at the correct location.

[0] https://reviews.llvm.org/D61600#1493393
[1] https://github.com/llvm/llvm-project/blob/c93f56d39e629b7bcd0f4657705264fcd7e47c0d/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp#L1242

[bjope]

Adding myself as reviewer to remember to look at this patch.

Downstream we do things differently for our OOT target. I tried to upstream that once-upon-a-time: https://reviews.llvm.org/D50638
But for some reason it never landed (I think I had some doubts about something).

We have also patched calculateDbgEntityHistory to step inside bundles. Not sure if we need that nowadays, but once upon a time we did not rely on that the def/use info on the BUNDLE instruction telling the full truth here (or maybe we allowed DEBUG_VALUE inside bundles... it is nothin upstream that prevents that afaik). So our downstream version here is slightly different compared to upstream for various historical reasons.

If we can get rid of the "ChangingRegs" stuff, then I guess we would get rid of some downstream hacks as well :-)

[dstenb]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

360–393 ↗

(On Diff #199577)

(I saw the note about this not being for review right now; just pointing this out if this is productified later on.)

Now that we close all register-described values I think that this code can be simplified a bit (e.g. not calling clobberRegisterUses), by instead closing all values by iterating over LiveEntries.

Something like this (I have not even tested compiling it, so it is probably a bit broken):

// Clobber all live debug values (both those that are register-described and
// those that are described by other values).
for (auto &Pair : LiveEntries) {
  if (Pair.second.empty())
    continue;

  // Create a clobbering entry.
  EntryIndex ClobIdx = DbgValues.startClobber(Pair.first, MBB.back());

  // End all entries.
  for (EntryIndex Idx : Pair.second) {
    DbgValueHistoryMap::Entry &Ent = DbgValues.getEntry(Pair.first, Idx);
    assert(Ent.isDbgValue() && !Ent.isClosed());
    Ent.endEntry(ClobIdx);
  }
}

LiveEntries.erase();
RegVars.erase();

Or if there is something that I have overlooked that makes it required (or preferable) to use clobberRegisterUses for closing register-described values, we should be able to change it to something like this:

// Clobber registers at the end of BB.
for (auto I = RegVars.begin(), E = RegVars.end(); I != E;) {
  auto CurElem = I++; // CurElem can be erased below.
  clobberRegisterUses(RegVars, CurElem, DbgValues, LiveEntries,
                      MBB.back());
}

// Clobber the rest of the live debug values.
for (auto &Pair : LiveEntries) {
  if (Pair.second.empty())
    continue;

  // Create a clobbering entry.
  EntryIndex ClobIdx = DbgValues.startClobber(Pair.first, MBB.back());

  // End all entries.
  for (EntryIndex Idx : Pair.second) {
    DbgValueHistoryMap::Entry &Ent = DbgValues.getEntry(Pair.first, Idx);
    assert(Ent.isDbgValue() && !Ent.isClosed());
    Ent.endEntry(ClobIdx);
  }
}

LiveEntries.erase();

[aprantl]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

226 ↗

(On Diff #199577)

I appreciate the amount of red in this patch :-)

[jmorse]

Update diff to more production-ised version, will edit summary and add reviewers shortly

[aprantl]

My understanding is that we deliberately don't terminate stack-based variable locations during frame destruction, because the debugger should understand that those become invalid as the frame is destroyed.

CC'ing @jasonmolenda and @jingham to double-check that statement.

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

265 ↗

(On Diff #199786)

My intuition says that your typical function call will clobber more than 8 registers, but I don't know what a good number is either.

282 ↗

(On Diff #199786)

For readability it might be a good idea to move the short cases to the top, with a continue, then we have more horizontal space for the meat of the function.

test/DebugInfo/AArch64/struct_by_value.ll

3 ↗

(On Diff #199786)

Thanks! Feel free to land this ahead of time as an NFC commit.

[bjope]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

257 ↗

(On Diff #199786)

The old solution using getFirstEpilogueInst was a little bit weird so it is nice to get rid of that. However, if I remember correctly FrameDestroy isn't used by all in-tree targets (and this change is not fully backwards compatible for OOT targets either).

Maybe that shouldn't stop us. But just something that should be taken into consideration (maybe we should make sure in-tree targets are using FrameDestroy consistently and make it obvious in commit msg or something that we now rely on FrameDestroy here).
It could even be possible to swap into relying on FrameDestory in a separate patch.

[jmorse]

Apply review comments (change a vector size, shuffle the order of some blocks).

[jmorse]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

257 ↗

(On Diff #199786)

Ah, I read about this in the history, but didn't think about it too deeply. The interaction between OOT backends and the rest of LLVM is largely opaque to me, however IIUC, the impact of this behaviour change is that stack-locations will end prematurely in the epilogue, and they won't get as nice DWARF-expressions for such locations. Would this be sufficiently minor for it to go ahead?

I'll examine the in-tree backends to check that they're using FrameDestroy correctly.

Splitting this patch would likely require marking some tests XFail in the meantime, as many single-location values wouldn't be generated. Perhaps that's desirable, as OOT backends could then revert the FrameDestroy patch and have the correct tests marked XFail?

282 ↗

(On Diff #199786)

I've shuffled the shorter cases to the top and added a continue; alas, this makes for a very different patch to the previous one.

For the RegsToClobber vector I've upped the size to 32 elements, on the principle that stack allocation is more or less free, and we're (broadly) a leaf function.

[jmorse]

With this update I've separated the FrameDestroy portion from the rest of the patch. The result is that debug-loc-offset.mir and pr19307.mir are marked XFAIL, as the locations they check no longer extend through the frame-destroy'd part of the produced code. I'll upload the remaining part of the patch in a moment, that adds in use of the frame-destroy flag and un-xfails those tests.

Looking through in-tree backends, only X86, AArch64 and RISC-V actually set FrameDestroy. Nevertheless I think FrameDestroy is still something we should make use of: if the assumption I stated above still holds (we deliberately let stack locations run through frame destruction), then we can only detect frame destruction if the backend tells us where it is. The failure-case is that variable locations end slightly earlier than necessary, and we might get fewer single-value variable locations. An acceptable trade-off IMHO.

I ran check-llvm with LLVM_DEFAULT_TARGET_TRIPLE="arm-none-eabi" to check that no generic tests on a non-frame-destroy backend get hit by this change.

Note that this will still leave inlined-argument.ll XFailed, as I don't believe our current implementation can soundly produce the correct output anyway.

[bjope]

Is the intention to change the behavior for prologues as well here? The description mentions that we do a different analysis for the epilogues. But with this patch we no longer ignore register clobbering for "FrameSetup" (collectChangingRegs used to ignore FrameSetup code).

Maybe that is a good thing (just not obvious when reading the description that this patch also treats prologues differently).

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

257 ↗

(On Diff #199786)

Ah, I read about this in the history, but didn't think about it too deeply. The interaction between OOT backends and the rest of LLVM is largely opaque to me, however IIUC, the impact of this behaviour change is that stack-locations will end prematurely in the epilogue, and they won't get as nice DWARF-expressions for such locations. Would this be sufficiently minor for it to go ahead?

Ok, makes sense.

277 ↗

(On Diff #200939)

Comment here is related to D62314 (that is a child). Maybe D62314 should be a parent instead (unless that complicates things even more for you)?

[jmorse]

Bjorn wrote:

Is the intention to change the behavior for prologues as well here? The description mentions that we do a different analysis for the epilogues. But with this patch we no longer ignore register clobbering for "FrameSetup" (collectChangingRegs used to ignore FrameSetup code).

Maybe that is a good thing (just not obvious when reading the description that this patch also treats prologues differently).

Oooo, I'd missed that part, good spot. However when adding insensitivity-to-prologues into this patch, it causes a change in behaviour -- fpo-shrink-wrap.ll now extends a stack location through the whole of a prologue, where it previously terminated after the first 'push'. (The change manifests as one fewer temporary labels).

IMO this is an improvement in the output, and what ChangingRegs was _supposed_ to detect. It's not completely clear why it didn't before. AFAIUI, FrameSetup is more widely supported, and so it doesn't need to be rolled into the subsequent patch?

[jmorse]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

277 ↗

(On Diff #200939)

I'll roll the FrameDestroy comment into the child patch, it should be simpler (IMO).

[jmorse]

Ping on this -- AFAIUI people believe the change is good, and depending on FrameDestroy is deployed in a separate patch.

The assumption that stack variable locations run to the end of a function, even over the frame destruction, is seen in GCC here [0]. The location for "sandal" gets "DW_AT_location (DW_OP_breg5 EBP-12)" across the whole function, including the 'leave' inst and fiddling-with-esp. I think it's fair to say we're implementing existing expectations.

(This reverse-blocks D61600).

[0] https://godbolt.org/z/lLJg4c

[aprantl]

The assumption that stack variable locations run to the end of a function, even over the frame destruction, is seen in GCC here [0].

I'm fine with implementing this behavior as long as we document it.

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

static void clobberRegisterUses(RegDescribedVarsMap &RegVars, unsigned RegNo,
                                 DbgValueEntriesMap &LiveEntries,
                                 const MachineInstr &ClobberingInstr) {
   const auto &I = RegVars.find(RegNo);
   if (I == RegVars.end())
     return;
   clobberRegisterUses(RegVars, I, HistMap, LiveEntries, ClobberingInstr);
 }
 
-// Returns the first instruction in @MBB which corresponds to
-// the function epilogue, or nullptr if @MBB doesn't contain an epilogue.
-static const MachineInstr *getFirstEpilogueInst(const MachineBasicBlock &MBB) {
-  auto LastMI = MBB.getLastNonDebugInstr();
-  if (LastMI == MBB.end() || !LastMI->isReturn())
-    return nullptr;
-  // Assume that epilogue starts with instruction having the same debug location
-  // as the return instruction.
-  DebugLoc LastLoc = LastMI->getDebugLoc();
-  auto Res = LastMI;
-  for (MachineBasicBlock::const_reverse_iterator I = LastMI.getReverse(),
-                                                 E = MBB.rend();
-       I != E; ++I) {
-    if (I->getDebugLoc() != LastLoc)
-      return &*Res;
-    Res = &*I;
-  }
-  // If all instructions have the same debug location, assume whole MBB is
-  // an epilogue.
-  return &*MBB.begin();
-}
-
-// Collect registers that are modified in the function body (their
-// contents is changed outside of the prologue and epilogue).
-static void collectChangingRegs(const MachineFunction *MF,
-                                const TargetRegisterInfo *TRI,
-                                BitVector &Regs) {
-  for (const auto &MBB : *MF) {
-    auto FirstEpilogueInst = getFirstEpilogueInst(MBB);
-
-    for (const auto &MI : MBB) {
-      // Avoid looking at prologue or epilogue instructions.
-      if (&MI == FirstEpilogueInst)
-        break;
-      if (MI.getFlag(MachineInstr::FrameSetup))
-        continue;
-
-      // Look for register defs and register masks. Register masks are
-      // typically on calls and they clobber everything not in the mask.
-      for (const MachineOperand &MO : MI.operands()) {
-        // Skip virtual registers since they are handled by the parent.
-        if (MO.isReg() && MO.isDef() && MO.getReg() &&
-            !TRI->isVirtualRegister(MO.getReg())) {
-          for (MCRegAliasIterator AI(MO.getReg(), TRI, true); AI.isValid();
-               ++AI)
-            Regs.set(*AI);
-        } else if (MO.isRegMask()) {
-          Regs.setBitsNotInMask(MO.getRegMask());
-        }
-      }
-    }
-  }
-}
-
 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()) {
-          if (MO.isReg() && MO.isDef() && MO.getReg()) {
-            // Ignore call instructions that claim to clobber SP. The AArch64
-            // backend does this for aggregate function arguments.
-            if (MI.isCall() && MO.getReg() == SP)
-              continue;
-            // If this is a virtual register, only clobber it since it doesn't
-            // have aliases.
-            if (TRI->isVirtualRegister(MO.getReg()))
-              clobberRegisterUses(RegVars, MO.getReg(), DbgValues, LiveEntries,
-                                  MI);
-            // If this is a register def operand, it may end a debug value
-            // range.
-            else {
-              for (MCRegAliasIterator AI(MO.getReg(), TRI, true); AI.isValid();
-                   ++AI)
-                if (ChangingRegs.test(*AI))
-                  clobberRegisterUses(RegVars, *AI, DbgValues, LiveEntries, MI);
-            }
-          } else if (MO.isRegMask()) {
-            // If this is a register mask operand, clobber all debug values in
-            // non-CSRs.
-            for (unsigned I : ChangingRegs.set_bits()) {
-              // Don't consider SP to be clobbered by register masks.
-              if (unsigned(I) != SP && TRI->isPhysicalRegister(I) &&
-                  MO.clobbersPhysReg(I)) {
-                clobberRegisterUses(RegVars, I, DbgValues, LiveEntries, MI);
-              }
-            }
+      if (MI.isDebugValue()) {
+        assert(MI.getNumOperands() > 1 && "Invalid DBG_VALUE instruction!");
+        // Use the base variable (without any DW_OP_piece expressions)
+        // as index into History. The full variables including the
+        // piece expressions are attached to the MI.
+        const DILocalVariable *RawVar = MI.getDebugVariable();
+        assert(RawVar->isValidLocationForIntrinsic(MI.getDebugLoc()) &&
+               "Expected inlined-at fields to agree");
+        InlinedEntity Var(RawVar, MI.getDebugLoc()->getInlinedAt());
+
+        handleNewDebugValue(Var, MI, RegVars, LiveEntries, DbgValues);
+      } else if (MI.isDebugLabel()) {
+        assert(MI.getNumOperands() == 1 && "Invalid DBG_LABEL instruction!");
+        const DILabel *RawLabel = MI.getDebugLabel();
+        assert(RawLabel->isValidLocationForIntrinsic(MI.getDebugLoc()) &&
+            "Expected inlined-at fields to agree");
+        // When collecting debug information for labels, there is no MCSymbol
+        // generated for it. So, we keep MachineInstr in DbgLabels in order
+        // to query MCSymbol afterward.
+        InlinedEntity L(RawLabel, MI.getDebugLoc()->getInlinedAt());
+        DbgLabels.addInstr(L, MI);
+      }
+
+      if (MI.isDebugInstr())
+        continue;
+
+          // range. Ignore defs of the frame register in the prologue.
+          else if (MO.getReg() != FrameReg ||
+                   !MI.getFlag(MachineInstr::FrameSetup)) {
+          SmallVector<unsigned, 32> RegsToClobber;
+          for (auto It : RegVars) {
+            unsigned int Reg = It.first;
+            if (Reg != SP && TRI->isPhysicalRegister(Reg) &&
+                MO.clobbersPhysReg(Reg))
+              RegsToClobber.push_back(Reg);
+
+          for (unsigned Reg : RegsToClobber) {
+            clobberRegisterUses(RegVars, Reg, DbgValues, LiveEntries, MI);
           }
         }
+      } // End MO loop.
+    }   // End instr loop.
+
+    // Make sure locations for all 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()) {
+      // Iterate over all variables that have open debug values.
+      for (auto &Pair : LiveEntries) {
+        if (Pair.second.empty())
           continue;
-      }
 
-      if (MI.isDebugValue()) {
-        assert(MI.getNumOperands() > 1 && "Invalid DBG_VALUE instruction!");
-        // Use the base variable (without any DW_OP_piece expressions)
-        // as index into History. The full variables including the
-        // piece expressions are attached to the MI.
-        const DILocalVariable *RawVar = MI.getDebugVariable();
-        assert(RawVar->isValidLocationForIntrinsic(MI.getDebugLoc()) &&
-               "Expected inlined-at fields to agree");
-        InlinedEntity Var(RawVar, MI.getDebugLoc()->getInlinedAt());
+        // Create a clobbering entry.
+        EntryIndex ClobIdx = DbgValues.startClobber(Pair.first, MBB.back());
 
-        handleNewDebugValue(Var, MI, RegVars, LiveEntries, DbgValues);
-      } else if (MI.isDebugLabel()) {
-        assert(MI.getNumOperands() == 1 && "Invalid DBG_LABEL instruction!");
-        const DILabel *RawLabel = MI.getDebugLabel();
-        assert(RawLabel->isValidLocationForIntrinsic(MI.getDebugLoc()) &&
-            "Expected inlined-at fields to agree");
-        // When collecting debug information for labels, there is no MCSymbol
-        // generated for it. So, we keep MachineInstr in DbgLabels in order
-        // to query MCSymbol afterward.
-        InlinedEntity L(RawLabel, MI.getDebugLoc()->getInlinedAt());
-        DbgLabels.addInstr(L, MI);
+        // End all entries.
+        for (EntryIndex Idx : Pair.second) {
+          DbgValueHistoryMap::Entry &Ent = DbgValues.getEntry(Pair.first, Idx);
+          assert(Ent.isDbgValue() && !Ent.isClosed());
+          Ent.endEntry(ClobIdx);
         }
       }
 
-    // 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) ||
-            CurElem->first == FrameReg)
-          clobberRegisterUses(RegVars, CurElem, DbgValues, LiveEntries,
-                              MBB.back());
-      }
+      LiveEntries.clear();
+      RegVars.clear();
     }
   }
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void DbgValueHistoryMap::dump() const {
   dbgs() << "DbgValueHistoryMap:\n";
   for (const auto &VarRangePair : *this) {

llvm/trunk/lib/CodeGen/LiveDebugValues.cpp

struct VarLoc {
     };
 
     const DebugVariable Var;
     const MachineInstr &MI; ///< Only used for cloning a new DBG_VALUE.
     mutable UserValueScopes UVS;
     enum VarLocKind {
       InvalidKind = 0,
       RegisterKind,
-      SpillLocKind
+      SpillLocKind,
+      ImmediateKind
     } Kind = InvalidKind;
 
     /// The value location. Stored separately to avoid repeatedly
     /// extracting it from MI.
     union {
       uint64_t RegNo;
       SpillLoc SpillLocation;
       uint64_t Hash;
+      int64_t Immediate;
+      const ConstantFP *FPImm;
+      const ConstantInt *CImm;
     } Loc;
 
     VarLoc(const MachineInstr &MI, LexicalScopes &LS)
         : Var(MI.getDebugVariable(), MI.getDebugLoc()->getInlinedAt()), MI(MI),
           UVS(MI.getDebugLoc(), LS) {
       static_assert((sizeof(Loc) == sizeof(uint64_t)),
                     "hash does not cover all members of Loc");
       assert(MI.isDebugValue() && "not a DBG_VALUE");
       assert(MI.getNumOperands() == 4 && "malformed DBG_VALUE");
       if (int RegNo = isDbgValueDescribedByReg(MI)) {
         Kind = RegisterKind;
         Loc.RegNo = RegNo;
+      } else if (MI.getOperand(0).isImm()) {
+        Kind = ImmediateKind;
+        Loc.Immediate = MI.getOperand(0).getImm();
+      } else if (MI.getOperand(0).isFPImm()) {
+        Kind = ImmediateKind;
+        Loc.FPImm = MI.getOperand(0).getFPImm();
+      } else if (MI.getOperand(0).isCImm()) {
+        Kind = ImmediateKind;
+        Loc.CImm = MI.getOperand(0).getCImm();
       }
     }
 
     /// The constructor for spill locations.
     VarLoc(const MachineInstr &MI, unsigned SpillBase, int SpillOffset,
            LexicalScopes &LS)
         : Var(MI.getDebugVariable(), MI.getDebugLoc()->getInlinedAt()), MI(MI),
           UVS(MI.getDebugLoc(), LS) {
       assert(MI.isDebugValue() && "not a DBG_VALUE");
       assert(MI.getNumOperands() == 4 && "malformed DBG_VALUE");
       Kind = SpillLocKind;
       Loc.SpillLocation = {SpillBase, SpillOffset};
     }
 
+    // Is the Loc field a constant or constant object?
+    bool isConstant() const { return Kind == ImmediateKind; }
+
     /// If this variable is described by a register, return it,
     /// otherwise return 0.
     unsigned isDescribedByReg() const {
       if (Kind == RegisterKind)
         return Loc.RegNo;
       return 0;
     }
 
     /// Determine whether the lexical scope of this value's debug location
     /// dominates MBB.
     bool dominates(MachineBasicBlock &MBB) const { return UVS.dominates(&MBB); }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
     LLVM_DUMP_METHOD void dump() const { MI.dump(); }
 #endif
 
     bool operator==(const VarLoc &Other) const {
-      return Var == Other.Var && Loc.Hash == Other.Loc.Hash;
+      return Kind == Other.Kind && Var == Other.Var &&
+             Loc.Hash == Other.Loc.Hash;
     }
 
     /// This operator guarantees that VarLocs are sorted by Variable first.
     bool operator<(const VarLoc &Other) const {
       if (Var == Other.Var)
         return Loc.Hash < Other.Loc.Hash;
       return Var < Other.Var;
     }
void LiveDebugValues::transferDebugValue(const MachineInstr &MI,
   assert(Var->isValidLocationForIntrinsic(DebugLoc) &&
          "Expected inlined-at fields to agree");
 
   // End all previous ranges of Var.
   DebugVariable V(Var, InlinedAt);
   OpenRanges.erase(V);
 
   // Add the VarLoc to OpenRanges from this DBG_VALUE.
-  // TODO: Currently handles DBG_VALUE which has only reg as location.
-  if (isDbgValueDescribedByReg(MI)) {
+  unsigned ID;
+  if (isDbgValueDescribedByReg(MI) || MI.getOperand(0).isImm() ||
+      MI.getOperand(0).isFPImm() || MI.getOperand(0).isCImm()) {
+    // Use normal VarLoc constructor for registers and immediates.
     VarLoc VL(MI, LS);
-    unsigned ID = VarLocIDs.insert(VL);
+    ID = VarLocIDs.insert(VL);
+    OpenRanges.insert(ID, VL.Var);
+  } else if (MI.hasOneMemOperand()) {
+    // It's a stack spill -- fetch spill base and offset.
+    VarLoc::SpillLoc SpillLocation = extractSpillBaseRegAndOffset(MI);
+    VarLoc VL(MI, SpillLocation.SpillBase, SpillLocation.SpillOffset, LS);
+    ID = VarLocIDs.insert(VL);
     OpenRanges.insert(ID, VL.Var);
+  } else {
+    // This must be an undefined location. We should leave OpenRanges closed.
+    assert(MI.getOperand(0).isReg() && MI.getOperand(0).getReg() == 0 &&
+           "Unexpected non-undef DBG_VALUE encountered");
   }
 }
 
 /// Create new TransferDebugPair and insert it in \p Transfers. The VarLoc
 /// with \p OldVarID should be deleted form \p OpenRanges and replaced with
 /// new VarLoc. If \p NewReg is different than default zero value then the
 /// new location will be register location created by the copy like instruction,
 /// otherwise it is variable's location on the stack.
bool LiveDebugValues::join(
   VarLocSet Diff = InLocsT;
   Diff.intersectWithComplement(ILS);
   for (auto ID : Diff) {
     // This VarLoc is not found in InLocs i.e. it is not yet inserted. So, a
     // new range is started for the var from the mbb's beginning by inserting
     // a new DBG_VALUE. process() will end this range however appropriate.
     const VarLoc &DiffIt = VarLocIDs[ID];
     const MachineInstr *DebugInstr = &DiffIt.MI;
-    MachineInstr *MI = BuildMI(
-        MBB, MBB.instr_begin(), DebugInstr->getDebugLoc(),
+    MachineInstr *MI = nullptr;
+    if (DiffIt.isConstant()) {
+      MachineOperand MO(DebugInstr->getOperand(0));
+      MI = BuildMI(MBB, MBB.instr_begin(), DebugInstr->getDebugLoc(),
+                   DebugInstr->getDesc(), false, MO,
+                   DebugInstr->getDebugVariable(),
+                   DebugInstr->getDebugExpression());
+    } else {
+      MI = BuildMI(MBB, MBB.instr_begin(), DebugInstr->getDebugLoc(),
                    DebugInstr->getDesc(), DebugInstr->isIndirectDebugValue(),
-        DebugInstr->getOperand(0).getReg(), DebugInstr->getDebugVariable(),
+                   DebugInstr->getOperand(0).getReg(),
+                   DebugInstr->getDebugVariable(),
                    DebugInstr->getDebugExpression());
       if (DebugInstr->isIndirectDebugValue())
         MI->getOperand(1).setImm(DebugInstr->getOperand(1).getImm());
+    }
     LLVM_DEBUG(dbgs() << "Inserted: "; MI->dump(););
     ILS.set(ID);
     ++NumInserted;
     Changed = true;
   }
   return Changed;
 }
 

llvm/trunk/test/DebugInfo/AArch64/inlined-argument.ll

 ; RUN: llc -filetype=obj -o - %s | llvm-dwarfdump --name resource - | FileCheck %s
 ; CHECK: DW_TAG_formal_parameter
 ; CHECK: DW_TAG_formal_parameter
 ; CHECK-NEXT:  DW_AT_location (DW_OP_reg1 W1)
 ; CHECK-NEXT:  DW_AT_abstract_origin {{.*}}"resource"
 ;
+; XFAIL: *
+; This test now fails as it requires the single-location variable recognizer
+; to spot that the inlined function goes out of scope before the 'find.exit'
+; exit block. Previously, unchanging variable locations could be extended to
+; the end of the function, often erronously, and that's why this test used to
+; pass.
+; A future algorithm _should_ be able to recognize that "resource"/!37 covers
+; all blocks in its lexical scope.
+;
 ; Generated from:
 ; typedef struct t *t_t;
 ; extern unsigned int enable;
 ; struct t {
 ;   struct q {
 ;     struct q *next;
 ;     unsigned long long resource;
 ;   } * s;

llvm/trunk/test/DebugInfo/AArch64/struct_by_value.ll

 ; A by-value struct is a register-indirect value (breg).
-; RUN: llc %s -filetype=asm -o - | FileCheck %s
+; RUN: llc %s -filetype=obj -o - | llvm-dwarfdump - | FileCheck %s
+; REQUIRES: object-emission
 
-; CHECK: Lsection_info:
-; CHECK: DW_AT_location
-; CHECK-NEXT: .byte 112
-; 112 = 0x70 = DW_OP_breg0
+; Test that the 'f' parameter is present, with a location, and that the
+; expression for the location contains a DW_OP_breg
+; CHECK: DW_TAG_formal_parameter
+; CHECK-NEXT: DW_AT_location
+; CHECK-NEXT: DW_OP_breg
 
 ; rdar://problem/13658587
 ;
 ; Generated from
 ;
 ; struct five
 ; {
 ;   int a;

llvm/trunk/test/DebugInfo/COFF/fpo-shrink-wrap.ll

 
 ; ASM:         calll   _doSomething
 
 ; ASM:         popl    %esi
 ; ASM:         popl    %edi
 ; ASM:         popl    %ebx
 ; ASM: [[EPILOGUE]]:                                 # %return
 ; ASM:         retl    $8
-; ASM: Ltmp11:
+; ASM: Ltmp10:
 ; ASM:         .cv_fpo_endproc
 
 ; Note how RvaStart advances 7 bytes to skip the shrink-wrapped portion.
 ; OBJ: SubSectionType: FrameData (0xF5)
 ; OBJ:    FrameData {
 ; OBJ:      RvaStart: 0x0
 ; OBJ:      CodeSize: 0x34
 ; OBJ:      PrologSize: 0x9

llvm/trunk/test/DebugInfo/COFF/pieces.ll

 ; }
 
 ; ASM-LABEL: loop_csr: # @loop_csr
 ; ASM:        #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 0 32] 0
 ; ASM:        #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 32 32] 0
 ; ASM: # %bb.2:                                 # %for.body.preheader
 ; ASM:         xorl    %edi, %edi
 ; ASM:         xorl    %esi, %esi
+; ASM: [[oy_ox_start:\.Ltmp[0-9]+]]:
 ; ASM:         .p2align        4, 0x90
 ; ASM: .LBB0_3:                                # %for.body
-; ASM: [[oy_ox_start:\.Ltmp[0-9]+]]:
+; ASM:        #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 0 32] 0
 ; ASM:        #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 0 32] $edi
 ; ASM:        #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 32 32] $esi
 ; ASM:        .cv_loc 0 1 13 11               # t.c:13:11
 ; ASM:        movl    %edi, %ecx
 ; ASM:        callq   g
 ; ASM:        movl    %eax, %edi
 ; ASM: [[ox_start:\.Ltmp[0-9]+]]:
 ; ASM:         #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 0 32] $edi
 ; ASM:         .cv_loc 0 1 14 11               # t.c:14:11
 ; ASM:         movl    %esi, %ecx
 ; ASM:         callq   g
 ; ASM:         movl    %eax, %esi
 ; ASM: [[oy_start:\.Ltmp[0-9]+]]:
 ; ASM:         #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 32 32] $esi
 ; ASM:         cmpl    n(%rip), %eax
 ; ASM:         jl      .LBB0_3
+; ASM: [[loopskip_start:\.Ltmp[0-9]+]]:
+; ASM:         #DEBUG_VALUE: loop_csr:o <- [DW_OP_LLVM_fragment 0 32] 0
+; ASM:         xorl    %esi, %esi
+; ASM:         xorl    %edi, %edi
 ; ASM: [[oy_end:\.Ltmp[0-9]+]]:
 ; ASM:         addl    %edi, %esi
 ; ASM:         movl    %esi, %eax
 
+; XXX FIXME: the debug value line after loopskip_start should be repeated
+; because both fields of 'o' are zero flowing into this block. However, it
+; appears livedebugvalues doesn't account for fragments.
 
 ; ASM-LABEL: pad_right: # @pad_right
 ; ASM:         movq    %rcx, %rax
+; ASM: [[pad_right_tmp:\.Ltmp[0-9]+]]:
 ; ASM:         #DEBUG_VALUE: pad_right:o <- [DW_OP_LLVM_fragment 32 32] $eax
 ; ASM:         retq
 
 
 ; ASM-LABEL: pad_left: # @pad_left
 ; ASM:         .cv_loc 2 1 24 3                # t.c:24:3
 ; ASM:         movq    %rcx, %rax
+; ASM: [[pad_left_tmp:\.Ltmp[0-9]+]]:
 ; ASM:         #DEBUG_VALUE: pad_left:o <- [DW_OP_LLVM_fragment 0 32] $eax
 ; ASM:         retq
 
 
 ; ASM-LABEL: nested: # @nested
 ; ASM:         #DEBUG_VALUE: nested:o <- [DW_OP_deref] [$rcx+0]
 ; ASM:         movl    12(%rcx), %eax
 ; ASM: [[p_start:\.Ltmp[0-9]+]]:

 
 
 ; ASM-LABEL:  .short  4423                    # Record kind: S_GPROC32_ID
 ; ASM:        .asciz  "loop_csr"              # Function name
 ; ASM:        .short  4414                    # Record kind: S_LOCAL
 ; ASM:        .asciz  "o"
 ; ASM:        .cv_def_range    [[oy_ox_start]] [[ox_start]], "C\021\030\000\000\000\000\000\000\000"
 ; ASM:        .cv_def_range    [[oy_ox_start]] [[oy_start]], "C\021\027\000\000\000\004\000\000\000"
-; ASM:        .cv_def_range    [[ox_start]] [[oy_end]], "C\021\030\000\000\000\000\000\000\000"
-; ASM:        .cv_def_range    [[oy_start]] [[oy_end]], "C\021\027\000\000\000\004\000\000\000"
+; ASM:        .cv_def_range    [[ox_start]] [[loopskip_start]], "C\021\030\000\000\000\000\000\000\000"
+; ASM:        .cv_def_range    [[oy_start]] [[loopskip_start]], "C\021\027\000\000\000\004\000\000\000"
 
 
 ; OBJ-LABEL: GlobalProcIdSym {
 ; OBJ:         Kind: S_GPROC32_ID (0x1147)
 ; OBJ:         DisplayName: loop_csr
 ; OBJ:       }
 ; OBJ:       LocalSym {
 ; OBJ:         VarName: o

 ; OBJ:       }
 ; OBJ:       ProcEnd {
 ; OBJ:       }
 
 ; ASM-LABEL:  .short  4423                    # Record kind: S_GPROC32_ID
 ; ASM:        .asciz  "pad_right"             # Function name
 ; ASM:        .short  4414                    # Record kind: S_LOCAL
 ; ASM:        .asciz  "o"
-; ASM:        .cv_def_range    .Ltmp8 .Ltmp8, "C\021\021\000\000\000\004\000\000\000"
+; ASM:        .cv_def_range    [[pad_right_tmp]] [[pad_right_tmp]], "C\021\021\000\000\000\004\000\000\000"
 
 ; OBJ-LABEL: GlobalProcIdSym {
 ; OBJ:         Kind: S_GPROC32_ID (0x1147)
 ; OBJ:         DisplayName: pad_right
 ; OBJ:       }
 ; OBJ:       LocalSym {
 ; OBJ:         VarName: o
 ; OBJ:       }
 ; OBJ:       DefRangeSubfieldRegisterSym {
 ; OBJ:         Register: EAX (0x11)
 ; OBJ:         MayHaveNoName: 0
 ; OBJ:         OffsetInParent: 4
 ; OBJ:         LocalVariableAddrRange {
 ; OBJ:         }
 ; OBJ:       }
 ; OBJ:       ProcEnd {
 ; OBJ:       }
 
 ; ASM-LABEL:  .short  4423                    # Record kind: S_GPROC32_ID
 ; ASM:        .asciz  "pad_left"              # Function name
 ; ASM:        .short  4414                    # Record kind: S_LOCAL
 ; ASM:        .asciz  "o"
-; ASM:        .cv_def_range    .Ltmp10 .Ltmp10, "C\021\021\000\000\000\000\000\000\000"
+; ASM:        .cv_def_range    [[pad_left_tmp]] [[pad_left_tmp]], "C\021\021\000\000\000\000\000\000\000"
 
 ; OBJ-LABEL: GlobalProcIdSym {
 ; OBJ:         Kind: S_GPROC32_ID (0x1147)
 ; OBJ:         DisplayName: pad_left
 ; OBJ:       }
 ; OBJ:       LocalSym {
 ; OBJ:         VarName: o
 ; OBJ:       }

llvm/trunk/test/DebugInfo/MIR/AArch64/implicit-def-dead-scope.mir

 # RUN: llc -start-after=livedebugvalues -filetype=obj -o - %s \
 # RUN:   | llvm-dwarfdump -v - | FileCheck %s
 
 # This tests for a crash in DwarfDebug's singular DBG_VALUE range promotion when
 #  encountering an IMPLICIT_DEF in its own lexical scope.
 
 # CHECK: .debug_info contents:
-# CHECK: DW_TAG_formal_parameter
-# CHECK:   DW_AT_const_value [DW_FORM_udata]   (0)
+# CHECK:       DW_TAG_formal_parameter
+# CHECK:       DW_AT_location [DW_FORM_sec_offset]
+# CHECK-NEXT:                 DW_OP_lit0, DW_OP_stack_value
+# CHECK-NEXT:  DW_AT_abstract_origin {{.*}} "name"
 --- |
   ; ModuleID = 't.ll'
   source_filename = "t.ll"
   target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
   target triple = "aarch64--linux-gnu"
   
   %class.v = type <{ i32, i8, i8, [2 x i8] }>
   %class.j = type <{ %"class.j<6, a::f>::D", i32, [4 x i8] }>
--- |
   !10 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "q", scope: !11, file: !5, line: 39, size: 64, elements: !2, identifier: "_ZTSN1jILi6EN1a1fEE1qE")
   !11 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "j<6, a::f>", file: !5, line: 7, size: 128, elements: !2, templateParams: !2, identifier: "_ZTS1jILi6EN1a1fEE")
   !12 = !{i32 2, !"Debug Info Version", i32 3}
   !13 = !{i32 1, !"wchar_size", i32 4}
   !14 = distinct !DISubprogram(name: "bv", linkageName: "_ZN1v2bvEv", scope: !15, file: !5, line: 104, type: !16, isLocal: false, isDefinition: true, scopeLine: 104, flags: DIFlagPrototyped, isOptimized: true, unit: !0, declaration: !17, retainedNodes: !2)
   !15 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "v", file: !5, line: 97, size: 64, elements: !2, identifier: "_ZTS1v")
   !16 = !DISubroutineType(types: !2)
   !17 = !DISubprogram(name: "bv", linkageName: "_ZN1v2bvEv", scope: !15, file: !5, line: 98, type: !16, isLocal: false, isDefinition: false, scopeLine: 98, flags: DIFlagPrototyped, isOptimized: true)
-  !18 = !DILocalVariable(arg: 2, scope: !19, file: !5, line: 22, type: !21)
+  !18 = !DILocalVariable(name: "name", arg: 2, scope: !19, file: !5, line: 22, type: !21)
   !19 = distinct !DISubprogram(name: "m", linkageName: "_ZN1jILi6EN1a1fEE1mEj", scope: !11, file: !5, line: 22, type: !16, isLocal: false, isDefinition: true, scopeLine: 22, flags: DIFlagPrototyped, isOptimized: true, unit: !0, declaration: !20, retainedNodes: !2)
   !20 = !DISubprogram(name: "m", linkageName: "_ZN1jILi6EN1a1fEE1mEj", scope: !11, file: !5, line: 22, type: !16, isLocal: false, isDefinition: false, scopeLine: 22, flags: DIFlagPublic | DIFlagPrototyped, isOptimized: true)
   !21 = !DIDerivedType(tag: DW_TAG_typedef, name: "h", file: !5, line: 10, baseType: !22)
   !22 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
   !23 = !DIExpression()
   !24 = !DILocation(line: 22, scope: !19, inlinedAt: !25)
   !25 = distinct !DILocation(line: 109, scope: !26)
   !26 = distinct !DILexicalBlock(scope: !14, file: !5, line: 106)

llvm/trunk/test/DebugInfo/X86/debug-loc-offset.mir

 # RUN: llc -o - %s -start-after=patchable-function -filetype=obj -O0 -mtriple=i386-unknown-linux-gnu -dwarf-version=4 | llvm-dwarfdump -v - | FileCheck %s
 
+# XFAIL: *
+# Marked XFail due to the removal of "ChangingRegs" from
+# DbgEntityHistoryCalculator, shortening the checked range to not reach the
+# end of the function. Fixed by an about-to-land patch using the FrameDestroy
+# flag to identify the end of functions.
+
 # From the code:
 #
 # debug-loc-offset1.cc
 # int bar (int b) {
 #   return b+4;
 # }
 #
 # debug-loc-offset2.cc

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

 ; CHECK-NEXT: DW_AT_GNU_dwo_name
 ; CHECK-NEXT: DW_AT_GNU_dwo_id
 ; CHECK-NEXT: DW_AT_GNU_ranges_base
 ; CHECK-NEXT: DW_AT_low_pc
 ; CHECK-NEXT: DW_AT_high_pc
 ; CHECK-NEXT: DW_AT_GNU_addr_base [DW_FORM_sec_offset]                   (0x00000000)
 
 ; CHECK: .debug_info.dwo contents:
+; CHECK:      DW_TAG_formal_parameter
+; CHECK-NEXT:   DW_AT_const_value [DW_FORM_sdata] (1)
+; CHECK-NEXT:   DW_AT_name {{.*}} "p")
 ; CHECK: DW_AT_location [DW_FORM_sec_offset]   ([[A:0x[0-9a-z]*]]
 ; CHECK: DW_AT_location [DW_FORM_sec_offset]   ([[E:0x[0-9a-z]*]]
 ; CHECK: DW_AT_location [DW_FORM_sec_offset]   ([[B:0x[0-9a-z]*]]
 ; CHECK: DW_AT_location [DW_FORM_sec_offset]   ([[D:0x[0-9a-z]*]]
 ; CHECK: DW_AT_ranges [DW_FORM_sec_offset]   (0x00000000
 ; CHECK-NOT: .debug_loc contents:
 ; CHECK-NOT: Beginning address offset
 ; CHECK: .debug_loc.dwo contents:
 
 ; 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]]:
 ; CHECK-NEXT:   Addr idx 2 (w/ length 169): DW_OP_consts +0, DW_OP_stack_value
 ; CHECK-NEXT:   Addr idx 3 (w/ length 15): DW_OP_reg0 RAX
 ; CHECK-NEXT:   Addr idx 4 (w/ length 18): DW_OP_breg7 RSP-8
 ; CHECK:      [[E]]:
-; CHECK-NEXT:   Addr idx 5 (w/ length 23): DW_OP_reg0 RAX
+; CHECK-NEXT:   Addr idx 5 (w/ length 9): DW_OP_reg0 RAX
+; CHECK-NEXT:   Addr idx 6 (w/ length 98): DW_OP_breg7 RSP-44
 ; CHECK:      [[B]]:
-; CHECK-NEXT:   Addr idx 6 (w/ length 15): DW_OP_reg0 RAX
-; CHECK-NEXT:   Addr idx 7 (w/ length 66): DW_OP_breg7 RSP-32
+; CHECK-NEXT:   Addr idx 7 (w/ length 15): DW_OP_reg0 RAX
+; CHECK-NEXT:   Addr idx 8 (w/ length 66): DW_OP_breg7 RSP-32
 ; CHECK:      [[D]]:
-; CHECK-NEXT:   Addr idx 8 (w/ length 15): DW_OP_reg0 RAX
-; CHECK-NEXT:   Addr idx 9 (w/ length 42): DW_OP_breg7 RSP-20
+; CHECK-NEXT:   Addr idx 9 (w/ length 15): DW_OP_reg0 RAX
+; CHECK-NEXT:   Addr idx 10 (w/ length 42): DW_OP_breg7 RSP-20
 
 ; 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
-; (9 is the last index in debug_loc.dwo, making 10 entries of 8 bytes each,
-; 10 * 8 == 80 base 10 == 50 base 16)
+; (10 is the last index in debug_loc.dwo, making 11 entries of 8 bytes each,
+; 11 * 8 == 88 base 10 == 58 base 16)
 
-; HDR: .debug_addr 00000050
+; HDR: .debug_addr 00000058
 ; HDR-NOT: .rela.{{.*}}.dwo
 
 ; Check for the existence of a DWARF v5-style range list table in the .debug_rnglists
 ; section and that the compile unit has a DW_AT_rnglists_base attribute.
 ; The table should contain at least one rangelist with at least 2 individual ranges.
 
 ; V5RNGLISTS:      .debug_info contents:
 ; V5RNGLISTS:      DW_TAG_compile_unit

llvm/trunk/test/DebugInfo/X86/pr19307.mir

 # RUN: llc -o - %s -start-after=patchable-function -O0 | FileCheck %s
 
+# XFAIL: *
+# Marked XFail due to the removal of "ChangingRegs" from
+# DbgEntityHistoryCalculator, shortening the checked range to not reach the
+# end of the function. Fixed by an about-to-land patch using the FrameDestroy
+# flag to identify the end of functions.
+
 # Generated from the source file pr19307.cc:
 # #include <string>
 # void parse_range(unsigned long long &offset, unsigned long long &limit,
 #                  std::string range) {
 #   if (range.compare(0, 6, "items=") != 0 || range[6] == '-')
 #     offset = 1;
 #   range.erase(0, 6);
 #   limit = 2;

llvm/trunk/test/DebugInfo/X86/reference-argument.ll

 ; RUN: llc -mtriple=x86_64-apple-macosx10.9.0 -filetype=obj -O0 < %s \
 ; RUN:   | llvm-dwarfdump -v -debug-info - | FileCheck %s
 ; ModuleID = 'aggregate-indirect-arg.cpp'
 ; extracted from debuginfo-tests/aggregate-indirect-arg.cpp
 
 ; v should be a pointer.
 ; CHECK:  .debug_info contents:
 ; CHECK:   DW_TAG_subprogram
 ; CHECK:     DW_AT_specification {{.*}} "_ZN1A3fooE4SVal"
 ; CHECK-NOT: DW_TAG_subprogram
 ; CHECK:     DW_TAG_formal_parameter
 ; CHECK:       DW_AT_name {{.*}} "this"
 ; CHECK-NOT:   DW_TAG_subprogram
 ; CHECK:     DW_TAG_formal_parameter
-; CHECK-NEXT:  DW_AT_location {{.*}}(DW_OP_breg4 RSI+0)
+; CHECK-NEXT:  DW_AT_location
+; CHECK-NEXT:  DW_OP_breg4 RSI+0
 ; CHECK-NEXT:  DW_AT_name {{.*}} "v"
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.9.0"
 
 %class.SVal = type { i8*, i32 }
 %class.A = type { i8 }
 

llvm/trunk/test/DebugInfo/X86/stack-value-piece.ll

 ; CHECK-NEXT:   DW_AT_location {{.*}} ([[I:.*]]
 ; CHECK-NEXT:     [{{.*}}, {{.*}}): DW_OP_reg5 RDI, DW_OP_piece 0x4, DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x4)
 ; CHECK-NEXT:   DW_AT_name {{.*}} "r"
 ;
 ; CHECK: DW_TAG_subprogram
 ; CHECK:   DW_AT_name {{.*}} "f"
 ; CHECK:   DW_TAG_variable
 ; CHECK-NEXT:   DW_AT_location {{.*}} ([[F:.*]]
-; CHECK-NEXT:     [{{.*}}, {{.*}}): DW_OP_reg17 XMM0, DW_OP_piece 0x4, DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x4)
+; CHECK-NEXT:     [{{.*}}, {{.*}}): DW_OP_reg17 XMM0, DW_OP_piece 0x4, DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x4
+; CHECK-NEXT:     [{{.*}}, {{.*}}): DW_OP_piece 0x4, DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x4)
 ; CHECK-NEXT:   DW_AT_name {{.*}} "r"
 ;
 ; CHECK: .debug_loc contents:
 ; CHECK:      [[I]]:
 ; CHECK-NEXT:   [{{.*}}, {{.*}}): DW_OP_reg5 RDI, DW_OP_piece 0x4, DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x4
 ; CHECK:      [[F]]:
 ; CHECK-NEXT:   [{{.*}}, {{.*}}): DW_OP_reg17 XMM0, DW_OP_piece 0x4, DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x4