[llvm-dwarfdump][test] Add a --statistics test for a DW_AT_artificial variable

[MaskRay]

Summary

There is an untested case.

[dblaikie]

llvm/test/tools/llvm-dwarfdump/X86/statistics.ll

26–29

Does this function need a return value or would a void-returning member function suffice?

[MaskRay]

llvm/test/tools/llvm-dwarfdump/X86/statistics.ll

26–29

void T::address() { (void)this; } works as well.

void T::address() { this; } (-Wunused-variable) does not generate this

It is also interesting that the !dbg metadata on ret void, !dbg !17 is important. Dropping the metadata will cause the absence of this as a DW_TAG_formal_parameter (I think it has something to do with DW_AT_location (DW_OP_fbreg -8))

[dblaikie]

llvm/test/tools/llvm-dwarfdump/X86/statistics.ll

26–29

Is the IR being produced with optimizations enabled? Because at least for a simple test it looks like a location is generated even for a function that never names/mentions "this" (& if adding "(void)this;" changes the DWARF generation I'd be surprised/concerned, really - I don't think it should change the DWARF or the code generated)

$ cat test.cpp
struct t1 { void f1(); }; void t1::f1() { }
$ clang++-tot test.cpp -g -c && llvm-dwarfdump-tot test.o
test.o: file format elf64-x86-64

.debug_info contents:
0x00000000: Compile Unit: length = 0x00000070, format = DWARF32, version = 0x0004, abbr_offset = 0x0000, addr_size = 0x08 (next unit at 0x00000074)

0x0000000b: DW_TAG_compile_unit
              DW_AT_producer    ("clang version 12.0.0 ([e-mail]:llvm/llvm-project.git 545abf55a70686cd1335a95075bc2c6eba8b135d)")
              DW_AT_language    (DW_LANG_C_plus_plus_14)
              DW_AT_name        ("test.cpp")
              DW_AT_stmt_list   (0x00000000)
              DW_AT_comp_dir    ("/usr/local/google/home/blaikie/dev/scratch")
              DW_AT_low_pc      (0x0000000000000000)
              DW_AT_high_pc     (0x000000000000000a)

0x0000002a:   DW_TAG_structure_type
                DW_AT_calling_convention        (DW_CC_pass_by_value)
                DW_AT_name      ("t1")
                DW_AT_byte_size (0x01)
                DW_AT_decl_file ("/usr/local/google/home/blaikie/dev/scratch/test.cpp")
                DW_AT_decl_line (1)

0x00000033:     DW_TAG_subprogram
                  DW_AT_linkage_name    ("_ZN2t12f1Ev")
                  DW_AT_name    ("f1")
                  DW_AT_decl_file       ("/usr/local/google/home/blaikie/dev/scratch/test.cpp")
                  DW_AT_decl_line       (1)
                  DW_AT_declaration     (true)
                  DW_AT_external        (true)

0x0000003e:       DW_TAG_formal_parameter
                    DW_AT_type  (0x00000045 "t1*")
                    DW_AT_artificial    (true)

0x00000043:       NULL

0x00000044:     NULL

0x00000045:   DW_TAG_pointer_type
                DW_AT_type      (0x0000002a "t1")

0x0000004a:   DW_TAG_subprogram
                DW_AT_low_pc    (0x0000000000000000)
                DW_AT_high_pc   (0x000000000000000a)
                DW_AT_frame_base        (DW_OP_reg6 RBP)
                DW_AT_object_pointer    (0x00000061)
                DW_AT_specification     (0x00000033 "_ZN2t12f1Ev")

0x00000061:     DW_TAG_formal_parameter
                  DW_AT_location        (DW_OP_fbreg -8)
                  DW_AT_name    ("this")
                  DW_AT_type    (0x0000006e "t1*")
                  DW_AT_artificial      (true)

0x0000006d:     NULL

[MaskRay]

llvm/test/tools/llvm-dwarfdump/X86/statistics.ll

26–29

Reperformed my experiment. You are right: this does not need to appear in the C++ file. I'll drop this.

My other comment about !dbg metadata on ret void, !dbg !17 is correct.

