This is an archive of the discontinued LLVM Phabricator instance.

[codeview] Describe int local variables using .cv_def_range
ClosedPublic

Authored by rnk on Feb 5 2016, 11:59 AM.

Details

Summary

Refactor common value, scope, and label tracking logic out of DwarfDebug
into a common base class called DebugHandlerBase.

Update an old LLVM IR test case to avoid an assertion in LexicalScopes.

Diff Detail

Event Timeline

rnk updated this revision to Diff 47035.Feb 5 2016, 11:59 AM
rnk retitled this revision from to [codeview] Describe int local variables using .cv_def_range.
rnk updated this object.
rnk added reviewers: dblaikie, majnemer.
rnk added a subscriber: llvm-commits.
majnemer added inline comments.Feb 5 2016, 12:56 PM
lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
23–25

Sort these.

30–43

Any reason why this had to move?

rnk updated this revision to Diff 47049.Feb 5 2016, 2:29 PM
  • Tweak data structures to satisfy MSVC
rnk added a reviewer: echristo.Feb 8 2016, 2:50 PM
rnk added a comment.Feb 8 2016, 2:52 PM

Eric wanted to be CC'd, and David asked "why inheritance not composition". I could potentially see making a separate class that is specifically responsible for inserting labels before and after dbg value ranges, but I suspect I'll want to factor more stuff out of DwarfDebug and into here as time goes on. In particular, findPrologueEnd is currently duplicated. Using a base class also allows me to make most of these members protected, making it clear that they aren't intended for use by any other clients.

dblaikie edited edge metadata.Feb 9 2016, 2:34 PM

I must be missing something - how does the begin/endFunction functionality that you moved into teh base class get fired for DwarfDebug? It looks like there should be a call to Base::beginFunction in DwarfDebug::beginFunction where the code was moved from?

lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
569

Some comments here might not go astray (what is the format of these records you're emitting (maybe a broad comment at the top describing the format overall, then details on each field))

Specifically I don't know what the 4 here and 2, below (flags), are.

majnemer added inline comments.Feb 9 2016, 2:37 PM
lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
569

They (4 and 2) are the size of the value in bytes. Otherwise, EmitIntValue wouldn't know how much data to emit.

rnk added inline comments.Feb 9 2016, 3:53 PM
lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
569

The on-disk format is described as structs in llvm/DebugInfo/CodeView/SymbolRecord.h and TypeRecord.h. All the information that we know about any of these fields should probably live there, not here. I guess I can say "see SymbolRecord.h".

lib/CodeGen/AsmPrinter/DwarfDebug.cpp
1059

I did call beginFunction() right at the point where I scooped out the shared code. :)

The dwarf CU id emission stuff doesn't need to be ordered with respect to the dbg value history calculation.

dblaikie accepted this revision.Feb 9 2016, 4:01 PM
dblaikie edited edge metadata.

Interaction with the existing DWARF stuff looks fine to me

This revision is now accepted and ready to land.Feb 9 2016, 4:01 PM
majnemer added inline comments.Feb 9 2016, 4:25 PM
lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
569

We could do sizeof(uint32_t) and sizeof(Flags).

This revision was automatically updated to reflect the committed changes.