[PATCH] D31439: PR32382: Emit complex DWARF expressions with the correct location description kind

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

There are a lot of small, careful changes in here - I might need a bit of a high level walkthrough of what the changes are and how they achieve the goal. More detail on how this changes behavior would be good. "makes LLVM aware of these DWARF restrictions" is a bit too high level.



================
Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1536-1540
+        for (I = 0; I < N - 1; ++I) {
+          Elts[I] = Elts[I + 1];
+          if (N > 3 && I == N - 4 && Elts[I + 1] == dwarf::DW_OP_LLVM_fragment)
+            break;
+        }
----------------
This seems a bit complicated - any way to simplify it/make it more obvious? (or at least add comments)

Some of it looks the same as the condition in the case 0 of this switch. Could it be factored into a common function wiht a name that might aid readability?


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:1759-1760
   Record.reserve(N->getElements().size() + 1);
-
-  const uint64_t HasOpFragmentFlag = 1 << 1;
-  Record.push_back((uint64_t)N->isDistinct() | HasOpFragmentFlag);
+  const uint64_t Version = 2 << 1;
+  Record.push_back((uint64_t)N->isDistinct() | Version);
   Record.append(N->elements_begin(), N->elements_end());
----------------
Where did the HasOpFragmentFlag go? Or it morphed into a version flag stored above the first bit?


================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:177
+      // to emit a memory location description.
+      Ops.push_back(dwarf::DW_OP_deref);
       const MCSymbol *Sym = Asm->getSymbol(Global);
----------------
So is this another "not quite DWARF expression" contract it DIExpression? (like the LLVM_fragment?)


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1517
     if (Location.getOffset()) {
+    //if (Location.isIndirect()) {
       Ops.push_back(dwarf::DW_OP_plus);
----------------
Remove commented-out code.


https://reviews.llvm.org/D31439