[dblaikie]

Sorry, looking at the test now - is there any particular reason to test this case in particular? Looks like there's no particular statistics for artificial variables & I guess the statistics code doesn't look at this property one way or another?

[MaskRay]

Simply the test to void T::empty() {}

[MaskRay]

In D86044#2222140, @dblaikie wrote:

Sorry, looking at the test now - is there any particular reason to test this case in particular? Looks like there's no particular statistics for artificial variables & I guess the statistics code doesn't look at this property one way or another?

Statistics.cpp:343 has a different DW_AT_artificial use. If we change Statistics.cpp:564 to use NumArtificial in IsFunction case, a DW_AT_artificial variable will be significant.

[dblaikie]

In D86044#2222156, @MaskRay wrote:

In D86044#2222140, @dblaikie wrote:

Sorry, looking at the test now - is there any particular reason to test this case in particular? Looks like there's no particular statistics for artificial variables & I guess the statistics code doesn't look at this property one way or another?

Statistics.cpp:343 has a different DW_AT_artificial use. If we change Statistics.cpp:564 to use NumArtificial in IsFunction case, a DW_AT_artificial variable will be significant.

Not sure I follow, sorry. Are you planning to add a new statistic?

[MaskRay]

In D86044#2222168, @dblaikie wrote:

In D86044#2222156, @MaskRay wrote:

In D86044#2222140, @dblaikie wrote:

Sorry, looking at the test now - is there any particular reason to test this case in particular? Looks like there's no particular statistics for artificial variables & I guess the statistics code doesn't look at this property one way or another?

Statistics.cpp:343 has a different DW_AT_artificial use. If we change Statistics.cpp:564 to use NumArtificial in IsFunction case, a DW_AT_artificial variable will be significant.

Not sure I follow, sorry. Are you planning to add a new statistic?

No. The new test checks that this (even if not written) is counted as a source variable.

[dblaikie]

Ah, I see - variables are classified as artificial, constant, or local. Even though there's no artificial variable statistic, as such.

I think it might be better if the code tracked the total vars directly - rather than classifying them into 3 groups then merging those again later - if some code decides to have a 4th classification, this merging could get missed & become buggy, compared to maintaining it all along as a separate stat.

Also it looks like artificial parameters aren't getting tracking for all the other stuff - "NumParams", "NumParamTypes", "NumParamSourceLocations", "NumParamLocations"... which seems questionable. Wonder if there was any particular thinking around/justification for that in the original commits of this functionality.

(that said, I haven't had much to do with the Statistics code - so perhaps other folks have other ideas)

llvm/test/tools/llvm-dwarfdump/X86/statistics.ll

 ; int __attribute__((always_inline)) square(int i) { return i * i; }
 ; int cube(int i) {
 ;   int squared = square(i);
 ;   return squared*i;
 ; }
 ;
 ; int boo(int, int) {}
 
+; struct T {
+;   void empty();
+; };
+; void T::empty() {}
+
 ; Following variables/arguments/members should be counted:
 ;     - GlobalConst,
 ;     - Global,
 ;     - s, s.constant,
 ;     - square::i,
 ;     - cube::i, cube::squared
 ;     - boo::1, boo::2
+;     - this in T::empty()
 ; Skipped entities:
 ;     - declaration of test::a,
 ;     - non-constant member S:fn,
 ;     - arguments of S:fn.
 
-; CHECK: "#unique source variables":9
+; CHECK: "#unique source variables":10
 ; +1 extra inline i.
-; CHECK: "#source variables":10
+; CHECK: "#source variables":11
 ; -1 square::i
-; CHECK: "#source variables with location":9
+; CHECK: "#source variables with location":10
 ; CHECK: "sum_all_local_vars(#bytes in parent scope)":[[BYTES:[0-9]+]]
 ; Because of the dbg.value in the middle of the function, the pc range coverage
 ; must be below 100%.
 ; CHECK-NOT: "sum_all_local_vars(#bytes in parent scope covered by DW_AT_location)":0
 ; CHECK-NOT: "sum_all_local_vars(#bytes in parent scope covered by DW_AT_location)":[[BYTES]]
 ; CHECK: "sum_all_local_vars(#bytes in parent scope covered by DW_AT_location)":
 ; CHECK: "#bytes witin functions":[[FUNCSIZE:[0-9]+]]
 ; CHECK: "#bytes witin inlined functions":[[INLINESIZE:[0-9]+]]
-; CHECK: "#bytes in __debug_info":380
+; CHECK: "#bytes in __debug_info":459
 ; CHECK: "#bytes in __debug_loc":35
-; CHECK: "#bytes in __debug_abbrev":303
-; CHECK: "#bytes in __debug_line":117
-; CHECK: "#bytes in __debug_str":204
+; CHECK: "#bytes in __debug_abbrev":384
+; CHECK: "#bytes in __debug_line":126
+; CHECK: "#bytes in __debug_str":231
 
 ; ModuleID = '/tmp/quality.cpp'
 source_filename = "/tmp/quality.cpp"
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.12.0"
 
 %struct.S = type { i8 (i32)* }
 
entry:
   %.addr1 = alloca i32, align 4
   store i32 %0, i32* %.addr, align 4
   call void @llvm.dbg.declare(metadata i32* %.addr, metadata !55, metadata !DIExpression()), !dbg !56
   store i32 %1, i32* %.addr1, align 4
   call void @llvm.dbg.declare(metadata i32* %.addr1, metadata !57, metadata !DIExpression()), !dbg !58
   ret i32 0, !dbg !58
 }
 
