[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

We've been investigating a serious performance regression in our latest Clang 
roll that is affecting coverage. Whereas previously, we could process all 
coverage data in under an hour, now the job times out after 6 hours. After 
investigation, I found out that this change is the culprit. Specifically, it 
seems like `llvm-cov` has gotten significantly slower.

Before this change:

  $ time llvm-cov show -instr-profile merged.profdata -summary-only 
e91b5a1a324aa156
  llvm-cov     0.92s user 0.12s system 99% cpu 1.038 total

After this change:

  $ time llvm-cov show -instr-profile merged.profdata -summary-only 
e91b5a1a324aa156
  llvm-cov     5.61s user 0.89s system 99% cpu 6.495 total

I did some investigation and it seems like the time increase is related to 
`InstrProfSymtab` that used in the `CoverageReader`. I saw that subsequent 
changes fix some issues trying to avoid unnecessary copies, but that still 
hasn't addressed the slowdown since the tip-of-tree is still exhibiting this 
issue.

It's also possible that there isn't any issue that was introduced here, the 
problem is that the implementation simply doesn't scale. For example, the 
binary `e91b5a1a324aa156` I used here has 95527 symbol entries, most of them 
are the `__covrec_*` symbols, and that's one of the smaller binaries we have in 
our build.

Regardless, this is still a serious regression and I'm wondering if there's 
anything we can do short of reverting this change?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D157913#4648313 , @ellis wrote:

> I have seen that MachO sections remain in the header even if the size is zero 
> which seems to be the case here. You could use `llvm-objdump` to verify that 
> the section exists but the size is zero.

Thanks, I didn't notice that. And this can be easily detected.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

I feel like either this feature not working well with mac or we should keep 
`__llvm_prf_names` section in the binary.

Current idea is to completely get rid of `__llvm_prf_names` section in the 
binary. Even if I fixed the issue https://reviews.llvm.org/D157913#4648296, 
llvm-cov will need to read information from 3 different places on mac: the 
binary (for `__llvm_cov_fun` and `__llvm_cov_map`), debug info (unused function 
names, which could live in `__llvm_prf_names`) and indexed profile file for 
(counter, data, instrumented function names). If we keep `__llvm_prf_names` in 
the binary, then llvm-cov doesn't need to look into debug info again.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Ellis Hoag via Phabricator via cfe-commits]

ellis added a comment.

In D157913#4648296 , @zequanwu wrote:

> For some unknown reasons, linker still generates __llvm_prf_names section in 
> the final binary on mac even if the object file doesn't have this section. 
> And I cannot use llvm-objdump to get anything from that section.
>
>   $ llvm-readelf -S ../tmp/a.out
>   ...
> Section {
>   Index: 15
>   Name: __llvm_prf_names (5F 5F 6C 6C 76 6D 5F 70 72 66 5F 6E 61 6D 65 73)
>   Segment: __DATA (5F 5F 44 41 54 41 00 00 00 00 00 00 00 00 00 00)
>   Address: 0x100018000
>   Size: 0x0
>   Offset: 98304
>   Alignment: 0
>   RelocationOffset: 0x0
>   RelocationCount: 0
>   Type: Regular (0x0)
>   Attributes [ (0x0)
>   ]
>   Reserved1: 0x0
>   Reserved2: 0x0
>   Reserved3: 0x0
> }
>   ...
>   $ llvm-objdump --section=__llvm_prf_names --full-contents ../tmp/a.out
>   ../tmp/a.out:   file format mach-o arm64
>
> I will delete the test for now as it's not working on mac: 
> `compiler-rt/test/profile/Darwin/coverage-debug-info-correlate.cpp`

I have seen that MachO sections remain in the header even if the size is zero 
which seems to be the case here. You could use `llvm-objdump` to verify that 
the section exists but the size is zero.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu marked an inline comment as done.
zequanwu added a comment.

For some unknown reasons, linker still generates __llvm_prf_names section in 
the final binary on mac even if the object file doesn't have this section. And 
I cannot use llvm-objdump to get anything from that section.

  $ llvm-readelf -S ../tmp/a.out
  ...
Section {
  Index: 15
  Name: __llvm_prf_names (5F 5F 6C 6C 76 6D 5F 70 72 66 5F 6E 61 6D 65 73)
  Segment: __DATA (5F 5F 44 41 54 41 00 00 00 00 00 00 00 00 00 00)
  Address: 0x100018000
  Size: 0x0
  Offset: 98304
  Alignment: 0
  RelocationOffset: 0x0
  RelocationCount: 0
  Type: Regular (0x0)
  Attributes [ (0x0)
  ]
  Reserved1: 0x0
  Reserved2: 0x0
  Reserved3: 0x0
}
  ...
  $ llvm-objdump --section=__llvm_prf_names --full-contents ../tmp/a.out
  ../tmp/a.out: file format mach-o arm64

I will delete the test for now as it's not working on mac: 
`compiler-rt/test/profile/Darwin/coverage-debug-info-correlate.cpp`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu marked an inline comment as done.
zequanwu added inline comments.



Comment at: llvm/lib/ProfileData/InstrProfCorrelator.cpp:42
+const char *InstrProfCorrelator::CovFunctionNameAttributeName =
+"Cov Function Name";
 

phosek wrote:
> I missed this earlier, but I think that spelling this out in full, that is 
> `Coverage Function Name`, would be more consistent with the `Coverage Type`.
Renamed at: 
https://github.com/llvm/llvm-project/commit/37402c34918a4627de77c3b20964207892383242


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: llvm/lib/ProfileData/InstrProfCorrelator.cpp:42
+const char *InstrProfCorrelator::CovFunctionNameAttributeName =
+"Cov Function Name";
 

I missed this earlier, but I think that spelling this out in full, that is 
`Coverage Function Name`, would be more consistent with the `Coverage Type`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D157913#4646718 , @thakis wrote:

> Looks like this breaks check-clang on mac: 
> http://45.33.8.238/macm1/69297/step_7.txt
>
> Please take a look and revert for now if it takes a while to fix.

https://github.com/llvm/llvm-project/commit/1f33bfc23fb9e94b4db75b1b18fd00a438446f51
 should fix it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks check-clang on mac: 
http://45.33.8.238/macm1/69297/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG32db121b29f7: [Coverage] Allow Clang coverage to be used 
with debug info correlation. (authored by zequanwu).

Changed prior to commit:
  https://reviews.llvm.org/D157913?vs=556809=556869#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CodeGen/coverage-profile-raw-version.c
  compiler-rt/test/profile/Darwin/coverage-debug-info-correlate.cpp
  compiler-rt/test/profile/Linux/coverage-debug-info-correlate.cpp
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/include/llvm/ProfileData/InstrProfCorrelator.h
  llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/InstrProfCorrelator.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-byte-coverage.ll
  
llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-clang-coverage.ll
  llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-coverage.ll

Index: llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-clang-coverage.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-clang-coverage.ll
@@ -0,0 +1,64 @@
+; Test if unused function names are correctly emitted as dwarf variable debug info under DW_TAG_compile_unit
+
+; RUN: opt < %s -passes=instrprof -debug-info-correlate -S > %t.ll
+; RUN: FileCheck < %t.ll --implicit-check-not "{{__llvm_prf_data|__llvm_prf_names}}" %s
+; RUN: %llc_dwarf -O0 -filetype=obj < %t.ll | llvm-dwarfdump - | FileCheck %s --check-prefix=CHECK-DWARF
+
+; REQUIRES: system-linux, object-emission
+
+@__profn_foo = private constant [3 x i8] c"foo"
+@__profn_bar = private constant [3 x i8] c"bar"
+@__profn_baz = private constant [3 x i8] c"baz"
+@__llvm_coverage_names = internal constant [2 x ptr] [ptr @__profn_bar, ptr @__profn_baz]
+; CHECK: @__llvm_coverage_names = internal constant [2 x ptr] [ptr @__profn_bar, ptr @__profn_baz], !dbg ![[DBG:[0-9]+]]
+; CHECK: ![[DBG]] = !DIGlobalVariableExpression(var: ![[VAR:[0-9]+]], expr: !DIExpression())
+; CHECK: ![[VAR]] = {{.*}} !DIGlobalVariable(name: "__llvm_coverage_names"
+; CHECK-SAME: scope: ![[SCOPE:[0-9]+]]
+; CHECK-SAME: annotations: ![[ANNOTATIONS:[0-9]+]]
+; CHECK: ![[SCOPE]] = {{.*}} !DICompileUnit(
+; CHECK: ![[ANNOTATIONS]] = !{![[FUNC_NAME1:[0-9]+]], ![[FUNC_NAME2:[0-9]+]]}
+; CHECK: ![[FUNC_NAME1]] = !{!"Cov Function Name", !"bar"}
+; CHECK: ![[FUNC_NAME2]] = !{!"Cov Function Name", !"baz"}
+
+define void @_Z3foov() !dbg !12 {
+  call void @llvm.instrprof.increment(ptr @__profn_foo, i64 12345678, i32 2, i32 0)
+  ret void
+}
+
+declare void @llvm.instrprof.increment(ptr, i64, i32, i32)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 14.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!2 = !{i32 7, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"branch-target-enforcement", i32 0}
+!6 = !{i32 8, !"sign-return-address", i32 0}
+!7 = !{i32 8, !"sign-return-address-all", i32 0}
+!8 = !{i32 8, !"sign-return-address-with-bkey", i32 0}
+!9 = !{i32 7, !"uwtable", i32 1}
+!10 = !{i32 7, !"frame-pointer", i32 1}
+!11 = !{!"clang version 14.0.0"}
+!12 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !13, file: !13, line: 1, type: !14, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !16)
+!13 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!14 = !DISubroutineType(types: !15)
+!15 = !{null}
+!16 = !{}
+
+; CHECK-DWARF: DW_TAG_compile_unit
+; CHECK-DWARF:   DW_TAG_variable
+; CHECK-DWARF: DW_AT_name	("__llvm_coverage_names")
+; CHECK-DWARF: DW_AT_type	({{.*}} "Coverage Type")
+; CHECK-DWARF: DW_TAG_LLVM_annotation
+; CHECK-DWARF:   DW_AT_name	("Cov Function Name")
+; CHECK-DWARF:   DW_AT_const_value	("bar")
+; CHECK-DWARF: DW_TAG_LLVM_annotation
+; CHECK-DWARF:   DW_AT_name	("Cov Function Name")
+; CHECK-DWARF:   DW_AT_const_value	("baz")
+; CHECK-DWARF:   DW_TAG_unspecified_type
+; CHECK-DWARF: DW_AT_name ("Coverage Type")
+; CHECK-DWARF:   NULL
Index: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
===
--- 

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Ellis Hoag via Phabricator via cfe-commits]

ellis accepted this revision.
ellis added a comment.
This revision is now accepted and ready to land.

In D157913#4646201 , @zequanwu wrote:

> In D157913#4646184 , @ellis wrote:
>
>> It looks like `debug-info-correlate-coverage.ll` was renamed twice. Is this 
>> intended?
>
> I just moved 
> `llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-coverage.ll.` 
> to 
> `llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-byte-coverage.ll`
>  and created a new test file 
> `llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-clang-coverage.ll.`

Got it! Sorry I got confused by the UI.

Sounds good to me! I'm excited to see clang instrumentation can take full 
advantage of debug info correlation!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D157913#4646184 , @ellis wrote:

> It looks like `debug-info-correlate-coverage.ll` was renamed twice. Is this 
> intended?

I just moved 
`llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-coverage.ll.` to 
`llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-byte-coverage.ll`
 and created a new test file 
`llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-clang-coverage.ll.`




Comment at: compiler-rt/lib/profile/InstrProfilingWriter.c:276-277
   __llvm_profile_get_num_counters(CountersBegin, CountersEnd);
-  const uint64_t NamesSize = DebugInfoCorrelate ? 0 : NamesEnd - NamesBegin;
+  const uint64_t NamesSize =
+  DebugInfoCorrelate && IRLevelProfile ? 0 : NamesEnd - NamesBegin;
 

ellis wrote:
> Can we remove changes to this file now that we are stripping the names 
> section?
Reverted all changes in this file.



Comment at: llvm/lib/ProfileData/InstrProfCorrelator.cpp:423
+  }
+  return;
+}

