Page MenuHomePhabricator

[DebugInfo] Add regression test for __fp16, float, and double constants.
Needs ReviewPublic

Authored by dgross on Dec 7 2016, 3:09 PM.

Details

Reviewers
llvm-commits
Summary

Verfies that D27549 (partial fix for PR26619) produces IR that llvm can
translate to the expected DWARF.

Prior to the change "DebugInfo: New metadata representation for global
variables." (D20147, D20415), clang emitted debug metadata for static
const values of floating-point types, but llvm did not translate this
debug metadata into DWARF. After that change, clang did not emit debug
metadata for static const values of floating-point types, but llvm is
capable of translating it into DWARF if it is emitted.

Event Timeline

dgross updated this revision to Diff 80675.Dec 7 2016, 3:09 PM
dgross retitled this revision from to [DebugInfo] Add regression test for __fp16, float, and double constants..
dgross updated this object.
dgross added a reviewer: llvm-commits.
probinson added inline comments.
test/DebugInfo/Generic/static-const-fp.ll
39

Generally we remove all the unnecessary attributes, rather than just take the raw -emit-llvm output.

79

To verify that the next attribute is part of the same DIE we usually do:

CHECK-NOT: {{DW_TAG|NULL}}

which shows it's not in a child and not in a DIE from a higher nesting level.

dgross updated this revision to Diff 80703.Dec 7 2016, 6:14 PM

Incorporate code review feedback.

  • Remove attributes.
  • Change pattern for detecting end-of-DIE.
dgross marked 2 inline comments as done.Dec 7 2016, 6:16 PM

Does this need the test coverage if it's just "hey, LLVM can splat these bytes out into DWARF even if we (independently) say the type of those bytes is a float"? The code that emits the bytes is presumably independent of the code that emits the type description, so testing the combination seems unnecessary to me.

aprantl added inline comments.
test/DebugInfo/Generic/static-const-fp.ll
4

Can you wrap this to 80 columns? clang-format can do this for you.

79

Alternatively, you can also use CHECK-NEXT, which is often more readable if the match is expected to be on the next line (but not as flexible).

Does this need the test coverage if it's just "hey, LLVM can splat these bytes out into DWARF even if we (independently) say the type of those bytes is a float"? The code that emits the bytes is presumably independent of the code that emits the type description, so testing the combination seems unnecessary to me.

Can you explain this further? What parts of this test case are you suggesting may be unnecessary?

dgross added inline comments.Dec 12 2016, 4:51 PM
test/DebugInfo/Generic/static-const-fp.ll
79

I prefer not to be as strict as requiring the match to be on the next line. (In fact, it would be better not to require name to precede const_value, but there may not be an easy way to relax that.)

dgross updated this revision to Diff 81163.Dec 12 2016, 4:54 PM

Per code review, wrap text paragraph to 80 columns.

dgross marked an inline comment as done.Dec 12 2016, 4:55 PM