[llvm] r215384 - Debug Info: Move the sorting and uniqueing of pieces from emitLocPieces()

[David Blaikie]

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
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.

> +      DIVariable Var(Variable);

Could we just change Value::Variable's type to DIVariable?

> +      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