[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