[llvm] r204679 - DwarfDebug: Simplify debug_loc merging

[Eric Christopher]

On Mon, Mar 24, 2014 at 4:08 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Mon, Mar 24, 2014 at 4:03 PM, Adrian Prantl <aprantl at apple.com> wrote:
>> Although it means updating my patch-in-review,
>
> Right - sorry about that!
>
>> I really like the direction this is going. The old design with the isMerged flag bothered me quite a bit :-)
>
> Yeah - it was pretty gross.

Agreed. It's been on my mental todo list forever.

This should also solve the problem (I think) that you could see IIRC
in sret.cpp where you'd have a zero length location list.

>
> r204680 is as far as I'm going for now - though we could go further,
> rather than using raw vectors for each list, we could create a
> structure with a label (so we don't create the same label name in two
> places "magically" (spooking action at a distance/hardcoded
> constants/etc)), maybe even use a linked list of vectors then we could
> just point to the vectors (rather than using an index) from the DIEs,
> etc. That way we wouldn't have to look it up again in the DIE hashing
> code, we'd just follow the pointer.

Fair. That might be a good incremental on top.

-eric

>
> - David
>
>>
>> thanks,
>> adrian
>>
>> On Mar 24, 2014, at 3:27 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>> Author: dblaikie
>>> Date: Mon Mar 24 17:27:06 2014
>>> New Revision: 204679
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=204679&view=rev
>>> Log:
>>> DwarfDebug: Simplify debug_loc merging
>>>
>>> No functional change intended.
>>>
>>> Merging up-front rather than delaying this task until later. This just
>>> seems simpler and more efficient (avoiding growing the debug loc list
>>> only to have to skip over those post-merged entries, etc).
>>>
>>> Modified:
>>>    llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp
>>>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>>
>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp?rev=204679&r1=204678&r2=204679&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp Mon Mar 24 17:27:06 2014
>>> @@ -296,8 +296,6 @@ void DIEHash::hashLocList(const DIELocLi
>>>     // which is the next empty entry.
>>>     if (Entry.isEmpty())
>>>       return;
>>> -    else if (Entry.isMerged())
>>> -      continue;
>>>     else
>>>       AP->getDwarfDebug()->emitDebugLocEntry(Streamer, Entry);
>>>   }
>>>
>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=204679&r1=204678&r2=204679&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Mar 24 17:27:06 2014
>>> @@ -1297,8 +1297,9 @@ DwarfDebug::collectVariableInfo(SmallPtr
>>>       // The value is valid until the next DBG_VALUE or clobber.
>>>       LexicalScope *FnScope = LScopes.getCurrentFunctionScope();
>>>       DwarfCompileUnit *TheCU = SPMap.lookup(FnScope->getScopeNode());
>>> -      DotDebugLocEntries.push_back(
>>> -          getDebugLocEntry(Asm, FLabel, SLabel, Begin, TheCU));
>>> +      DebugLocEntry Loc = getDebugLocEntry(Asm, FLabel, SLabel, Begin, TheCU);
>>> +      if (DotDebugLocEntries.empty() || !DotDebugLocEntries.back().Merge(Loc))
>>> +        DotDebugLocEntries.push_back(std::move(Loc));
>>>     }
>>>     DotDebugLocEntries.push_back(DebugLocEntry());
>>>   }
>>> @@ -2378,15 +2379,6 @@ void DwarfDebug::emitDebugLoc() {
>>>   if (DotDebugLocEntries.empty())
>>>     return;
>>>
>>> -  for (SmallVectorImpl<DebugLocEntry>::iterator
>>> -           I = DotDebugLocEntries.begin(),
>>> -           E = DotDebugLocEntries.end();
>>> -       I != E; ++I) {
>>> -    DebugLocEntry &Entry = *I;
>>> -    if (I + 1 != DotDebugLocEntries.end())
>>> -      Entry.Merge(I + 1);
>>> -  }
>>> -
>>>   // Start the dwarf loc section.
>>>   Asm->OutStreamer.SwitchSection(
>>>       Asm->getObjFileLowering().getDwarfLocSection());
>>> @@ -2398,8 +2390,6 @@ void DwarfDebug::emitDebugLoc() {
>>>            E = DotDebugLocEntries.end();
>>>        I != E; ++I, ++index) {
>>>     const DebugLocEntry &Entry = *I;
>>> -    if (Entry.isMerged())
>>> -      continue;
>>>
>>>     if (Entry.isEmpty()) {
>>>       Asm->OutStreamer.EmitIntValue(0, Size);
>>>
>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=204679&r1=204678&r2=204679&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Mon Mar 24 17:27:06 2014
>>> @@ -84,34 +84,31 @@ class DebugLocEntry {
>>>   // The compile unit to which this location entry is referenced by.
>>>   const DwarfCompileUnit *Unit;
>>>
>>> -  // Whether this location has been merged.
>>> -  bool Merged;
>>> -
>>> public:
>>> -  DebugLocEntry() : Begin(0), End(0), Variable(0), Unit(0), Merged(false) {
>>> +  DebugLocEntry() : Begin(0), End(0), Variable(0), Unit(0) {
>>>     Constants.Int = 0;
>>>   }
>>>   DebugLocEntry(const MCSymbol *B, const MCSymbol *E, MachineLocation &L,
>>>                 const MDNode *V, const DwarfCompileUnit *U)
>>> -      : Begin(B), End(E), Loc(L), Variable(V), Unit(U), Merged(false) {
>>> +      : Begin(B), End(E), Loc(L), Variable(V), Unit(U) {
>>>     Constants.Int = 0;
>>>     EntryKind = E_Location;
>>>   }
>>>   DebugLocEntry(const MCSymbol *B, const MCSymbol *E, int64_t i,
>>>                 const DwarfCompileUnit *U)
>>> -      : Begin(B), End(E), Variable(0), Unit(U), Merged(false) {
>>> +      : Begin(B), End(E), Variable(0), Unit(U) {
>>>     Constants.Int = i;
>>>     EntryKind = E_Integer;
>>>   }
>>>   DebugLocEntry(const MCSymbol *B, const MCSymbol *E, const ConstantFP *FPtr,
>>>                 const DwarfCompileUnit *U)
>>> -      : Begin(B), End(E), Variable(0), Unit(U), Merged(false) {
>>> +      : Begin(B), End(E), Variable(0), Unit(U) {
>>>     Constants.CFP = FPtr;
>>>     EntryKind = E_ConstantFP;
>>>   }
>>>   DebugLocEntry(const MCSymbol *B, const MCSymbol *E, const ConstantInt *IPtr,
>>>                 const DwarfCompileUnit *U)
>>> -      : Begin(B), End(E), Variable(0), Unit(U), Merged(false) {
>>> +      : Begin(B), End(E), Variable(0), Unit(U) {
>>>     Constants.CIP = IPtr;
>>>     EntryKind = E_ConstantInt;
>>>   }
>>> @@ -119,12 +116,11 @@ public:
>>>   /// \brief Empty entries are also used as a trigger to emit temp label. Such
>>>   /// labels are referenced is used to find debug_loc offset for a given DIE.
>>>   bool isEmpty() const { return Begin == 0 && End == 0; }
>>> -  bool isMerged() const { return Merged; }
>>> -  void Merge(DebugLocEntry *Next) {
>>> -    if (!(Begin && Loc == Next->Loc && End == Next->Begin))
>>> -      return;
>>> -    Next->Begin = Begin;
>>> -    Merged = true;
>>> +  bool Merge(const DebugLocEntry &Next) {
>>> +    if (!(Begin && Loc == Next.Loc && End == Next.Begin))
>>> +      return false;
>>> +    End = Next.End;
>>> +    return true;
>>>   }
>>>   bool isLocation() const { return EntryKind == E_Location; }
>>>   bool isInt() const { return EntryKind == E_Integer; }
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>