Page MenuHomePhabricator

[WebAssembly] Fixed byval args missing DWARF DW_AT_LOCATION
ClosedPublic

Authored by aardappel on Tue, Jan 5, 5:03 PM.

Details

Summary

A struct in C passed by value did not get debug information. Such values are currently
lowered to a Wasm local even in -O0 (not to an alloca like on other archs), which becomes
a Target Index operand (TI_LOCAL). The DWARF writing code was not emitting locations
in for TI's specifically if the location is a single range (not a list).

In addition, the ExplicitLocals pass which removes the ARGUMENT pseudo instructions did
not update the associated DBG_VALUEs, and couldn't even find these values since the code
assumed such instructions are adjacent, which is not the case here.

Also fixed asm printing of TIs needed by a test.

Diff Detail

Event Timeline

aardappel created this revision.Tue, Jan 5, 5:03 PM
aardappel requested review of this revision.Tue, Jan 5, 5:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Jan 5, 5:03 PM

@dblaikie Added you for review since this modifies common DWARF code, would love to hear if that is problematic or not. It would seem not, since we're the only users of these TI's it seems, and the new code is generic enough that it could work for future targets using TIs?

@dblaikie Added you for review since this modifies common DWARF code, would love to hear if that is problematic or not. It would seem not, since we're the only users of these TI's it seems, and the new code is generic enough that it could work for future targets using TIs?

Yeah, since you folks are the ones that are using this feature for now (I guess/can you confirm that the TI things were added specifically for WebAssembly?) it's pretty much up to you. For more nuanced review of debug info locations, @aprantl is probably a more useful reference point - the actual optimized debug info variable location tracking, etc, isn't something I've stayed particularly close to.

Mechanically this LGTM.

llvm/test/CodeGen/WebAssembly/dbgvalue.ll
4

it would be nicer if we piped the output to llvm-dwarfdump and CHECK that output rather than checking for a comment in the assembler output.

dschuff accepted this revision.Wed, Jan 6, 3:51 PM

@dblaikie Added you for review since this modifies common DWARF code, would love to hear if that is problematic or not. It would seem not, since we're the only users of these TI's it seems, and the new code is generic enough that it could work for future targets using TIs?

Yeah, since you folks are the ones that are using this feature for now (I guess/can you confirm that the TI things were added specifically for WebAssembly?) it's pretty much up to you. For more nuanced review of debug info locations, @aprantl is probably a more useful reference point - the actual optimized debug info variable location tracking, etc, isn't something I've stayed particularly close to.

I can confirm that the TI things were added for wasm. We tried to make them so that if another target used TargetIndex operands the same way, then it would just work (although of course TI operands are intended to be used however a target wants, so there's no guarantee).

This revision is now accepted and ready to land.Wed, Jan 6, 3:51 PM
aardappel added inline comments.Thu, Jan 7, 10:30 AM
llvm/test/CodeGen/WebAssembly/dbgvalue.ll
4

Agreed dwarfdump is the more exact way to do this. Here I am fixing an existing test though, and we have plenty of dwarfdump based tests already.