[PATCH] D68620: DebugInfo: Use base address selection entries for debug_loc

[David Blaikie via Phabricator via llvm-commits]

dblaikie marked an inline comment as done.
dblaikie added a comment.

In D68620#1699420 <https://reviews.llvm.org/D68620#1699420>, @labath wrote:

> LLDB seems to have support for base address selection in v4 debug_loc. It does not have support for v5 LLE_base_address(x) stuff, but the whole of v5 location list support is kind of wonky, which also is why I am looking at getting it to use the llvm version of the parser.


Yeah, that sort of summarizes GDB's support too.

> As for llvm-dwarfdump, feel free to add new encodings there. My plan is to add support for all LLE encodings, but since I also need to figure out a way to refactor all of that stuff, it may take a while before I get to that. Having one or two new encodings appear in the mean time should only be a minor nuisance.

Had to take a few goes at this to see if there was a good mid-point of refactoring & think I found one that coalesces some of the codepaths for verbose, non-verbose, and inline dumping - insofar as seemed reasonable, I tried to make things more similar to debug_rnglists (in several cases just at least making the code look similar, even though it's not shared yet).



================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2328
+        BaseIsSet = true;
+        if (UseDwarf5) {
+          Asm->OutStreamer->AddComment(StringifyEnum(BaseAddressx));
----------------
probinson wrote:
> Would it be more readable this way?
> ```
> if (!UseDwarf5) {
>   Base = NewBase;
>   BaseIsSet = true;
>   Asm-OutStreamer->EmitIntValue(-1, Size);
>   // etc
> } else if (NewBase != Begin || P.second.size() > 1) {
>   Base = NewBase;
>   BaseIsSet = true;
>   Asm->OutStreamer->AddComment(StringifyEnum(BaseAddressx);
>   // etc
> }
> ```
> As there are only 2 lines in common.  (My eye caught `if (!UseDwarf5` and two lines later `if (UseDwarf5)` and did a double-take.)
Sure, looks good to me! I know this whole function's got several cases to think about & is a bit unwieldy.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68620