[PATCH] D116179: [InstrProf][NFC] Do not assume size of counter type
Kyungwoo Lee via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 30 10:06:32 PST 2021
kyulee added inline comments.
================
Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:159
for (unsigned I = 0; I < NC; I++)
- DstCounters[I] += SrcCounters[I];
+ ((uint64_t *)DstCounters)[I] += ((uint64_t *)SrcCounters)[I];
----------------
This also assumes the counter type is a 64 bit integer. Should we just use the counter element size like `NC * __llvm_profile_counter_entry_size()` for the loop bound?
It appears this is actually just a memcpy, and I guess the compiler will emit it for you.
================
Comment at: llvm/include/llvm/ProfileData/InstrProfReader.h:317
+ // The initial CountersDelta is the in-memory address difference between
+ // the data and counts sections:
+ // start(__llvm_prf_cnts) - start(__llvm_prf_data)
----------------
counts -> counter
================
Comment at: llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test:6
// INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL))
// INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSize)
// INSTR_PROF_RAW_HEADER(uint64_t, CountersSize, CountersSize)
----------------
DataSize -> NumData
Same thing for Counter.
================
Comment at: llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test:56
RUN: not llvm-profdata merge -o /dev/null %t.profraw 2>&1 | FileCheck %s
-CHECK: warning: {{.+}}: malformed instrumentation profile data: number of counters 3 is greater than the maximum number of counters 2
+CHECK: warning: {{.+}}: malformed instrumentation profile data: number of counters 2 is greater than the maximum number of counters 0
CHECK: error: no profile can be merged
----------------
Why is the behavior changed?
My understanding is that now you check # of counters against the maximum # of counters read from the header which appears 0.
What happens if we fix it in the header as 2 so that # of counters are okay because of type size truncation, but still off by 1?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116179/new/
https://reviews.llvm.org/D116179
More information about the llvm-commits
mailing list