[llvm] r215384 - Debug Info: Move the sorting and uniqueing of pieces from emitLocPieces()
Adrian Prantl
aprantl at apple.com
Mon Aug 11 16:33:46 PDT 2014
> On Aug 11, 2014, at 2:34 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
> On Mon, Aug 11, 2014 at 2:05 PM, Adrian Prantl <aprantl at apple.com> wrote:
>> Author: adrian
>> Date: Mon Aug 11 16:05:55 2014
>> New Revision: 215384
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=215384&view=rev
>> Log:
>> Debug Info: Move the sorting and uniqueing of pieces from emitLocPieces()
>> into buildLocationList(). By keeping the list of Values sorted,
>> DebugLocEntry::Merge can also merge multi-piece entries.
>
> Is this last sentence a description of new functionality that could be
Yes indeed it is. I’m still searching for a good testcase to reliably reproduce this though. It would certainly be good to test this.
> tested - or just "this merging can now happen here rather than where
> it was previously happening (somewhere else)"?
>
>>
>> Modified:
>> llvm/trunk/lib/CodeGen/AsmPrinter/DebugLocEntry.h
>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DebugLocEntry.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DebugLocEntry.h?rev=215384&r1=215383&r2=215384&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DebugLocEntry.h (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DebugLocEntry.h Mon Aug 11 16:05:55 2014
>> @@ -76,6 +76,13 @@ public:
>> llvm_unreachable("unhandled EntryKind");
>> }
>>
>> + // Compare two pieces based on their offset.
>> + bool operator<(const Value &other) const {
>
> Vague preference for operator overloads as non-members if they can be
> (the general justification being about implicit conversions being
> equally valid/desired on LHS and RHS - if it's a member you don't get
> LHS implicit conversions but you do get RHS implicit conversions...
> yeah, it's not a /great/ argument, but just a thought), friended if
> necessary.
Done in 215403.
>
>> + DIVariable Var(Variable);
>
> Could we just change Value::Variable's type to DIVariable?
This would make DebugLocEntry::Value loose its POD-likeness. I did add a couple of convenience accessors to it though, which should give us most of the same positive effects.
thanks,
adrian
>
>> + DIVariable OtherVar(other.Variable);
>> + return Var.getPieceOffset() < OtherVar.getPieceOffset();
>> + }
>> +
>> bool isLocation() const { return EntryKind == E_Location; }
>> bool isInt() const { return EntryKind == E_Integer; }
>> bool isConstantFP() const { return EntryKind == E_ConstantFP; }
>> @@ -87,7 +94,7 @@ public:
>> const MDNode *getVariable() const { return Variable; }
>> };
>> private:
>> - /// A list of locations/constants belonging to this entry.
>> + /// A list of locations/constants belonging to this entry, sorted by offset.
>> SmallVector<Value, 1> Values;
>>
>> public:
>> @@ -108,6 +115,7 @@ public:
>> if (Var.getName() == NextVar.getName() &&
>> Var.isVariablePiece() && NextVar.isVariablePiece()) {
>> Values.append(Next.Values.begin(), Next.Values.end());
>> + sortUniqueValues();
>> End = Next.End;
>> return true;
>> }
>> @@ -135,6 +143,14 @@ public:
>> assert(DIVariable(Val.Variable).isVariablePiece() &&
>> "multi-value DebugLocEntries must be pieces");
>> Values.push_back(Val);
>> + sortUniqueValues();
>> + }
>> +
>> + // Sort the pieces by offset.
>> + // Remove any duplicate entries by dropping all but the first.
>> + void sortUniqueValues() {
>> + std::sort(Values.begin(), Values.end());
>> + Values.erase(std::unique(Values.begin(), Values.end()), Values.end());
>> }
>> };
>>
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=215384&r1=215383&r2=215384&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Aug 11 16:05:55 2014
>> @@ -2065,30 +2065,18 @@ void DwarfDebug::emitDebugStr() {
>> void DwarfDebug::emitLocPieces(ByteStreamer &Streamer,
>> const DITypeIdentifierMap &Map,
>> ArrayRef<DebugLocEntry::Value> Values) {
>> - typedef DebugLocEntry::Value Piece;
>> - SmallVector<Piece, 4> Pieces(Values.begin(), Values.end());
>> - assert(std::all_of(Pieces.begin(), Pieces.end(), [](Piece &P) {
>> + assert(std::all_of(Values.begin(), Values.end(), [](DebugLocEntry::Value P) {
>> return DIVariable(P.getVariable()).isVariablePiece();
>> }) && "all values are expected to be pieces");
>> -
>> - // Sort the pieces so they can be emitted using DW_OP_piece.
>> - std::sort(Pieces.begin(), Pieces.end(), [](const Piece &A, const Piece &B) {
>> - DIVariable VarA(A.getVariable());
>> - DIVariable VarB(B.getVariable());
>> - return VarA.getPieceOffset() < VarB.getPieceOffset();
>> - });
>> - // Remove any duplicate entries by dropping all but the first.
>> - Pieces.erase(std::unique(Pieces.begin(), Pieces.end(),
>> - [] (const Piece &A,const Piece &B){
>> - return A.getVariable() == B.getVariable();
>> - }), Pieces.end());
>> + assert(std::is_sorted(Values.begin(), Values.end()) &&
>> + "pieces are expected to be sorted");
>>
>> unsigned Offset = 0;
>> - for (auto Piece : Pieces) {
>> + for (auto Piece : Values) {
>> DIVariable Var(Piece.getVariable());
>> unsigned PieceOffset = Var.getPieceOffset();
>> unsigned PieceSize = Var.getPieceSize();
>> - assert(Offset <= PieceOffset && "overlapping pieces in DebugLocEntry");
>> + assert(Offset <= PieceOffset && "overlapping or duplicate pieces");
>> if (Offset < PieceOffset) {
>> // The DWARF spec seriously mandates pieces with no locations for gaps.
>> Asm->EmitDwarfOpPiece(Streamer, (PieceOffset-Offset)*8);
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list