This is an archive of the discontinued LLVM Phabricator instance.

Issue local statics in correct DWARF lexical scope
Needs ReviewPublic

Authored by FlameTop on Jan 22 2018, 6:43 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Currently all static vars local to a function are issued in the main scope of the function. This change ensures that statics local to a lexical block are issued in the DWARF in the correct lexical block.

For example:

void foo()
{

static int local = 1234;
{
  static  int local = 5678;
}

}

This change detects debug info meta data assigned to a lexical block and defers its issuing to DWARF until we are in the correct lexical block.

There is a matching change to clang to ensure the debug info meta-data is assigned to the lexical block.

A matching test is to be added to the debuginfo-tests repository.

Diff Detail

Event Timeline

FlameTop created this revision.Jan 22 2018, 6:43 AM

+llvm-commits
See also the Clang change in D42370 and a debuginfo-tests test in D42371.

I'm not seeing where the add/getLocalLexicalDIEs APIs are being used.

include/llvm/IR/DebugInfoMetadata.h
477

Fix typos separately, rather than mixing them in with a functional change.

vsk added a subscriber: vsk.Jan 22 2018, 11:57 AM
vsk added inline comments.
lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
88

Can we allocate fewer DIE pointers inline? The first time a DenseMap grows, it allocates space for 64 entries, so this might get big a bit too quick.

142

Just return an Optional<LocalLexicalDIEs>? As written, I think this creates a distinct 'LocalLexicalDIEs Empty' object in each translation unit that includes the header.

FlameTop updated this revision to Diff 131049.Jan 23 2018, 5:19 AM

Apologies, I had a lot of trouble getting svn setup on my machine. Seems I lost some changes in the original diff. Updated for code to issues the lexical DIEs.

I'm not seeing where the add/getLocalLexicalDIEs APIs are being used.

Sorry Paul. My svn setup somehow lost some changes. Diff has been updated.

Did you see David's emailed comment?

lib/CodeGen/AsmPrinter/DwarfUnit.cpp
347

Looks like createAndAddDIE could become simply
return Parent.addChild(createDIE(Tag, N));
now or as an NFC follow-up, to reduce duplication.

Herald added a project: Restricted Project. · View Herald TranscriptFeb 15 2023, 9:12 AM