[PATCH] D111916: [DebugInfo] Fixup DIEs for inlined functions

[Ellis Hoag via Phabricator via llvm-commits]

ellis added a comment.

In D111916#3067928 <https://reviews.llvm.org/D111916#3067928>, @dblaikie wrote:

> So, currently we put the DIEs in the concrete definition and then when we didn't need a concrete one we end up with this weird uninitialized DIE with a few children, that we'd like to have those up in the abstract origin instead, right?

Yep

> And this change moves those DIEs up at the end, trying to filter the DIEs even if we did end up legitimately creating the concrete DIE (so we only move the DIEs that should reside in the abstract origin? (ie: we don't move up a local non-static variable from concrete to abstract))?

Yeah, even if the concrete DIE is legitimate we don't want it to hold the static variables since they wouldn't be visible to inlined subroutines. For non-static variables, those DIEs actually belong to the abstract origin, but the concrete DIE also has a DIE that references them via `DW_AT_abstract_origin`.

  // Concrete
  0x0000002a:   DW_TAG_subprogram
                  DW_AT_low_pc	(0x0000000000000000)
                  DW_AT_high_pc	(0x0000000000000011)
                  DW_AT_frame_base	(DW_OP_reg6 RBP)
                  DW_AT_abstract_origin	(0x00000046 "_Z7removedv")
  
  0x0000003d:     DW_TAG_variable
                    DW_AT_location	(DW_OP_fbreg -4)
                    DW_AT_abstract_origin	(0x00000056 "A")
  
  0x00000045:     NULL
  
  // Abstract origin
  0x00000046:   DW_TAG_subprogram
                  DW_AT_linkage_name	("_Z7removedv")
                  DW_AT_name	("removed")
                  DW_AT_decl_file	("foo.cpp")
                  DW_AT_decl_line	(2)
                  DW_AT_type	(0x00000062 "int")
                  DW_AT_external	(true)
                  DW_AT_inline	(DW_INL_inlined)
  
  0x00000056:     DW_TAG_variable
                    DW_AT_name	("A")
                    DW_AT_decl_file	("foo.cpp")
                    DW_AT_decl_line	(2)
                    DW_AT_type	(0x00000062 "int")

I mentioned this in one of the comments, but even with this patch we don't do this for static variables and I was wondering if we should.

> Hmm - what would happen if we moved them up earlier - like as soon as we create the concrete one properly/legitimately, we move anything that came before that up to the abstract origin?
>
> I guess that would break down if we hadn't created the abstract origin at that point (ie: would leave the static local down in the concrete definition instead of the abstract definition)?

Yep, at that point there is no abstract origin and we don't know if there will be one.

> Maybe we could always create the abstract origin, then at the end collapse it down into the concrete definition if it's unused/there are no inline instances? Less need to try to separate things out?

I'd have to think about this a bit more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111916