This is an archive of the discontinued LLVM Phabricator instance.

[codeview] Translate bitpiece metadata to DEFRANGE_SUBFIELD* records
ClosedPublic

Authored by rnk on Oct 4 2016, 2:26 PM.

Details

Summary

This allows LLVM to describe locations of aggregate variables that have
been split by SROA.

Fixes PR29141

Diff Detail

Repository
rL LLVM

Event Timeline

rnk updated this revision to Diff 73555.Oct 4 2016, 2:26 PM
rnk retitled this revision from to [codeview] Translate bitpiece metadata to DEFRANGE_SUBFIELD* records.
rnk updated this object.
rnk added reviewers: amccarth, majnemer.
rnk added a subscriber: llvm-commits.
amccarth edited edge metadata.Oct 4 2016, 3:04 PM

Looks fine overall. I just have a couple inline questions.

include/llvm/DebugInfo/CodeView/SymbolRecord.h
813 ↗(On Diff #73555)

Is the mask correct? Three nybbles kinda looks like a typo, and it's used on the StructOffset which is 15 bits, so I expected 0x7FFF. If the mask is correct, then maybe 0x0FFF would be clearer.

test/DebugInfo/COFF/pieces.ll
2 ↗(On Diff #73555)

Is it redundant to check both the ASM and OBJ output? Your change affects the ASM directly. The OBJ check seems like it's covering ground that should already be handled in the llvm-readobj tests. Am I missing something?

majnemer added inline comments.Oct 4 2016, 3:42 PM
test/DebugInfo/COFF/pieces.ll
168 ↗(On Diff #73555)

What is wrong?

rnk added inline comments.Oct 4 2016, 4:14 PM
include/llvm/DebugInfo/CodeView/SymbolRecord.h
813 ↗(On Diff #73555)
This comment has been deleted.
813 ↗(On Diff #73555)

Uh, actually I screwed it up. It's 12 bits, but I got them backwards. :(

Apparently I didn't exercise this case. I need to come up with a test case that spills a piece of sliced aggregate.

test/DebugInfo/COFF/pieces.ll
2 ↗(On Diff #73555)

I'm trying to test that the assembly that we emit forms valid codeview records that llvm-readobj can understand. Basically, I'm worried that .cv_def_range .Ltmp7 .Lfunc_end0, "C\021\030\000\000\000\000\000\000\000" might not actually result in a well-formed record, and the object file test gives me confidence in it.

168 ↗(On Diff #73555)

We aren't turning #DEBUG_VALUE: nested:o <- [%RCX+0] into something good. It's not really SROA / subfield related, though, since o is passed indirectly as a pointer in RCX.

rnk updated this revision to Diff 73690.Oct 5 2016, 1:36 PM
rnk edited edge metadata.
  • Fix and test spilled pieces of aggregates
rnk added a comment.Oct 5 2016, 2:20 PM

I think Adrian's out today, so I'll go ahead and land this now so it can bake a few hours before I leave work.

This revision was automatically updated to reflect the committed changes.