+%struct.T = type { i8 }
+
+define void @_ZN1T5emptyEv(%struct.T* %this) #2 !dbg !59 {
+entry:
+  %this.addr = alloca %struct.T*, align 8
+  store %struct.T* %this, %struct.T** %this.addr, align 8
+  call void @llvm.dbg.declare(metadata %struct.T** %this.addr, metadata !67, metadata !DIExpression()), !dbg !69
+  %this1 = load %struct.T*, %struct.T** %this.addr, align 8
+  ret void, !dbg !70
+}
+
 attributes #0 = { alwaysinline nounwind ssp uwtable }
 attributes #1 = { nounwind readnone speculatable }
 attributes #2 = { noinline nounwind optnone ssp uwtable }
 
 !llvm.dbg.cu = !{!2}
 !llvm.module.flags = !{!15, !16, !17, !18}
 !llvm.ident = !{!19}
 

 !51 = !DINamespace(name: "test", scope: !2)
 !52 = distinct !DISubprogram(name: "boo", linkageName: "_Z3booii", scope: !3, file: !3, line: 10, type: !53, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !4)
 !53 = !DISubroutineType(types: !54)
 !54 = !{!8, !8, !8}
 !55 = !DILocalVariable(arg: 1, scope: !52, file: !3, line: 10, type: !8)
 !56 = !DILocation(line: 10, column: 12, scope: !52)
 !57 = !DILocalVariable(arg: 2, scope: !52, file: !3, line: 10, type: !8)
 !58 = !DILocation(line: 10, column: 17, scope: !52)
+
+!59 = distinct !DISubprogram(name: "empty", linkageName: "_ZN1T5emptyEv", scope: !60, file: !3, line: 25, type: !63, scopeLine: 25, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !2, declaration: !62, retainedNodes: !4)
+!60 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "T", file: !3, line: 22, size: 8, flags: DIFlagTypePassByValue, elements: !61, identifier: "_ZTS1T")
+!61 = !{!62}
+!62 = !DISubprogram(name: "empty", linkageName: "_ZN1T5emptyEv", scope: !60, file: !3, line: 23, type: !63, scopeLine: 23, flags: DIFlagPrototyped, spFlags: 0)
+!63 = !DISubroutineType(types: !64)
+!64 = !{!65, !66}
+!65 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64)
+!66 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !60, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)
+!67 = !DILocalVariable(name: "this", arg: 1, scope: !59, type: !68, flags: DIFlagArtificial | DIFlagObjectPointer)
+!68 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !60, size: 64)
+!69 = !DILocation(line: 0, scope: !59)
+!70 = !DILocation(line: 25, column: 19, scope: !59)