ellis wrote:
> I think this return should be on the line above to break out of the for loop 
> early. Or did I miscount the brackets?
Removed that if. It's already checked inside `IsDIEOfCovName`.



Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:769
   }
+  if (DebugInfoCorrelate && !ReferencedNames.empty()) {
+MDNode *Node = *M->debug_compile_units_begin();

ellis wrote:
> Just so that I understand. Does this code run only for names of unused 
> functions? Or will we store all function names with 
> `CovFunctionNameAnnotation`?
Just the names of unused functions. All instrumented functions names will still 
be stored at the same place as before.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 556809.
zequanwu marked 3 inline comments as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CodeGen/coverage-profile-raw-version.c
  compiler-rt/lib/profile/InstrProfilingWriter.c
  compiler-rt/test/profile/Darwin/coverage-debug-info-correlate.cpp
  compiler-rt/test/profile/Linux/coverage-debug-info-correlate.cpp
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/ProfileData/InstrProfCorrelator.h
  llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/InstrProfCorrelator.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-byte-coverage.ll
  
llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-clang-coverage.ll
  llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-coverage.ll

Index: llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-clang-coverage.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-clang-coverage.ll
@@ -0,0 +1,66 @@
+; Test if unused function names are correctly emitted as dwarf variable debug info under DW_TAG_compile_unit
+
+; RUN: opt < %s -passes=instrprof -debug-info-correlate -S > %t.ll
+; RUN: FileCheck < %t.ll --implicit-check-not "{{__llvm_prf_data|__llvm_prf_names}}" %s
+; RUN: %llc_dwarf -O0 -filetype=obj < %t.ll | llvm-dwarfdump - | FileCheck %s --check-prefix=CHECK-DWARF
+
+; REQUIRES: system-linux, object-emission
+
+
+
+@__profn_foo = private constant [3 x i8] c"foo"
+@__profn_bar = private constant [3 x i8] c"bar"
+@__profn_baz = private constant [3 x i8] c"baz"
+@__llvm_coverage_names = internal constant [2 x ptr] [ptr @__profn_bar, ptr @__profn_baz]
+; CHECK: @__llvm_coverage_names = internal constant [2 x ptr] [ptr @__profn_bar, ptr @__profn_baz], !dbg ![[DBG:[0-9]+]]
+; CHECK: ![[DBG]] = !DIGlobalVariableExpression(var: ![[VAR:[0-9]+]], expr: !DIExpression())
+; CHECK: ![[VAR]] = {{.*}} !DIGlobalVariable(name: "__llvm_coverage_names"
+; CHECK-SAME: scope: ![[SCOPE:[0-9]+]]
+; CHECK-SAME: annotations: ![[ANNOTATIONS:[0-9]+]]
+; CHECK: ![[SCOPE]] = {{.*}} !DICompileUnit(
+; CHECK: ![[ANNOTATIONS]] = !{![[FUNC_NAME1:[0-9]+]], ![[FUNC_NAME2:[0-9]+]]}
+; CHECK: ![[FUNC_NAME1]] = !{!"Cov Function Name", !"bar"}
+; CHECK: ![[FUNC_NAME2]] = !{!"Cov Function Name", !"baz"}
+
+define void @_Z3foov() !dbg !12 {
+  call void @llvm.instrprof.increment(ptr @__profn_foo, i64 12345678, i32 2, i32 0)
+  ret void
+}
+
+declare void @llvm.instrprof.increment(ptr, i64, i32, i32)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 14.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!2 = !{i32 7, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"branch-target-enforcement", i32 0}
+!6 = !{i32 8, !"sign-return-address", i32 0}
+!7 = !{i32 8, !"sign-return-address-all", i32 0}
+!8 = !{i32 8, !"sign-return-address-with-bkey", i32 0}
+!9 = !{i32 7, !"uwtable", i32 1}
+!10 = !{i32 7, !"frame-pointer", i32 1}
+!11 = !{!"clang version 14.0.0"}
+!12 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !13, file: !13, line: 1, type: !14, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !16)
+!13 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!14 = !DISubroutineType(types: !15)
+!15 = !{null}
+!16 = !{}
+
+; CHECK-DWARF: DW_TAG_compile_unit
+; CHECK-DWARF:   DW_TAG_variable
+; CHECK-DWARF: DW_AT_name	("__llvm_coverage_names")
+; CHECK-DWARF: DW_AT_type	({{.*}} "Coverage Type")
+; CHECK-DWARF: DW_TAG_LLVM_annotation
+; CHECK-DWARF:   DW_AT_name	("Cov Function Name")
+; CHECK-DWARF:   DW_AT_const_value	("bar")
+; CHECK-DWARF: DW_TAG_LLVM_annotation
+; CHECK-DWARF:   DW_AT_name	("Cov Function Name")
+; CHECK-DWARF:   DW_AT_const_value	("baz")
+; CHECK-DWARF:   DW_TAG_unspecified_type
+; CHECK-DWARF: DW_AT_name ("Coverage Type")
+; CHECK-DWARF:   NULL
Index: llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-coverage.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-coverage.ll
@@ -1,34 +0,0 @@
-; RUN: opt < %s -passes=instrprof 

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Ellis Hoag via Phabricator via cfe-commits]

ellis added a comment.

It looks like `debug-info-correlate-coverage.ll` was renamed twice. Is this 
intended?




Comment at: compiler-rt/lib/profile/InstrProfilingWriter.c:276-277
   __llvm_profile_get_num_counters(CountersBegin, CountersEnd);
-  const uint64_t NamesSize = DebugInfoCorrelate ? 0 : NamesEnd - NamesBegin;
+  const uint64_t NamesSize =
+  DebugInfoCorrelate && IRLevelProfile ? 0 : NamesEnd - NamesBegin;
 

Can we remove changes to this file now that we are stripping the names section?



Comment at: llvm/lib/ProfileData/InstrProfCorrelator.cpp:423
+  }
+  return;
+}

