This is an archive of the discontinued LLVM Phabricator instance.

[Debuginfo][SROA] (7/7) Support for DW_OP_implicit_pointer for named and unnamed variables (second strategy).
Needs ReviewPublic

Authored by alok on Jul 18 2020, 5:14 PM.

Details

Summary
This patch stems from D84112.
This patch includes changes for SROA optimization.

Diff Detail

Event Timeline

alok created this revision.Jul 18 2020, 5:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2020, 5:14 PM
jmorse added a subscriber: jmorse.Jul 30 2020, 6:09 AM

I see what seem to be changes to codegen here (see inline comments), is that intentional? And if so, why is that needed for DW_OP_LLVM_explicit_pointer to work?

llvm/lib/IR/Metadata.cpp
395

Why is this now permitted behaviour?

llvm/lib/Transforms/Scalar/EarlyCSE.cpp
1648–1654

Is this necessary for the implicit pointer work?

llvm/lib/Transforms/Scalar/SROA.cpp
4419–4503

Again, this block of code will need comments about its purpose adding.

4449–4453

This part looks like it's changing the code generated, replacing a GEP with an alloca. If that's the case, IMO this should be split out into a different patch (one for codegen, one for debuginfo), as any codegen change will need an independent argument as to why it's a good idea.

Plus, this would only be a codegen change for people building with dwarf-5, which would mean you get different code with and without the -g option to clang.

llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
424 ↗(On Diff #279045)

This should be folded into the previous patch (7?)

alok updated this revision to Diff 304755.Nov 12 2020, 1:58 AM
alok retitled this revision from [Debuginfo] (8/N) Support for DW_OP_implicit_pointer for named and unnamed variables (second strategy). to [Debuginfo][SROA] (7/7) Support for DW_OP_implicit_pointer for named and unnamed variables (second strategy)..

Updated to re-base.

phosek added a subscriber: phosek.Mar 10 2022, 3:29 PM

This change was brought up in https://github.com/llvm/llvm-project/issues/54290, @alok is this still active? Is there a way we can help to get this patch stack reviewed and landed?

Herald added a project: Restricted Project. · View Herald TranscriptMar 10 2022, 3:29 PM

This change was brought up in https://github.com/llvm/llvm-project/issues/54290, @alok is this still active? Is there a way we can help to get this patch stack reviewed and landed?

Looks like there was some further discussion here: https://reviews.llvm.org/D84112 and an alluded to llvm-dev thread that might be worth finding.

Also, I really wonder if it might be worth proposing a more flexible DWARF form/operator for this encoding. Currently the implicit_pointer only allows describing the pointer as pointing to another variable described by a variable DIE - I think it'd be nice to be able to avoid that indirection and creating synthetic variables to describe this in its full generality - and instead being able to describe the location (& maybe you need to point to a type too, but not a whole variable) directly in this variable location, maybe. It'd more closely model how I was leaning towards/pushing for this to be implemented in LLVM (LLVM_explicit_pointer was the prototype name) & I think makes more sense in general (in many cases you wouldn't be pointing to a known/named variable anyway - you could be pointing to a value in the calling frame that could be a composite itself, maybe several levels of indirection have been removed, etc)

This change was brought up in https://github.com/llvm/llvm-project/issues/54290, @alok is this still active? Is there a way we can help to get this patch stack reviewed and landed?

Looks like there was some further discussion here: https://reviews.llvm.org/D84112 and an alluded to llvm-dev thread that might be worth finding.

I believe the corresponding discussion is https://discourse.llvm.org/t/dw-op-implicit-pointer-design-implementation-in-general/53698

This change was brought up in https://github.com/llvm/llvm-project/issues/54290, @alok is this still active? Is there a way we can help to get this patch stack reviewed and landed?

Looks like there was some further discussion here: https://reviews.llvm.org/D84112 and an alluded to llvm-dev thread that might be worth finding.

I believe the corresponding discussion is https://discourse.llvm.org/t/dw-op-implicit-pointer-design-implementation-in-general/53698

It seems like not all the content was transferred over to Discourse, the original thread is https://lists.llvm.org/pipermail/llvm-dev/2019-November/136798.html

alok added a comment.Mar 28 2022, 9:47 PM

This change was brought up in https://github.com/llvm/llvm-project/issues/54290, @alok is this still active? Is there a way we can help to get this patch stack reviewed and landed?

Looks like there was some further discussion here: https://reviews.llvm.org/D84112 and an alluded to llvm-dev thread that might be worth finding.

I believe the corresponding discussion is https://discourse.llvm.org/t/dw-op-implicit-pointer-design-implementation-in-general/53698

It seems like not all the content was transferred over to Discourse, the original thread is https://lists.llvm.org/pipermail/llvm-dev/2019-November/136798.html

Thanks for your interest. I hope to re-activate it soon.