[DebugInfo] Pass all values in DebugLocEntry's constructor, NFC

[dstenb]

Summary

With MergeValues() removed, amend DebugLocEntry's constructor so that it
takes multiple values rather than a single, and keep non-fragment values
in OpenRanges, as this allows some cleanup of the code in
buildLocationList().

[dblaikie]

lib/CodeGen/AsmPrinter/DebugLocEntry.h

127 ↗

(On Diff #190427)

This'll get flagged as an unused variable (-Wunused) in a no-asserts build, so best to roll it back into the assertion.

[dstenb]

Move lambda expression back into assert() to avoid -Wunused-variable warning in non-assert builds.

[dstenb]

Ping.

[aprantl]

llvm/lib/CodeGen/AsmPrinter/DebugLocEntry.h

103 ↗

(On Diff #190522)

Since you modified this, you are probably in a good position to add a doxygen comment explaining what the individual parameters mean? Alternatively renaming B to Begin might also suffice.

[dstenb]

Rebase, and add Doxygen comment.

[dstenb]

llvm/lib/CodeGen/AsmPrinter/DebugLocEntry.h

103 ↗

(On Diff #190522)

I renamed the variables and added a Doxygen comment to clarify that End is excluded from the range, and that Vals can't be empty.

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

#endif
   };
 
 private:
   /// A nonempty list of locations/constants belonging to this entry,
   /// sorted by offset.
   SmallVector<Value, 1> Values;
 
 public:
-  DebugLocEntry(const MCSymbol *B, const MCSymbol *E, Value Val)
-      : Begin(B), End(E) {
-    Values.push_back(std::move(Val));
+  /// Create a location list entry for the range [\p Begin, \p End).
+  ///
+  /// \param Vals One or more values describing (parts of) the variable.
+  DebugLocEntry(const MCSymbol *Begin, const MCSymbol *End,
+                ArrayRef<Value> Vals)
+      : Begin(Begin), End(End) {
+    addValues(Vals);
   }
 
   /// Attempt to merge this DebugLocEntry with Next and return
   /// true if the merge was successful. Entries can be merged if they
   /// share the same Loc/Constant and if Next immediately follows this
   /// Entry.
   bool MergeRanges(const DebugLocEntry &Next) {
     // If this and Next are describing the same variable, merge them.
     if ((End == Next.Begin && Values == Next.Values)) {
       End = Next.End;
       return true;
     }
     return false;
   }
 
   const MCSymbol *getBeginSym() const { return Begin; }
   const MCSymbol *getEndSym() const { return End; }
   ArrayRef<Value> getValues() const { return Values; }
   void addValues(ArrayRef<DebugLocEntry::Value> Vals) {
     Values.append(Vals.begin(), Vals.end());
     sortUniqueValues();
-    assert(all_of(Values, [](DebugLocEntry::Value V) {
-          return V.isFragment();
-        }) && "value must be a piece");
+    assert((Values.size() == 1 ||
+            all_of(Values,
+                   [](DebugLocEntry::Value V) { return V.isFragment(); })) &&
+           "must either have a single value or multiple pieces");
   }
 
   // Sort the pieces by offset.
   // Remove any duplicate entries by dropping all but the first.
   void sortUniqueValues() {
     llvm::sort(Values);
     Values.erase(
         std::unique(

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

for (auto I = Ranges.begin(), E = Ranges.end(); I != E; ++I) {
 
     // Check if a variable is inaccessible in this range.
     if (Begin->getNumOperands() > 1 &&
         Begin->getOperand(0).isReg() && !Begin->getOperand(0).getReg()) {
       OpenRanges.clear();
       continue;
     }
 
-    // If this fragment overlaps with any open ranges, truncate them.
+    // If this debug value overlaps with any open ranges, truncate them.
     const DIExpression *DIExpr = Begin->getDebugExpression();
     auto Last = remove_if(OpenRanges, [&](DebugLocEntry::Value R) {
       return DIExpr->fragmentsOverlap(R.getExpression());
     });
     OpenRanges.erase(Last, OpenRanges.end());
 
     const MCSymbol *StartLabel = getLabelBeforeInsn(Begin);
     assert(StartLabel && "Forgot label before DBG_VALUE starting a range!");
 
     const MCSymbol *EndLabel;
     if (End != nullptr)
       EndLabel = getLabelAfterInsn(End);
     else if (std::next(I) == Ranges.end())
       EndLabel = Asm->getFunctionEnd();
     else
       EndLabel = getLabelBeforeInsn(std::next(I)->first);
     assert(EndLabel && "Forgot label after instruction ending a range!");
 
     LLVM_DEBUG(dbgs() << "DotDebugLoc: " << *Begin << "\n");
 
     auto Value = getDebugLocValue(Begin);
+    OpenRanges.push_back(Value);
 
     // Omit entries with empty ranges as they do not have any effect in DWARF.
     if (StartLabel == EndLabel) {
-      // If this is a fragment, we must still add the value to the list of
-      // open ranges, since it may describe non-overlapping parts of the
-      // variable.
-      if (DIExpr->isFragment())
-        OpenRanges.push_back(Value);
       LLVM_DEBUG(dbgs() << "Omitting location list entry with empty range.\n");
       continue;
     }
 
-    DebugLocEntry Loc(StartLabel, EndLabel, Value);
-
-    if (DIExpr->isFragment()) {
-      // Add this value to the list of open ranges.
-      OpenRanges.push_back(Value);
-    }
-
-    // Add all values from still valid non-overlapping fragments.
-    if (OpenRanges.size())
-      Loc.addValues(OpenRanges);
-
-    DebugLoc.push_back(std::move(Loc));
+    DebugLoc.emplace_back(StartLabel, EndLabel, OpenRanges);
 
     // Attempt to coalesce the ranges of two otherwise identical
     // DebugLocEntries.
     auto CurEntry = DebugLoc.rbegin();
     LLVM_DEBUG({
       dbgs() << CurEntry->getValues().size() << " Values:\n";
       for (auto &Value : CurEntry->getValues())
         Value.dump();
static void emitDebugLocValue(const AsmPrinter &AP, const DIBasicType *BT,
   }
   DwarfExpr.addExpression(std::move(ExprCursor));
 }
 
 void DebugLocEntry::finalize(const AsmPrinter &AP,
                              DebugLocStream::ListBuilder &List,
                              const DIBasicType *BT,
                              DwarfCompileUnit &TheCU) {
+  assert(!Values.empty() &&
+         "location list entries without values are redundant");
   assert(Begin != End && "unexpected location list entry with empty range");
   DebugLocStream::EntryBuilder Entry(List, Begin, End);
   BufferByteStreamer Streamer = Entry.getStreamer();
   DebugLocDwarfExpression DwarfExpr(AP.getDwarfVersion(), Streamer, TheCU);
   const DebugLocEntry::Value &Value = Values[0];
   if (Value.isFragment()) {
     // Emit all fragments that belong to the same variable and range.
     assert(llvm::all_of(Values, [](DebugLocEntry::Value P) {