I think this return should be on the line above to break out of the for loop 
early. Or did I miscount the brackets?



Comment at: llvm/lib/ProfileData/InstrProfReader.cpp:577-578
 // Correlator.
-assert(DataSize == 0 && NamesSize == 0);
-assert(CountersDelta == 0 && NamesDelta == 0);
+assert(DataSize == 0 && (!isIRLevelProfile() || NamesSize == 0));
+assert(CountersDelta == 0 && (!isIRLevelProfile() || NamesDelta == 0));
 Data = Correlator->getDataPointer();

I think these changes can also be removed



Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:769
   }
+  if (DebugInfoCorrelate && !ReferencedNames.empty()) {
+MDNode *Node = *M->debug_compile_units_begin();

Just so that I understand. Does this code run only for names of unused 
functions? Or will we store all function names with `CovFunctionNameAnnotation`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 556805.
zequanwu added a comment.

Remove profile name section.

Unused coverage function names are stored in debug info as a fake global 
variable info:

  0x000b: DW_TAG_compile_unit
  ...
  0x0071:   DW_TAG_variable
  DW_AT_name  ("__llvm_coverage_names")
  DW_AT_type  (0x0097 "Coverage Type")
  DW_AT_location  (DW_OP_addr 0x0)
  
  0x0084: DW_TAG_LLVM_annotation
DW_AT_name("Cov Function Name")
DW_AT_const_value ("bar")
  
  0x008d: DW_TAG_LLVM_annotation
DW_AT_name("Cov Function Name")
DW_AT_const_value ("baz")

llvm-cov will need to retrieve those names from the binary's debug info.

The reasons for not writing those names into the indexed profile file are 
following:

1. llvm-cov always need to read __llvm_covfun and __llvm_covmap from the 
unstripped binary, so it doesn't matter if those names are read from the binary 
or the indexed profile.
2. Since those names are just an array of strings, they don't have counter/data 
info. When llvm-profdata writes indexed profile, it iterates profile data to 
write records, so those unreferenced names are skipped even if I append them to 
the end of profile symtab. I'm not sure how we can simply append those names 
into the end of function name strings at indexed profile.

Overall, there are basically two types of function names:

1. Instrumented function names, which are stored in dwarf and later copied to 
indexed profile. llvm-cov reads them from indexed profile.
2. Unused function names(could be empty), which are stored in dwarf. llvm-cov 
reads them from the debug info.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CodeGen/coverage-profile-raw-version.c
  compiler-rt/lib/profile/InstrProfilingWriter.c
  compiler-rt/test/profile/Darwin/coverage-debug-info-correlate.cpp
  compiler-rt/test/profile/Linux/coverage-debug-info-correlate.cpp
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/ProfileData/InstrProfCorrelator.h
  llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/InstrProfCorrelator.cpp
  llvm/lib/ProfileData/InstrProfReader.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-byte-coverage.ll
  
llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-clang-coverage.ll
  llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-coverage.ll

Index: llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-clang-coverage.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/InstrProfiling/debug-info-correlate-clang-coverage.ll
@@ -0,0 +1,66 @@
+; Test if unused function names are correctly emitted as dwarf variable debug info under DW_TAG_compile_unit
+
+; RUN: opt < %s -passes=instrprof -debug-info-correlate -S > %t.ll
+; RUN: FileCheck < %t.ll --implicit-check-not "{{__llvm_prf_data|__llvm_prf_names}}" %s
+; RUN: %llc_dwarf -O0 -filetype=obj < %t.ll | llvm-dwarfdump - | FileCheck %s --check-prefix=CHECK-DWARF
+
+; REQUIRES: system-linux, object-emission
+
+
+
+@__profn_foo = private constant [3 x i8] c"foo"
+@__profn_bar = private constant [3 x i8] c"bar"
+@__profn_baz = private constant [3 x i8] c"baz"
+@__llvm_coverage_names = internal constant [2 x ptr] [ptr @__profn_bar, ptr @__profn_baz]
+; CHECK: @__llvm_coverage_names = internal constant [2 x ptr] [ptr @__profn_bar, ptr @__profn_baz], !dbg ![[DBG:[0-9]+]]
+; CHECK: ![[DBG]] = !DIGlobalVariableExpression(var: ![[VAR:[0-9]+]], expr: !DIExpression())
+; CHECK: ![[VAR]] = {{.*}} !DIGlobalVariable(name: "__llvm_coverage_names"
+; CHECK-SAME: scope: ![[SCOPE:[0-9]+]]
+; CHECK-SAME: annotations: ![[ANNOTATIONS:[0-9]+]]
+; CHECK: ![[SCOPE]] = {{.*}} !DICompileUnit(
+; CHECK: ![[ANNOTATIONS]] = !{![[FUNC_NAME1:[0-9]+]], ![[FUNC_NAME2:[0-9]+]]}
+; CHECK: ![[FUNC_NAME1]] = !{!"Cov Function Name", !"bar"}
+; CHECK: ![[FUNC_NAME2]] = !{!"Cov Function Name", !"baz"}
+
+define void @_Z3foov() !dbg !12 {
+  call void @llvm.instrprof.increment(ptr @__profn_foo, i64 12345678, i32 2, i32 0)
+  ret void
+}
+
+declare void @llvm.instrprof.increment(ptr, i64, i32, i32)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 14.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "debug-info-correlate.cpp", 

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Ellis Hoag via Phabricator via cfe-commits]

ellis added a comment.

In D157913#4638845 , @zequanwu wrote:

> In D157913#4638569 , @phosek wrote:
>
>> In D157913#4626007 , @zequanwu 
>> wrote:
>>
 It seems that the `__llvm_prf_names` is retained in this mode. What is the 
 overhead of this section generally? Can we instead use debug info to 
 lookup function names?
>>>
>>> With debug info correlation enabled, `__llvm_prf_names` section will only 
>>> contain functions names that are not emitted to the final binary, not even 
>>> in debug info.
>>
>> Could we emit those names into the `.debug_str` section?
>
> Yes, I think it's feasible by creating a fake variable debug info that 
> contains all the skipped function names in the source file for each object 
> file, but that's a bit more overhead in debug info compared to just plain 
> `__llvm_prf_names` in binary, which is only about 0.2% or less of original 
> size [1].
>
> [1]: https://bugs.chromium.org/p/chromium/issues/detail?id=1463755#c8

Since the `.debug_str` section can be stripped before execution time, I don't 
count that towards the binary size overhead. But I believe the 
`__llvm_prf_names` section cannot be stripped as is. So it would be an 
improvement if we could completely remove the `__llvm_prf_names` section and 
emit names in the `.debug_str` section.

I see that you have one case where the overhead of the names section is just 
0.2%. I wonder if this is common or if there exist real-world scenarios where 
the overhead is more significant. If not then leaving the names section in the 
binary might be ok, but we should probably leave a note explaining how we could 
remove it if we emit the names in debug info.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D157913#4638569 , @phosek wrote:

> In D157913#4626007 , @zequanwu 
> wrote:
>
>>> It seems that the `__llvm_prf_names` is retained in this mode. What is the 
>>> overhead of this section generally? Can we instead use debug info to lookup 
>>> function names?
>>
>> With debug info correlation enabled, `__llvm_prf_names` section will only 
>> contain functions names that are not emitted to the final binary, not even 
>> in debug info.
>
> Could we emit those names into the `.debug_str` section?

Yes, I think it's feasible by creating a fake variable debug info that contains 
all the skipped function names in the source file for each object file, but 
that's a bit more overhead in debug info compared to just plain 
`__llvm_prf_names` in binary, which is only about 0.2% or less of original size 
[1].

[1]: https://bugs.chromium.org/p/chromium/issues/detail?id=1463755#c8


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D157913#4626007 , @zequanwu wrote:

>> It seems that the `__llvm_prf_names` is retained in this mode. What is the 
>> overhead of this section generally? Can we instead use debug info to lookup 
>> function names?
>
> With debug info correlation enabled, `__llvm_prf_names` section will only 
> contain functions names that are not emitted to the final binary, not even in 
> debug info.

Could we emit those names into the `.debug_str` section?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added inline comments.



Comment at: compiler-rt/test/profile/Darwin/coverage-debug-info-correlate.cpp:29
+
+// RUN: llvm-cov report --instr-profile=%t.profdata %t | FileCheck %s 
-check-prefix=NONAME
+

ellis wrote:
> Is it worth testing that `%t.normal.profdata` emits the same coverage?
Added testing for `%t.normal.profdata`.



Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp:1019-1029
+  if (auto E = NamesSection.takeError()) {
+if (ProfileNames.isEmpty())
+  return std::move(E);
+consumeError(std::move(E));
+  } else {
+std::vector NamesSectionRefs = *NamesSection;
+if (NamesSectionRefs.size() != 1)

ellis wrote:
> I don't quite understand why this was changed. Is there is a test case that 
> covers this?
This part of code is used by llvm-cov. Originally, it always checks if the 
instrumented binary contains the `__llvm_prf_names ` section and assumes all 
instrumented function names are store there. Now, it will firstly create 
`InstrProfSymtab` from the indexed profile file and add remaining function 
names to this `InstrProfSymtab` from `__llvm_prf_names` section in the 
instrumented binary, if there is one. 

This is covered by `// RUN: llvm-cov export --format=lcov 
--instr-profile=%t.profdata %t | FileCheck %s -check-prefix=NAME`, where 
`_Z9used_funcv` and `main` are stored in debug info and later moved to indexed 
profile file, and `_ZN1A11unused_funcEv` is stored in `__llvm_prf_names` 
section in the binary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 555889.
zequanwu marked 4 inline comments as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CodeGen/coverage-profile-raw-version.c
  compiler-rt/lib/profile/InstrProfilingWriter.c
  compiler-rt/test/profile/Darwin/coverage-debug-info-correlate.cpp
  compiler-rt/test/profile/Linux/coverage-debug-info-correlate.cpp
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/InstrProfReader.cpp

Index: llvm/lib/ProfileData/InstrProfReader.cpp
===
--- llvm/lib/ProfileData/InstrProfReader.cpp
+++ llvm/lib/ProfileData/InstrProfReader.cpp
@@ -574,8 +574,8 @@
   if (Correlator) {
 // These sizes in the raw file are zero because we constructed them in the
 // Correlator.
-assert(DataSize == 0 && NamesSize == 0);
-assert(CountersDelta == 0 && NamesDelta == 0);
+assert(DataSize == 0 && (!isIRLevelProfile() || NamesSize == 0));
+assert(CountersDelta == 0 && (!isIRLevelProfile() || NamesDelta == 0));
 Data = Correlator->getDataPointer();
 DataEnd = Data + Correlator->getDataSize();
 NamesStart = Correlator->getNamesPointer();
Index: llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
===
--- llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -25,6 +25,7 @@
 #include "llvm/Object/MachOUniversal.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/ProfileData/InstrProf.h"
+#include "llvm/ProfileData/InstrProfReader.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/Debug.h"
@@ -981,7 +982,8 @@
 }
 
 static Expected>
-loadBinaryFormat(std::unique_ptr Bin, StringRef Arch,
+loadBinaryFormat(std::unique_ptr Bin,
+ IndexedInstrProfReader , StringRef Arch,
  StringRef CompilationDir = "",
  object::BuildIDRef *BinaryID = nullptr) {
   std::unique_ptr OF;
@@ -1010,11 +1012,21 @@
 
   // Look for the sections that we are interested in.
   auto ObjFormat = OF->getTripleObjectFormat();
+  InstrProfSymtab ProfileNames = ProfileReader.getSymtab();
   auto NamesSection =
   lookupSections(*OF, getInstrProfSectionName(IPSK_name, ObjFormat,
  /*AddSegmentInfo=*/false));
-  if (auto E = NamesSection.takeError())
-return std::move(E);
+  if (auto E = NamesSection.takeError()) {
+if (ProfileNames.isEmpty())
+  return std::move(E);
+consumeError(std::move(E));
+  } else {
+std::vector NamesSectionRefs = *NamesSection;
+if (NamesSectionRefs.size() != 1)
+  return make_error(coveragemap_error::malformed);
+if (Error E = ProfileNames.create(NamesSectionRefs.back()))
+  return std::move(E);
+  }
   auto CoverageSection =
   lookupSections(*OF, getInstrProfSectionName(IPSK_covmap, ObjFormat,
   /*AddSegmentInfo=*/false));
@@ -1028,13 +1040,6 @@
 return CoverageMappingOrErr.takeError();
   StringRef CoverageMapping = CoverageMappingOrErr.get();
 
-  InstrProfSymtab ProfileNames;
-  std::vector NamesSectionRefs = *NamesSection;
-  if (NamesSectionRefs.size() != 1)
-return make_error(coveragemap_error::malformed);
-  if (Error E = ProfileNames.create(NamesSectionRefs.back()))
-return std::move(E);
-
   // Look for the coverage records section (Version4 only).
   auto CoverageRecordsSections =
   lookupSections(*OF, getInstrProfSectionName(IPSK_covfun, ObjFormat,
@@ -1104,7 +1109,8 @@
 
 Expected>>
 BinaryCoverageReader::create(
-MemoryBufferRef ObjectBuffer, StringRef Arch,
+MemoryBufferRef ObjectBuffer, IndexedInstrProfReader ,
+StringRef Arch,
 SmallVectorImpl> ,
 StringRef CompilationDir, SmallVectorImpl *BinaryIDs) {
   std::vector> Readers;
@@ -1150,8 +1156,8 @@
   }
 
   return BinaryCoverageReader::create(
-  ArchiveOrErr.get()->getMemoryBufferRef(), Arch, ObjectFileBuffers,
-  CompilationDir, BinaryIDs);
+  ArchiveOrErr.get()->getMemoryBufferRef(), ProfileReader, Arch,
+  ObjectFileBuffers, CompilationDir, BinaryIDs);
 }
   }
 
@@ -1164,8 +1170,8 @@
 return ChildBufOrErr.takeError();
 
   auto ChildReadersOrErr = BinaryCoverageReader::create(
-  ChildBufOrErr.get(), Arch, ObjectFileBuffers, CompilationDir,
-  BinaryIDs);
+  ChildBufOrErr.get(), ProfileReader, Arch, ObjectFileBuffers,
+  CompilationDir, BinaryIDs);
   if 

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Ellis Hoag via Phabricator via cfe-commits]

ellis added a comment.

In D157913#4626007 , @zequanwu wrote:

>> It seems that the `__llvm_prf_names` is retained in this mode. What is the 
>> overhead of this section generally? Can we instead use debug info to lookup 
>> function names?
>
> With debug info correlation enabled, `__llvm_prf_names` section will only 
> contain functions names that are not emitted to the final binary, not even in 
> debug info. So, this section is still much smaller compared to when not 
> enabling debug info correlation. This is used to indicate that those 
> functions are covered but not executed.
>
> Example from the test case above:
>
>   $ cat a.cpp
>   struct A {
> void unused_func() {}
>   };
>   void used_func() {}
>   int main() {
> used_func();
> return 0;
>   }
>   $ clang -fprofile-instr-generate -fcoverage-mapping -mllvm 
> -enable-name-compression=false -mllvm -debug-info-correlate -g a.cpp -o a.out
>   $ llvm-objdump --section=__llvm_prf_names --full-contents a.out
>   
>   a.out:  file format elf64-x86-64
>   Contents of section __llvm_prf_names:
>7b65 14005f5a 4e314131 31756e75 7365645f  .._ZN1A11unused_
>7b75 66756e63 4576funcEv

Thanks for the explanation!




Comment at: clang/test/CodeGen/coverage-debug-info-correlate.c:1
+// RUN: %clang_cc1 -debug-info-kind=standalone -mllvm -debug-info-correlate 
-fprofile-instrument=clang -fcoverage-mapping -emit-llvm -o - %s | FileCheck %s
+

I think this test can be generalized to check that `__llvm_profile_raw_version` 
is emitted with or without debug info correlation.



Comment at: compiler-rt/test/profile/Darwin/coverage-debug-info-correlate.cpp:9
+// RUN: %clang_profgen -o %t -g -mllvm --debug-info-correlate 
-fcoverage-mapping %S/../Inputs/instrprof-debug-info-correlate-main.cpp 
%S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t.dSYM %t.profraw

NIT



Comment at: compiler-rt/test/profile/Darwin/coverage-debug-info-correlate.cpp:29
+
+// RUN: llvm-cov report --instr-profile=%t.profdata %t | FileCheck %s 
-check-prefix=NONAME
+

Is it worth testing that `%t.normal.profdata` emits the same coverage?



Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp:1019-1029
+  if (auto E = NamesSection.takeError()) {
+if (ProfileNames.isEmpty())
+  return std::move(E);
+consumeError(std::move(E));
+  } else {
+std::vector NamesSectionRefs = *NamesSection;
+if (NamesSectionRefs.size() != 1)

I don't quite understand why this was changed. Is there is a test case that 
covers this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

> It seems that the `__llvm_prf_names` is retained in this mode. What is the 
> overhead of this section generally? Can we instead use debug info to lookup 
> function names?

With debug info correlation enabled, `__llvm_prf_names` section will only 
contain functions names that are not emitted to the final binary, not even in 
debug info. So, this section is still much smaller compared to when not 
enabling debug info correlation. This is used to indicate that those functions 
are covered but not executed.

Example from the test case above:

  $ cat a.cpp
  struct A {
void unused_func() {}
  };
  void used_func() {}
  int main() {
used_func();
return 0;
  }
  $ clang -fprofile-instr-generate -fcoverage-mapping -mllvm 
-enable-name-compression=false -mllvm -debug-info-correlate -g a.cpp -o a.out
  $ llvm-objdump --section=__llvm_prf_names --full-contents a.out
  
  a.out:  file format elf64-x86-64
  Contents of section __llvm_prf_names:
   7b65 14005f5a 4e314131 31756e75 7365645f  .._ZN1A11unused_
   7b75 66756e63 4576funcEv


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157913: [Coverage] Allow Clang coverage to be used with debug info correlation.

[Ellis Hoag via Phabricator via cfe-commits]

ellis added a comment.

Thanks for working on this! I've added a few other reviewers that might be 
interested. In particular it might conflict with the stack D138846 
 IIRC
It seems that the `__llvm_prf_names` is retained in this mode. What is the 
overhead of this section generally? Can we instead use debug info to lookup 
function names?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157913/new/

https://reviews.llvm.org/D157913

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits