[llvm] r285827 - Simplify control flow in the the DWARF expression compiler

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 2 09:12:20 PDT 2016


Author: adrian
Date: Wed Nov  2 11:12:20 2016
New Revision: 285827

URL: http://llvm.org/viewvc/llvm-project?rev=285827&view=rev
Log:
Simplify control flow in the the DWARF expression compiler
by refactoring common code into a DwarfExpressionCursor wrapper.

Modified:
    llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.h

Modified: llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfoMetadata.h?rev=285827&r1=285826&r2=285827&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/DebugInfoMetadata.h (original)
+++ llvm/trunk/include/llvm/IR/DebugInfoMetadata.h Wed Nov  2 11:12:20 2016
@@ -1951,6 +1951,7 @@ public:
     const uint64_t *Op;
 
   public:
+    ExprOperand() : Op(nullptr) {};
     explicit ExprOperand(const uint64_t *Op) : Op(Op) {}
 
     const uint64_t *get() const { return Op; }
@@ -1977,6 +1978,7 @@ public:
     ExprOperand Op;
 
   public:
+    expr_op_iterator() = default;
     explicit expr_op_iterator(element_iterator I) : Op(I) {}
 
     element_iterator getBase() const { return Op.get(); }

Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=285827&r1=285826&r2=285827&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (original)
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp Wed Nov  2 11:12:20 2016
@@ -176,7 +176,7 @@ DIE *DwarfCompileUnit::getOrCreateGlobal
 
       if (Expr) {
         DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
-        DwarfExpr.AddExpression(Expr->expr_op_begin(), Expr->expr_op_end());
+        DwarfExpr.AddExpression(Expr);
       }
     }
 
@@ -495,7 +495,7 @@ DIE *DwarfCompileUnit::constructVariable
         DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
         // If there is an expression, emit raw unsigned bytes.
         DwarfExpr.AddUnsignedConstant(DVInsn->getOperand(0).getImm());
-        DwarfExpr.AddExpression(Expr->expr_op_begin(), Expr->expr_op_end());
+        DwarfExpr.AddExpression(Expr);
         addBlock(*VariableDie, dwarf::DW_AT_location, Loc);
       } else
         addConstantValue(*VariableDie, DVInsn->getOperand(0), DV.getType());
@@ -522,7 +522,7 @@ DIE *DwarfCompileUnit::constructVariable
     assert(Expr != DV.getExpression().end() && "Wrong number of expressions");
     DwarfExpr.AddMachineRegIndirect(*Asm->MF->getSubtarget().getRegisterInfo(),
                                     FrameReg, Offset);
-    DwarfExpr.AddExpression((*Expr)->expr_op_begin(), (*Expr)->expr_op_end());
+    DwarfExpr.AddExpression(*Expr);
     ++Expr;
   }
   addBlock(*VariableDie, dwarf::DW_AT_location, Loc);
