This is an archive of the discontinued LLVM Phabricator instance.

[DebugInfo][test] Simplify/imporove a few tests using --impicit-check-not=DW_TAG. NFC
ClosedPublic

Authored by krisb on Nov 11 2021, 12:42 AM.

Details

Summary

This patch rewrites checks in a few debug info tests to avoid using
'CHECK-NOT: {{DW_TAG|NULL}}'. It proposes --impicit-check-not=DW_TAG
instead, as it makes the checks clearer, and easier to analyze and update.

Diff Detail

Event Timeline

krisb requested review of this revision.Nov 11 2021, 12:42 AM
krisb created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptNov 11 2021, 12:42 AM
krisb updated this revision to Diff 386525.Nov 11 2021, 8:27 AM

Include also llvm/test/DebugInfo/Generic/PR20038.ll.

dblaikie added inline comments.Nov 11 2021, 1:27 PM
llvm/test/DebugInfo/X86/align_c11.ll
27–33

Perhaps this test could avoid testing all these other DW_TAGs by using --name or some other filtering on the dump, so it only dumps the relevant DIEs?

llvm/test/DebugInfo/X86/align_cpp11.ll
81–84

Any chance some filtering could be done to avoid checking a bunch of other tags in this test?

krisb updated this revision to Diff 386750.Nov 12 2021, 12:12 AM
krisb marked an inline comment as done.

Make some tests to filter entities by name to reduce the number of auxiliary checks.

llvm/test/DebugInfo/X86/align_cpp11.ll
81–84

This test checks almost all entities from the dump, so filtering by name doesn't do much for it. Will update for comparing.

dblaikie accepted this revision.Nov 12 2021, 10:23 AM

Fair - I think the c11 test is better with the name query form. -your choice on the other one.

llvm/test/DebugInfo/X86/align_cpp11.ll
81–84

Fair enough - yeah, maybe in future tests we can intentionally name the relevant entities to make them easy to match by name, but for this one I agree it's probably more name queries than is especially helpful. It is nice that this makes the test less dependent on the structure of the rest of the DWARF (eg: if we change the order that basic types get emitted for whatever reason - the name-query based version of the test is resilient to that in a way that the check-all-tags form is not)

This revision is now accepted and ready to land.Nov 12 2021, 10:23 AM
krisb added a comment.Nov 13 2021, 7:37 AM

llvm/test/DebugInfo/X86/align_cpp11.ll with name filters looks a bit better to me, so I decided to keep it.
@dblaikie thank you for the review!