<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 23, 2015 at 1:24 PM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: adrian<br>
Date: Fri Jan 23 15:24:41 2015<br>
New Revision: 226939<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=226939&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=226939&view=rev</a><br>
Log:<br>
Move the accessor functions from DIExpression::iterator into a wrapper<br>
DIExpression::Operand, so we can write range-based for loops.<br>
<br>
Thanks to David Blaikie for the idea.<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/IR/DebugInfo.h<br>
llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp<br>
llvm/trunk/lib/IR/DebugInfo.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/IR/DebugInfo.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfo.h?rev=226939&r1=226938&r2=226939&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfo.h?rev=226939&r1=226938&r2=226939&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/IR/DebugInfo.h (original)<br>
+++ llvm/trunk/include/llvm/IR/DebugInfo.h Fri Jan 23 15:24:41 2015<br>
@@ -867,16 +867,17 @@ public:<br>
/// \brief Return the size of this piece in bytes.<br>
uint64_t getPieceSize() const;<br>
<br>
+ class Operand;<br>
+<br>
/// \brief An iterator for DIExpression elements.<br>
class iterator : public std::iterator<std::forward_iterator_tag, StringRef,<br>
unsigned, const uint64_t *, uint64_t> {<br></blockquote><div><br>I'm guessing these types passed to std::iterator need to be updated now that the value is an Operand, not a uint64_t.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
DIHeaderFieldIterator I;</blockquote><div><br>Really vaguely curious - should the expression be encoded as a raw byte sequence, rather than bytes in strings in a DIHeader? (bytes in strings seems inefficient?)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
iterator(DIHeaderFieldIterator I) : I(I) {}<br>
-<br>
public:<br>
iterator() {}<br>
iterator(const DIExpression &Expr) : I(++Expr.header_begin()) {}<br>
- uint64_t operator*() const { return I.getNumber<uint64_t>(); }<br>
+ Operand operator*() const { return Operand(I); }<br></blockquote><div><br>I /think/ you might need to return by reference than by value here (& have an Operand value as a member of the iterator) - I think that the iterator requirements require a reference to be returned or some other feature that makes returning by value not viable - but I could totally be wrong here, my memory is very vague.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
iterator &operator++() {<br>
increment();<br>
return *this;<br>
@@ -889,14 +890,7 @@ public:<br>
bool operator==(const iterator &X) const { return I == X.I; }<br>
bool operator!=(const iterator &X) const { return !(*this == X); }<br>
<br>
- uint64_t getArg(unsigned N) const {<br>
- auto In = I;<br>
- std::advance(In, N);<br>
- return In.getNumber<uint64_t>();<br>
- }<br>
-<br>
const DIHeaderFieldIterator &getBase() const { return I; }<br>
-<br>
private:<br>
void increment() {<br>
switch (**this) {<br>
@@ -911,6 +905,22 @@ public:<br>
<br>
iterator begin() const;<br>
iterator end() const;<br>
+<br>
+ /// \brief A lightweight wrapper around an element of a DIExpression.<br>
+ class Operand {<br>
+ DIHeaderFieldIterator I;<br>
+ public:<br>
+ Operand(DIHeaderFieldIterator I) : I(I) {}<br></blockquote><div><br></div><div>should this be private & friendedly accessible from the expr iterator?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ /// \brief Operands such as DW_OP_piece have explicit (non-stack) arguments.<br>
+ /// Argument 0 is the operand itself.<br>
+ uint64_t getArg(unsigned N) const {<br>
+ DIHeaderFieldIterator In = I;<br>
+ std::advance(In, N);<br>
+ return In.getNumber<uint64_t>();<br>
+ }<br>
+<br>
+ operator uint64_t () const { return I.getNumber<uint64_t>(); }<br></blockquote><div><br>Maybe this should be an accessor with a name (& type safety?) rather than an implicit conversion?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ };<br>
};<br>
<br>
/// \brief This object holds location information.<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp?rev=226939&r1=226938&r2=226939&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp?rev=226939&r1=226938&r2=226939&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp Fri Jan 23 15:24:41 2015<br>
@@ -210,8 +210,8 @@ bool DwarfExpression::AddMachineRegExpre<br>
switch (*I) {<br></blockquote><div><br>Can the loop this is within, be turend into a range-based-for loop so everyone doesn't have to do dereference the iterator.<br><br>If not, at least all the "(*I)." below could change to "I->" instead?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
case dwarf::DW_OP_piece: {<br>
unsigned SizeOfByte = 8;<br>
- unsigned OffsetInBits = I.getArg(1) * SizeOfByte;<br>
- unsigned SizeInBits = I.getArg(2) * SizeOfByte;<br>
+ unsigned OffsetInBits = (*I).getArg(1) * SizeOfByte;<br>
+ unsigned SizeInBits = (*I).getArg(2) * SizeOfByte;<br>
// Piece always comes at the end of the expression.<br>
return AddMachineRegPiece(MachineReg, SizeInBits,<br>
getOffsetOrZero(OffsetInBits, PieceOffsetInBits));<br>
@@ -219,7 +219,7 @@ bool DwarfExpression::AddMachineRegExpre<br>
case dwarf::DW_OP_plus:<br>
// [DW_OP_reg,Offset,DW_OP_plus,DW_OP_deref] --> [DW_OP_breg,Offset].<br>
if (*std::next(I) == dwarf::DW_OP_deref) {<br>
- unsigned Offset = I.getArg(1);<br>
+ unsigned Offset = (*I).getArg(1);<br>
ValidReg = AddMachineRegIndirect(MachineReg, Offset);<br>
std::advance(I, 2);<br>
break;<br>
@@ -248,14 +248,14 @@ void DwarfExpression::AddExpression(DIEx<br>
switch (*I) {<br>
case dwarf::DW_OP_piece: {<br>
unsigned SizeOfByte = 8;<br>
- unsigned OffsetInBits = I.getArg(1) * SizeOfByte;<br>
- unsigned SizeInBits = I.getArg(2) * SizeOfByte;<br>
+ unsigned OffsetInBits = (*I).getArg(1) * SizeOfByte;<br>
+ unsigned SizeInBits = (*I).getArg(2) * SizeOfByte;<br>
AddOpPiece(SizeInBits, getOffsetOrZero(OffsetInBits, PieceOffsetInBits));<br>
break;<br>
}<br>
case dwarf::DW_OP_plus:<br>
EmitOp(dwarf::DW_OP_plus_uconst);<br>
- EmitUnsigned(I.getArg(1));<br>
+ EmitUnsigned((*I).getArg(1));<br>
break;<br>
case dwarf::DW_OP_deref:<br>
EmitOp(dwarf::DW_OP_deref);<br>
<br>
Modified: llvm/trunk/lib/IR/DebugInfo.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=226939&r1=226938&r2=226939&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=226939&r1=226938&r2=226939&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/DebugInfo.cpp (original)<br>
+++ llvm/trunk/lib/IR/DebugInfo.cpp Fri Jan 23 15:24:41 2015<br>
@@ -1406,16 +1406,15 @@ void DIVariable::printInternal(raw_ostre<br>
}<br>
<br>
void DIExpression::printInternal(raw_ostream &OS) const {<br>
- for (auto E = end(), I = begin(); I != E; ++I) {<br>
- uint64_t OpCode = *I;<br>
- OS << " [" << OperationEncodingString(OpCode);<br>
- switch (OpCode) {<br>
+ for (auto Op : *this) {<br>
+ OS << " [" << OperationEncodingString(Op);<br>
+ switch (Op) {<br>
case DW_OP_plus: {<br>
- OS << " " << I.getArg(1);<br>
+ OS << " " << Op.getArg(1);<br>
break;<br>
}<br>
case DW_OP_piece: {<br>
- OS << " offset=" << I.getArg(1) << ", size=" << I.getArg(2);<br>
+ OS << " offset=" << Op.getArg(1) << ", size=" << Op.getArg(2);<br>
break;<br>
}<br>
case DW_OP_deref:<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>