@@ -746,20 +746,21 @@ void DwarfCompileUnit::addComplexAddress
                                          const MachineLocation &Location) {
   DIELoc *Loc = new (DIEValueAllocator) DIELoc;
   DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
-  const DIExpression *Expr = DV.getSingleExpression();
-  bool ValidReg;
+  DIExpressionCursor ExprCursor(DV.getSingleExpression());
   const TargetRegisterInfo &TRI = *Asm->MF->getSubtarget().getRegisterInfo();
-  if (Location.getOffset()) {
-    ValidReg = DwarfExpr.AddMachineRegIndirect(TRI, Location.getReg(),
-                                               Location.getOffset());
-    if (ValidReg)
-      DwarfExpr.AddExpression(Expr->expr_op_begin(), Expr->expr_op_end());
-  } else
-    ValidReg = DwarfExpr.AddMachineRegExpression(TRI, Expr, Location.getReg());
+  auto Reg = Location.getReg();
+  bool ValidReg =
+      Location.getOffset()
+          ? DwarfExpr.AddMachineRegIndirect(TRI, Reg, Location.getOffset())
+          : DwarfExpr.AddMachineRegExpression(TRI, ExprCursor, Reg);
+
+  if (!ValidReg)
+    return;
+
+  DwarfExpr.AddExpression(std::move(ExprCursor));
 
   // Now attach the location information to the DIE.
-  if (ValidReg)
-    addBlock(Die, Attribute, Loc);
+  addBlock(Die, Attribute, Loc);
 }
 
 /// Add a Dwarf loclistptr attribute data and value.

Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=285827&r1=285826&r2=285827&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Wed Nov  2 11:12:20 2016
@@ -1406,7 +1406,7 @@ static void emitDebugLocValue(const AsmP
                               ByteStreamer &Streamer,
                               const DebugLocEntry::Value &Value,
                               unsigned PieceOffsetInBits) {
-  const DIExpression *Expr = Value.getExpression();
+  DIExpressionCursor ExprCursor(Value.getExpression());
   DebugLocDwarfExpression DwarfExpr(AP.getDwarfDebug()->getDwarfVersion(),
                                     Streamer);
   // Regular entry.
@@ -1418,26 +1418,23 @@ static void emitDebugLocValue(const AsmP
       DwarfExpr.AddUnsignedConstant(Value.getInt());
   } else if (Value.isLocation()) {
     MachineLocation Loc = Value.getLoc();
-    if (!Expr || !Expr->getNumElements())
+    if (!ExprCursor)
       // Regular entry.
       AP.EmitDwarfRegOp(Streamer, Loc);
     else {
       // Complex address entry.
       const TargetRegisterInfo &TRI = *AP.MF->getSubtarget().getRegisterInfo();
-      if (Loc.getOffset()) {
+      if (Loc.getOffset())
         DwarfExpr.AddMachineRegIndirect(TRI, Loc.getReg(), Loc.getOffset());
-      } else {
-        DwarfExpr.AddMachineRegExpression(TRI, Expr, Loc.getReg(),
+      else
+        DwarfExpr.AddMachineRegExpression(TRI, ExprCursor, Loc.getReg(),
                                           PieceOffsetInBits);
-        return;
-      }
     }
   } else if (Value.isConstantFP()) {
     APInt RawBytes = Value.getConstantFP()->getValueAPF().bitcastToAPInt();
     DwarfExpr.AddUnsignedConstant(RawBytes);
   }
-  DwarfExpr.AddExpression(Expr->expr_op_begin(), Expr->expr_op_end(),
-                          PieceOffsetInBits);
+  DwarfExpr.AddExpression(std::move(ExprCursor), PieceOffsetInBits);
 }
 
 void DebugLocEntry::finalize(const AsmPrinter &AP,

Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp?rev=285827&r1=285826&r2=285827&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp (original)
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp Wed Nov  2 11:12:20 2016
@@ -203,76 +203,69 @@ static unsigned getOffsetOrZero(unsigned
 }
 
 bool DwarfExpression::AddMachineRegExpression(const TargetRegisterInfo &TRI,
-                                              const DIExpression *Expr,
+                                              DIExpressionCursor &ExprCursor,
                                               unsigned MachineReg,
                                               unsigned PieceOffsetInBits) {
-  auto I = Expr->expr_op_begin();
-  auto E = Expr->expr_op_end();
-  if (I == E)
+  if (!ExprCursor)
     return AddMachineRegPiece(TRI, MachineReg);
 
   // Pattern-match combinations for which more efficient representations exist
   // first.
   bool ValidReg = false;
-  switch (I->getOp()) {
+  auto Op = ExprCursor.peek();
+  switch (Op->getOp()) {
   case dwarf::DW_OP_bit_piece: {
-    unsigned OffsetInBits = I->getArg(0);
-    unsigned SizeInBits   = I->getArg(1);
+    unsigned OffsetInBits = Op->getArg(0);
+    unsigned SizeInBits = Op->getArg(1);
     // Piece always comes at the end of the expression.
-    return AddMachineRegPiece(TRI, MachineReg, SizeInBits,
-               getOffsetOrZero(OffsetInBits, PieceOffsetInBits));
+    AddMachineRegPiece(TRI, MachineReg, SizeInBits,
+                       getOffsetOrZero(OffsetInBits, PieceOffsetInBits));
+    ExprCursor.take();
+    break;
   }
   case dwarf::DW_OP_plus:
   case dwarf::DW_OP_minus: {
     // [DW_OP_reg,Offset,DW_OP_plus, DW_OP_deref] --> [DW_OP_breg, Offset].
     // [DW_OP_reg,Offset,DW_OP_minus,DW_OP_deref] --> [DW_OP_breg,-Offset].
-    auto N = I.getNext();
-    if (N != E && N->getOp() == dwarf::DW_OP_deref) {
-      unsigned Offset = I->getArg(0);
+    auto N = ExprCursor.peekNext();
+    if (N && N->getOp() == dwarf::DW_OP_deref) {
+      unsigned Offset = Op->getArg(0);
       ValidReg = AddMachineRegIndirect(
-          TRI, MachineReg, I->getOp() == dwarf::DW_OP_plus ? Offset : -Offset);
-      std::advance(I, 2);
-      break;
+          TRI, MachineReg, Op->getOp() == dwarf::DW_OP_plus ? Offset : -Offset);
+      ExprCursor.consume(2);
     } else
       ValidReg = AddMachineRegPiece(TRI, MachineReg);
+    break;
   }
-  case dwarf::DW_OP_deref: {
-      // [DW_OP_reg,DW_OP_deref] --> [DW_OP_breg].
-      ValidReg = AddMachineRegIndirect(TRI, MachineReg);
-      ++I;
-      break;
-  }
-  default:
-    llvm_unreachable("unsupported operand");
+  case dwarf::DW_OP_deref:
+    // [DW_OP_reg,DW_OP_deref] --> [DW_OP_breg].
+    ValidReg = AddMachineRegIndirect(TRI, MachineReg);
+    ExprCursor.take();
+    break;
   }
 
-  if (!ValidReg)
-    return false;
-
-  // Emit remaining elements of the expression.
-  AddExpression(I, E, PieceOffsetInBits);
-  return true;
+  return ValidReg;
 }
 
-void DwarfExpression::AddExpression(DIExpression::expr_op_iterator I,
-                                    DIExpression::expr_op_iterator E,
+void DwarfExpression::AddExpression(DIExpressionCursor &&ExprCursor,
                                     unsigned PieceOffsetInBits) {
-  for (; I != E; ++I) {
-    switch (I->getOp()) {
+  while (ExprCursor) {
+    auto Op = ExprCursor.take();
+    switch (Op->getOp()) {
     case dwarf::DW_OP_bit_piece: {
-      unsigned OffsetInBits = I->getArg(0);
-      unsigned SizeInBits   = I->getArg(1);
+      unsigned OffsetInBits = Op->getArg(0);
+      unsigned SizeInBits   = Op->getArg(1);
       AddOpPiece(SizeInBits, getOffsetOrZero(OffsetInBits, PieceOffsetInBits));
       break;
     }
     case dwarf::DW_OP_plus:
       EmitOp(dwarf::DW_OP_plus_uconst);
-      EmitUnsigned(I->getArg(0));
+      EmitUnsigned(Op->getArg(0));
       break;
     case dwarf::DW_OP_minus:
       // There is no OP_minus_uconst.
       EmitOp(dwarf::DW_OP_constu);
-      EmitUnsigned(I->getArg(0));
+      EmitUnsigned(Op->getArg(0));
       EmitOp(dwarf::DW_OP_minus);
       break;
     case dwarf::DW_OP_deref:
@@ -280,7 +273,7 @@ void DwarfExpression::AddExpression(DIEx
       break;
     case dwarf::DW_OP_constu:
       EmitOp(dwarf::DW_OP_constu);
-      EmitUnsigned(I->getArg(0));
+      EmitUnsigned(Op->getArg(0));
       break;
     case dwarf::DW_OP_stack_value:
       AddStackValue();

Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.h?rev=285827&r1=285826&r2=285827&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.h (original)
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.h Wed Nov  2 11:12:20 2016
@@ -25,6 +25,51 @@ class TargetRegisterInfo;
 class DwarfUnit;
 class DIELoc;
 
+/// Holds a DIExpression and keeps track of how many operands have been consumed
+/// so far.
+class DIExpressionCursor {
+  DIExpression::expr_op_iterator Start, End;
+public:
+  DIExpressionCursor(const DIExpression *Expr) {
+    if (!Expr) {
+      assert(Start == End);
+      return;
+    }
+    Start = Expr->expr_op_begin();
+    End = Expr->expr_op_end();
+  }
+
+  /// Consume one operation.
+  Optional<DIExpression::ExprOperand> take() {
+    if (Start == End)
+      return None;
+    return *(Start++);
+  }
+
+  /// Consume N operations.
+  void consume(unsigned N) { std::advance(Start, N); }
+
+  /// Return the current operation.
+  Optional<DIExpression::ExprOperand> peek() const {
+    if (Start == End)
+      return None;
+    return *(Start);
+  }
+
+  /// Return the next operation.
+  Optional<DIExpression::ExprOperand> peekNext() const {
+    if (Start == End)
+      return None;
+
+    auto Next = Start.getNext();
+    if (Next == End)
+      return None;
+
+    return *Next;
+  }
+  operator bool() const { return Start != End; }
+};
+
 /// Base class containing the logic for constructing DWARF expressions
 /// independently of whether they are emitted into a DIE or into a .debug_loc
 /// entry.
@@ -77,7 +122,7 @@ public:
   bool AddMachineRegIndirect(const TargetRegisterInfo &TRI, unsigned MachineReg,
                              int Offset = 0);
 
-  /// \brief Emit a partial DWARF register operation.
+  /// Emit a partial DWARF register operation.
   /// \param MachineReg        the register
   /// \param PieceSizeInBits   size and
   /// \param PieceOffsetInBits offset of the piece in bits, if this is one
@@ -102,19 +147,19 @@ public:
   /// Emit an unsigned constant.
   void AddUnsignedConstant(const APInt &Value);
 
-  /// \brief Emit an entire expression on top of a machine register location.
+  /// Emit a machine register location while consuming the prefix of a
+  /// DwarfExpression.
   ///
-  /// \param PieceOffsetInBits If this is one piece out of a fragmented
+  /// \param PieceOffsetInBits     If this is one piece out of a fragmented
   /// location, this is the offset of the piece inside the entire variable.
   /// \return false if no DWARF register exists for MachineReg.
   bool AddMachineRegExpression(const TargetRegisterInfo &TRI,
-                               const DIExpression *Expr, unsigned MachineReg,
+                               DIExpressionCursor &Expr, unsigned MachineReg,
                                unsigned PieceOffsetInBits = 0);
-  /// Emit a the operations remaining the DIExpressionIterator I.
-  /// \param PieceOffsetInBits If this is one piece out of a fragmented
+  /// Emit all remaining operations in the DIExpressionCursor.
+  /// \param PieceOffsetInBits     If this is one piece out of a fragmented
   /// location, this is the offset of the piece inside the entire variable.
-  void AddExpression(DIExpression::expr_op_iterator I,
-                     DIExpression::expr_op_iterator E,
+  void AddExpression(DIExpressionCursor &&Expr,
                      unsigned PieceOffsetInBits = 0);
 };
 




More information about the llvm-commits mailing list