[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

(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

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

236–237

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.

237

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

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

[bjope]

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

240

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

237

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.

240

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?

[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

240

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

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

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.

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp

static void clobberRegisterUses(RegDescribedVarsMap &RegVars, unsigned RegNo,
                                 DbgValueHistoryMap &HistMap,
                                 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);
-              }
-            }
-          }
-        }
-        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());
+      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 frame-register defs in the epilogue, we expect
+          // debuggers to understand that stack-locations are destroyed on
+          // frame destruction.
+          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);
 
-        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);
+          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.
-      SmallSet<unsigned, 8> RegsToClobber;
       for (auto &Pair : LiveEntries) {
-        // Iterate over history entries for all open fragments.
-        SmallVector<EntryIndex, 8> IdxesToRemove;
+        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());
-          const MachineInstr *DbgValue = Ent.getInstr();
-
-          // If this is a register or indirect DBG_VALUE, apply some futher
-          // tests to see if we should clobber it. Perform the clobbering
-          // later though, to keep LiveEntries iteration stable.
-          if (DbgValue->getOperand(0).isReg()) {
-            unsigned RegNo = DbgValue->getOperand(0).getReg();
-            if (TRI->isVirtualRegister(RegNo) || ChangingRegs.test(RegNo) ||
-                FrameReg == RegNo)
-              RegsToClobber.insert(RegNo);
-          } else {
-            // This is a constant, terminate it at end of the block. Store
-            // eliminated EntryIdx and delete later, for iteration stability.
-            EntryIndex ClobIdx = DbgValues.startClobber(Pair.first, MBB.back());
-            DbgValues.getEntry(Pair.first, Idx).endEntry(ClobIdx);
-            IdxesToRemove.push_back(Idx);
-          }
-        }
+          Ent.endEntry(ClobIdx);
 
-        for (EntryIndex Idx : IdxesToRemove)
-          Pair.second.erase(Idx);
-      }
-
-      // Implement clobbering of registers at the end of BB.
-      for (unsigned Reg : RegsToClobber)
-        clobberRegisterUses(RegVars, Reg, 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) {

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;

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;

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

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;

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 }
 

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