This is an archive of the discontinued LLVM Phabricator instance.

[DWARF] Put linkage-name on abstract origin even when there's a declaration
ClosedPublic

Authored by probinson on Dec 1 2016, 1:41 PM.

Details

Summary

In r266692, we made it possible to emit linkage-names only for inlined subroutines, putting the attribute
on the abstract origin DIE (which counts as the definition DIE). However, this fails when we emit a
declaration DIE prior to the definition DIE, because (a) when we process the declaration, we don't emit
the linkage name, because it's not an abstract origin; and (b) when we process the definition, we look at
the declaration metadata and see it has a linkage name, so we think it has already been emitted.

Look at the actual DIE, rather than the metadata, to see whether we've already emitted the linkage name.

Diff Detail

Repository
rL LLVM

Event Timeline

probinson updated this revision to Diff 79977.Dec 1 2016, 1:41 PM
probinson retitled this revision from to [DWARF] Put linkage-name on abstract origin even when there's a declaration.
probinson updated this object.
probinson added reviewers: dblaikie, echristo, aprantl.
probinson added a subscriber: llvm-commits.
aprantl added inline comments.Dec 1 2016, 1:43 PM
test/DebugInfo/Generic/linkage-name-abstract.ll
118 ↗(On Diff #79977)

We typically strip out all non-essential attributes.

dblaikie edited edge metadata.Dec 1 2016, 1:44 PM

If possible it'd be nice to avoid having to search through the DIE's attributes to make this determination - any way we can make the right determination more directly/without that information channel?

probinson added inline comments.Dec 1 2016, 1:44 PM
lib/CodeGen/AsmPrinter/DwarfUnit.cpp
1162 ↗(On Diff #79977)

I had thought about making this

if (DD->useAllLinkageNames())
  DeclLinkageName = SPDecl->getLinkageName();

and so not needing the extra static function, but actually looking into the DIE itself seemed more robust.

If possible it'd be nice to avoid having to search through the DIE's attributes to make this determination - any way we can make the right determination more directly/without that information channel?

See the inline comment. It's depending on the way the condition works for actually emitting the linkage name, but as it's all inside the same method I suppose it's okay.

probinson updated this revision to Diff 79979.Dec 1 2016, 2:03 PM
probinson edited edge metadata.

Simpler cheaper check. Remove noisy attributes from test.

probinson marked 2 inline comments as done.Dec 1 2016, 2:04 PM
dblaikie accepted this revision.Dec 1 2016, 2:39 PM
dblaikie edited edge metadata.

Ah, I get it - sorry.

A comment's probably a good idea.

Also - if you make the member function static it might simplify the test case a fair bit.

This revision is now accepted and ready to land.Dec 1 2016, 2:39 PM

Ah, I get it - sorry.

A comment's probably a good idea.

Also - if you make the member function static it might simplify the test case a fair bit.

Added a comment. Making the member function static did tighten up the IR and metadata a bit.
Thanks!

This revision was automatically updated to reflect the committed changes.