[PATCH] D56587: Introduce DW_OP_LLVM_convert

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

There's a nonzero chance that this patch will break llvm/tools/dsymutil. I'll see if I can come up with a testcase for that (hopefully later today).



================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:1196
+void DwarfCompileUnit::createBaseTypeDIEs() {
+  // Insert the base type DIEs directly after the CU. Maintain order by
+  // iterating backwards and inserting to the front of CU child list.
----------------
... `so their offsets fit into the 5 bits reserved inside the location expressions.`


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1938
+    bool EmitByte = true;
+    // XXX: This is admittedly pretty stupid but sadly appears to be the
+    // easiest way to pass custom values as the expressions are inserted into a
----------------
You might want to reword this comment to be more assertive :-)

Is my understanding correct that we only need to do this here because the inline DW_AT_location (DW_FORM_block, ..) are emitted ahead of time and thus have the correct offsets injected from the get go?

Could you please move this out into a `fixupLocEntryDIEReferences()` (or something) function?


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1978
+      case dwarf::DW_OP_bregx:
+        NumSkipLEB128s = 2;
+        State = SkipLEB128;
----------------
Didn't your dwarfdump patch have this info in an enum or am I confusing things?


================
Comment at: lib/CodeGen/AsmPrinter/DwarfExpression.cpp:29
+  "generate-typed-dwarf5-expr", cl::Hidden,
+  cl::desc("Generate typed DWARF5 expressions regardless of DWARF version targeted."),
+  cl::init(false));
----------------
Why is this needed? Shouldn't we key off the debugger tuning flag?


================
Comment at: lib/CodeGen/AsmPrinter/DwarfExpression.cpp:402
+        // DwarfDebug::emitDebugLocEntry has been fitted with means to extract it
+        // later.
+        unsigned I = 0, E = CU.ExprRefedBaseTypes.size();
----------------
Can you add a comment that explains what happens in the non-location-list cases?


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

https://reviews.llvm.org/D56587