[PATCH] D73283: Handle complex DWARF expressions in combination with "complex" registers

[David Stenberg via Phabricator via llvm-commits]

dstenb added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:242
+  // is not possible to apply any DWARF operation to the combined
+  // DW_OP_pieces. By manualy shifting the subregisters into place,
+  // this can be worked around at the expense of a slightly larger
----------------
Nit: s/manualy/manually/


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:250
+    // masking is necessary even if the subregisters are overlapping,
+    // since any overlapping bits in are expected be identical.
+    const TargetRegisterClass *RC = TRI.getMinimalPhysRegClass(MachineReg);
----------------
Nit: s/in are/in the registers are/


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:254
+    bool Large = RegSize > CU.getAddressSize();
+    bool First = true;
+    for (auto &Reg : DwarfRegs) {
----------------
Shouldn't there be a version check so that we don't emit `DW_OP_regval_type` when targeting earlier DWARF standards?

```
if (Large && DwarfVersion < 5)
  return false;
```

(Should we perhaps consider using the GNU extension, DW_OP_GNU_regval_type, for earlier versions?)


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:407
 
+unsigned DwarfExpression::getOrCreateBaseType(unsigned BitSize,
+                                              dwarf::TypeKind Encoding) {
----------------
Can this change be moved to a preceding NFC commit to reduce this patch's size?


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

https://reviews.llvm.org/D73283