dblaikie added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3842-3846 + if (EmitDwarf && + Args.hasFlag(options::OPT_fno_eliminate_unused_debug_types, + options::OPT_feliminate_unused_debug_types, false) && + DebugInfoKind >= codegenoptions::DebugInfoConstructor) + DebugInfoKind = codegenoptions::UnusedTypeInfo; ---------------- Would this be tidier if it were rolled into the if/checking around 380 since it's a very similar option? ================ Comment at: clang/test/CodeGen/debug-info-unused-types.cpp:24-30 +// NODBG-NOT: !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz" +// NODBG-NOT: !DIEnumerator(name: "BAZ" +// NODBG-NOT: !DICompositeType(tag: DW_TAG_enumeration_type, name: "z" +// NODBG-NOT: !DIEnumerator(name: "Z" +// NODBG-NOT: !DIDerivedType(tag: DW_TAG_typedef, name: "foo" +// NODBG-NOT: !DICompositeType(tag: DW_TAG_class_type, name: "bar" +// NODBG-NOT: !DICompositeType(tag: DW_TAG_class_type, name: "y" ---------------- Maybe simpler to test the NODBG by `NODBG-NOT: DI{{[a-zA-Z]*}}Type` ? Not sure if that'd quite work, but might be adequate. ================ Comment at: clang/test/CodeGen/debug-info-unused-types.cpp:15-21 +// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz" +// CHECK: !DIEnumerator(name: "BAZ" +// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "z" +// CHECK: !DIEnumerator(name: "Z" +// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "foo" +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "bar" +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "y" ---------------- nickdesaulniers wrote: > dblaikie wrote: > > nickdesaulniers wrote: > > > dblaikie wrote: > > > > Probably good to check these end up in the retained types list > > > What's the best way to check this? In particular, I worry about the > > > `!<number>` number changing in the future, resulting in this test > > > spuriously failing. For this test case, we have: > > > > > > ``` > > > !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, > > > producer: "Nick Desaulniers clang version 12.0.0 > > > (g...@github.com:llvm/llvm-project.git > > > e541558d11ca82b3664b907b350da5187f23c0c9)", isOptimized: false, > > > runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: > > > !15, splitDebugInlining: false, nameTableKind: None) > > > ``` > > > which tells us the `retainedTypes` is `!15`. `!15` looks like: > > > ``` > > > !15 = !{!16, !17, !3, !5, !18, !8} > > > ``` > > > In this case, should I just add the numbers? If we used regex patterns > > > `[0-9]`, I worry that we couldn't ensure the list would match. Is it ok > > > to just hardcode these? > > You can use captures in FileCheck to make references a little more > > flexible. Surprised there aren't /any/ tests in LLVM testing the > > retainedTypes list... test/CodeGen/debug-info-extern-call.c used to use the > > retained types list & still has some testing remnants of it you could draw > > from, something like:: > > ``` > > // CHECK: !DICompileUnit({{.*}}retainedTypes: [[RETTYPES:![0-9]+]] > > // CHECK: [[RETTYPES]] = !{[[T1:![0-9]+]], [[T2:![0-9]+]], ...} > > ... > > // CHECK: [[T1]] = !DICompositeType(... > > ``` > > > > It doesn't protect the test from changes in the order of the retained types > > list but otherwise resilient to extraneous metadata nodes being added or > > removed. > So one issue I'm running into with this approach is that the retained type > list is emitted in between the other debug nodes (some before it, some after > it). The capture logic seems to have an issue referring to captures that > have yet to be defined yet. > > note: uses undefined variable(s): "TYPE7" > > but if I move the `CHECK` for the retained types last, then `FileCheck` > errors because it seems it only doesn't one pass for the `CHECK` statements > (IIUC), so it's "too late" to parse. > > Is there another way to test this, or am I holding it wrong? Order dependence is a currently unavoidable part of FileCheck tests, so if the content you're trying to test (I'm not entirely clear from your description - so guessing a bit) looks something like: ``` !1 = DICompileUnit ... retainedTypes: !3 !2 = DICompositeType "x" !3 = !{!2, !4} !4 = DICompositeType "y" ``` Then you'd CHECK this something like: ``` ; CHECK: DICompileUnit {{.*}} retainedTypes: [[RET_TY:![0-9]*]] ; CHECK: [[X:![0-9]*]] = DICompositeType "x" ; CHECK: [[RET_TY]] = !{[[X]], [[Y:![0-9]*]]} ; CHECK: [[Y]] = DICompositeType "y" ``` Yes, this does mean that if the types were emitted in a different order, the test would fail - but that's a limitation we're living with for pretty much all LLVM and Clang testing at the moment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80242/new/ https://reviews.llvm.org/D80242 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits