[PATCH] D69886: [DebugInfo] Support for DW_OP_implicit_pointer (Post IR transformation phase)

[Alok Kumar Sharma via Phabricator via llvm-commits]

alok added a comment.

In D69886#1735632 <https://reviews.llvm.org/D69886#1735632>, @jmorse wrote:

> Hi Alok,
>
> Thanks for splitting up the patch and tests; I'm not very familiar with implicit pointers and I'm still working it through my mind, so I have a couple of questions if you'll bear with me. At the moment though, it looks like implicit-pointer dbg.value insts refer directly to DILocalVariables -- this will become a problem if the same function is inlined twice, as there's nothing to distinguish which inlining instance the dbg.value refers to. You can probably make a metadata pair of {DILocalVariable, DILocation} where the DILocation is the inlining location, and use that as the operand of the dbg.value instead. There should also be a test for multiple inlining instances too.


Thanks a lot for taking your time to review this, let me try to answer this, actually the functionality of DW_OP_implicit_pointer is to preserve the dereferenced values of optimized out variables, it doesnt preserve the location information. Our interest is to emit first operand which is offset of target variable within debuginfo dwarf section. We preserve DILocalVariable (temporarily) until debug info section is formed and variable's offset within it is known. Please let me know in case I misunderstood the question.

Sure, I shall add a test case.

> Looking at the tests, am I right in thinking that the "target" of the implicit pointer can change throughout the function? For example
> 
>   int a = 0, b = 1, c = 2;
>   int *foo = &a;
>   call();
>   foo = &b;
>   call();
>   foo = &c;
>    

Yes, it is possible.

> Is it (theoretically) possible to describe promoted non-local variables with implicit pointers? LLVM will occasionally promote an in-memory value into a register, after which there's no way for current debuginfo to describe where it is.

If a variable is a pointer and is eligible to have DW_AT_location attribute (in dwarf), then theoretically it should be possible to describe this attribute using DW_OP_implicit_pointer. Current patch covers for local variables only, we can investigate for non local variables later

> The code looks like a good start, although I don't know anything about the DWARF backend.

Thanks again for your comments.



================
Comment at: llvm/test/DebugInfo/dwarfdump-implicit_pointer_instcomb.ll:1
+; RUN: clang %s -O2 -gdwarf-5 -o %t.o
+; RUN: llvm-dwarfdump  %t.o | FileCheck %s
----------------
jmorse wrote:
> I don't believe running clang in an internal LLVM test is supported, this should probably be running llc -O2 -filetype=obj instead? Same with the other tests.
Thanks for pointing it out. I shall incorporate this comment.


================
Comment at: llvm/test/DebugInfo/dwarfdump-implicit_pointer_instcomb.ll:46
+
+attributes #0 = { nofree norecurse nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
----------------
jmorse wrote:
> Assuming these attributes are un-necessary, they should be dropped as they can be an extra maintenance burden.
Thanks. I shall incorporate this comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69886