[PATCH] Fix the remainder of PR22762 (GDB is crashing on DW_OP_piece being used inside of DW_AT_frame_base)

[David Blaikie]

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1575
@@ -1574,3 +1574,3 @@
       // Regular entry.
-      AP.EmitDwarfRegOp(Streamer, Loc);
-    else {
+      bool ValidReg = Loc.isIndirect()
+        ? DwarfExpr.AddMachineRegIndirect(Loc.getReg(), Loc.getOffset())
----------------
This code looks repetitious (same as the code in addAddress) - should it be wrapped up somewhere?

================
Comment at: lib/CodeGen/AsmPrinter/DwarfExpression.cpp:81
@@ -93,1 +80,3 @@
+                                         unsigned PieceOffsetInBits,
+                                         bool MayUsePieceForSubRegMask) {
   if (!TRI.isPhysicalRegister(MachineReg))
----------------
Maybe name this flag "MayUsePiece" - I imagine that if GDB has trouble with piece for subregs, it'd have trouble with it for anything else too?

================
Comment at: lib/CodeGen/AsmPrinter/DwarfExpression.cpp:103
@@ +102,3 @@
+      uint64_t SubRegOffset = TRI.getSubRegIdxOffset(Idx);
+      // +--------------+------+-----------+
+      // | SubRegOffset | Size | UpperBits |
----------------
Not quite sure what this asciiart is meant to add?

================
Comment at: lib/CodeGen/AsmPrinter/DwarfExpression.cpp:116
@@ +115,3 @@
+      if (MayUsePieceForSubRegMask && SubRegOffset == 0) {
+        // Use the smaller DW_OP_piece instead of creating a mask for a
+        // subregister at offset 0.
----------------
I'd phrase this the other way - this comment should just describe the obvious case in the positive "use the shorter encoding if no shifting is required" (paraphrased - those exact words are a bit too brief)

================
Comment at: lib/CodeGen/AsmPrinter/DwarfExpression.cpp:122
@@ +121,3 @@
+      }
+      if (Size > PieceSizeInBits) {
+        // Mask out the uninteresting higher bits.
----------------
This looks like it should be an elseif - since the last line of the previous if block establishes a condition that can't possibly be true for this condition anyway, right?

================
Comment at: lib/CodeGen/AsmPrinter/DwarfUnit.cpp:550
@@ +549,3 @@
+  DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+  bool ValidReg = Location.isIndirect()
+    ? DwarfExpr.AddMachineRegIndirect(Location.getReg(), Location.getOffset())
----------------
Same repetition again

http://reviews.llvm.org/D8233

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/