This is an archive of the discontinued LLVM Phabricator instance.

DebugInfo: make DW_TAG_atomic_type valid
ClosedPublic

Authored by vleschuk on Oct 31 2016, 5:31 AM.

Details

Summary

DW_TAG_atomic_type was already included in Dwarf.defs and emitted correctly, however Verifier didn't recognize it as valid. Thus we introduce the following changes:

  • Make DW_TAG_atomic_type valid tag for IR and DWARF (enabled only with -gdwarf-5)
  • Add it to related docs
  • Add DebugInfo test

Diff Detail

Repository
rL LLVM

Event Timeline

vleschuk updated this revision to Diff 76388.Oct 31 2016, 5:31 AM
vleschuk retitled this revision from to DebugInfo: make DW_TAG_atomic_type valid.
vleschuk updated this object.
vleschuk added a subscriber: llvm-commits.
aprantl added a reviewer: rnk.Oct 31 2016, 8:43 AM
aprantl added inline comments.Oct 31 2016, 8:56 AM
include/llvm/DebugInfo/CodeView/CodeView.h
284 ↗(On Diff #76388)

Can someone familiar with the CodeView code check this?

test/DebugInfo/X86/atomic_c11.ll
11 ↗(On Diff #76388)

Please check for the entire type tree: otherwise output like (hypothetical example)

error: unknown token: DW_TAG_atomic_type

would match, too.

16 ↗(On Diff #76388)

Could you include a comment with the source this was generated from?

30 ↗(On Diff #76388)

This seems unnecessary for the test?

vleschuk marked an inline comment as done.Oct 31 2016, 8:59 AM
vleschuk added inline comments.
test/DebugInfo/X86/atomic_c11.ll
11 ↗(On Diff #76388)

WIll do.

16 ↗(On Diff #76388)

It's included, see lines 4-7:

; Generated by clang -c -g -std=c11 -S -emit-llvm from the following C11 source
;
; _Atomic const int i;
;
vleschuk marked 2 inline comments as done.Oct 31 2016, 9:00 AM
aprantl edited edge metadata.Oct 31 2016, 9:01 AM

Should we restrict output of DW_TAG_atomic_type to -gdwarf-5 only?
In contrast to an attribute, a consumer can't really ignore the tag if it doesn't understand it.

rnk added inline comments.Oct 31 2016, 9:05 AM
lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
1412 ↗(On Diff #76388)

Let's ignore _Atomic qualifiers for now. I emailed Dave Bartolomeo about reserving that bit, and eventually getting MS debuggers to print the _Atomic qualifier, but until we hear back, I don't want to start emitting it. Leave a FIXME to follow up on it.

You would also want to update lib/DebugInfo/CodeView/EnumTables.cpp to dump this properly, and add a basic test to test/DebugInfo/COFF/ similar to the one you have.

vleschuk updated this revision to Diff 76439.Oct 31 2016, 10:09 AM
vleschuk updated this object.
vleschuk edited edge metadata.
  • Reverted CodeView-related changes, replaced with TODO comments instead
  • Made DW_TAG_atomic_type available only with DwarfVersion >= 5 (otherwise do not emit it)
  • Updated test to match all type tree (also changed dwarf version in test IR to 5)
vleschuk marked 3 inline comments as done.Oct 31 2016, 10:09 AM
aprantl accepted this revision.Oct 31 2016, 11:05 AM
aprantl edited edge metadata.

Thanks, LGTM with inline feedback addressed.

test/DebugInfo/X86/atomic_c11.ll
35 ↗(On Diff #76439)

We now also need a second copy of this testcase with "Dwarf Version"=4 that checks that the we see through the atomic type on older versions.

This revision is now accepted and ready to land.Oct 31 2016, 11:05 AM
This revision was automatically updated to reflect the committed changes.