Page MenuHomePhabricator

[DebugInfo] Support for DW_OP_implicit_pointer (DW_OP_LLVM_implicit_pointer)
Needs ReviewPublic

Authored by alok on Nov 24 2019, 9:45 AM.

Details

Summary

This patch (2/N) stems from D69787
It is suggested by @aprantl

Summary:

New dwarf operator DW_OP_LLVM_implicit_pointer is introduced (present only in LLVM IR)
This is needed ecause representation and specification (types of operands) of it a bit different that actual dwarf expression DW_OP_LLVM_implicit_pointer. while creating actual dwarf info it will be converted to DW_OP_LLVM_implicit_pointer.

Testing:
  • llvm-as, llvm-dis
  • check-llvm
  • check-debuginfo (the debug info integration tests)

Diff Detail

Event Timeline

alok created this revision.Nov 24 2019, 9:45 AM
jmorse added inline comments.Dec 3 2019, 7:48 AM
llvm/docs/LangRef.rst
4828–4829

IMHO, "variable represented by first operand" should become something like "the DILocalVariable referred to by the instructions value/address operand". This would make it clear that we're referring to a metadata node, and that the "first operand" is of the containing debug intrinsic, not a dwarf operators operand.

llvm/lib/IR/DebugInfoMetadata.cpp
909

Shouldn't this also check that the expression has the offset argument, and (as I understand it) no other elements?

alok marked 3 inline comments as done.Dec 3 2019, 9:40 PM
alok added inline comments.
llvm/docs/LangRef.rst
4828–4829

Thanks for your comment. It will be incorporated in next version.

alok updated this revision to Diff 232035.Dec 3 2019, 9:41 PM
alok marked an inline comment as done.

Update includes comments from @jmorse.

Can you please add an assembler round-trip test to test/IR that runs llvm-as | llvm-dis | llvm-as - | llvm-dis ? There are a bunch of examples in that directory.

alok added a comment.Mon, Jan 6, 8:43 PM

Can you please add an assembler round-trip test to test/IR that runs llvm-as | llvm-dis | llvm-as - | llvm-dis ? There are a bunch of examples in that directory.

Thanks @aprantl for your suggestion. Next version would have a testcase. Though the actual test case would need some changes from future patches, I would work-around that to just test the round-trip. Test case would be updated in future patch as needed.

alok updated this revision to Diff 236500.Mon, Jan 6, 8:49 PM
alok edited the summary of this revision. (Show Details)

Updated to re-base and include a test case as suggested by @aprantl.