[PATCH] D72938: Fix an assertion failure in DwarfExpression's subregister composition

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:154
     // covered by this subregister.
     SmallBitVector CurSubReg(RegSize, false);
     CurSubReg.set(Offset, Offset + Size);
----------------
Is CurSubReg the intersection between the bits already emitted & the bits covered by the subreg, or just the bits covered by this subreg?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:163
+        DwarfRegs.push_back(Register::createSubRegister(
+            -1, Offset - CurPos, "no DWARF register encoding"));
+      if (Offset == 0 && Size == MaxSize)
----------------
So, here: `DwarfRegs.push_back(Register::createSubRegister(-1, Offset - CurPos, "no DWARF register encoding")`

Are we saying: bits [0, Offset) of the value aren't going to be described?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:165
+      if (Offset == 0 && Size == MaxSize)
+        DwarfRegs.push_back(Register::createRegister(Reg, "sub-register"));
+      else
----------------
This (`DwarfRegs.push_back(Register::createRegister(Reg, "sub-register")`) is the only functional change, right? IIUC this means that we're describing bits [0, MaxSize) of the value. Can we break out of the MCSubRegIterator loop here?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:113
+
+    /// Create a full register, not extra DW_OP_piece operators necessary.
+    static Register createRegister(int RegNo, const char *Comment) {
----------------
s/not/no/


================
Comment at: llvm/test/DebugInfo/MIR/ARM/subregister-full-piece.mir:39
+    t2Bcc %bb.2.for.body, 0, killed $cpsr, debug-location !260
+    DBG_VALUE $q8, 0, !139, !DIExpression(DW_OP_plus_uconst, 208, DW_OP_LLVM_fragment, 0, 64), debug-location !260
+    tB %bb.2.for.body, 14, $noreg
----------------
Why is the DW_OP_plus_uconst needed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72938/new/

https://reviews.llvm.org/D72938