[PATCH] D94347: [WebAssembly] locals can now be indirect in DWARF

[Derek Schuff via Phabricator via llvm-commits]

dschuff added a comment.

This approach definitely makes for simple implementation. It does seem a little unfortunate (at the dwarf level) that we are inventing something target-specific for a case that could be covered by something generic.
Is it possible just to modify DwarfExpression::addWasmLocation() to slip an extra DW_OP_deref into the expression when the TI kind is indirect? or does that have to be done elsewhere, e.g. in the caller?

In D94347#2488099 <https://reviews.llvm.org/D94347#2488099>, @RReverser wrote:

> Actually, instead of using DW_AT_location, I think the better approach here would be to add `DW_AT_calling_convention` to the struct entry itself. Normally DWARF relies on consumers knowing the ABI of any given type, but that attribute is explicitly designed to indicate whether a composite type is passed by reference or by value.

That could maybe work. One potential downside is that it doesn't really match up with how LLVM decides to pass the structure. The calling_convention and other struct attributes are generated by the frontend but it's actually the use of 'byval' in the IR that makes the backend pass it indirectly. Although thinking about that a little more, maybe that doesn't really matter. Another frontend would have to generate its own type descriptors and also independently decide whether it wanted to use byval or not.

Other things to consider on that:

1. does lldb or the devtools debugger already use the calling_convention description on types, or would this require more work there? (relatedly, does clang generate this attribute already?)
2. we still intend to try a new ABI that would pass some structs by value in register(s). In this case the frontend would presumably just not use 'byval' for those, but it would also need to generate composite debug info. so it's not obvious right now (to me) whether that would be easier or harder with either option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94347