zequanwu added inline comments.
================
Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:129
+ // enabled.
+ if (Header->DataSize == 0) {
+ if (!(__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)) {
----------------
ellis wrote:
> Since we don't have the data section, we need to be confident that existing
> profile comes from the same binary (so that the counter section is
> identical). Can we add some extra checks here? I'm thinking we can verify
> that some fields in the headers match and that the variant flags are
> identical.
The [[
https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/profile/InstrProfiling.h#L105-L107
| comment ]] says the caller of this function is responsible for the check.
And we [[
https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/profile/InstrProfilingMerge.c#L48
| verifies ]] that fields in both headers math before merging at
doProfileMerging.
================
Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:134-136
+ for (SrcCounter = SrcCountersStart,
+ DstCounter = __llvm_profile_begin_counters();
+ SrcCounter < SrcCountersEnd;) {
----------------
ellis wrote:
> Can you add a check to make sure src and dst have the same number of counters
> (`SrcCountersEnd - SrcCountersStart`)?
Added the check at `__llvm_profile_check_compatibility`.
I use in-memory `__llvm_profile_counter_entry_size()` to calculate in-file
SrcCountersEnd, because the header only tells number of counters.
================
Comment at: compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c:26
+// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm
--disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp
%S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t.dSYM %t.profdir/
----------------
ellis wrote:
> We need to run this line twice to correctly test
> `__llvm_profile_merge_from_buffer()`
Added.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157632/new/
https://reviews.llvm.org/D157632
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
