[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#1736266 <https://reviews.llvm.org/D69886#1736266>, @dblaikie wrote:

> Apologies for not having enough time to review this in more detail at the moment, so this feedback is shorting from the hip slightly based on reading the existing conversations and patch description
>
> Broadly:
>
> I'd expect more than 3 patches and in this order:
>
> 1. Backend/DWARF emission support for new debug info IR/metadata/intrinsic usage/DW_OP operators. This should include tests with hand-crafted IR (or generated from future patches - but I think this should be narrow enough that a small example could be given mostly with IR generated from clang/LLVM, and only a minor edit to use the new feature)


Agree. Fortunately the current patch (submitted first) contains support for DWARF emission. The current patch also has the testcases with IR generated from future patches.

> 2-N) Individual patches, one for each optimization that generates this new representation where possible/useful - each one of these patches should be independent and include tests for the change (tests that are IR->IR tests, testing only the specific optimization)

Agree. I am working on splitting this way. I plan to submit second patch (after first patch gets place in trunk) which will include enhanced salvaging debug info utilities to be used (called) in different optimizations. once this second patch goes thru. the remaining individual patches for each optimization will be posted for review. 
Please let me know if it sounds good. Thanks for your suggestions @dblaikie.

> (the 2-N patches can't come before (1) or the trunk would be broken by passes generating IR that the DWARF backend could not handle)

Agree. I will hold other patches till (1) gets approved.

> @probinson @aprantl - do you folks know why implicit_pointer needs to point to another DIE, rather than describing bytes directly? I guess it's intended to handle void*, for instance? (where the type information for the pointee isn't present in the pointer type) But for something like int* if the int isn't in memory (so can't be pointed to) - say, it's in a register, I'd expect to describe the value of the int without any extra DIE references, etc? (who knows if the DIE is in a variable/described by any other DIE). I'm pretty confused by the way this DWARF feature is spec'd & wondering if we should avoid implementing it/instead implement a better extension? But quite possible there's things about the design that I don't understand.




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

https://reviews.llvm.org/D69886