[compiler-rt] [nfc][compiler-rt]Remove round-up in __llvm_profile_get_num_data (PR #83194)

Wentao Zhang via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 4 14:27:15 PDT 2024


================
@@ -61,19 +61,12 @@ uint64_t __llvm_profile_get_size_for_buffer(void) {
       NamesBegin, NamesEnd, VTableBegin, VTableEnd, VNamesBegin, VNamesEnd);
 }
 
+// NOTE: Caller should guarantee that `Begin` and `End` specifies a half-open
+// interval [Begin, End). Namely, `End` is one-byte past the end of the array.
 COMPILER_RT_VISIBILITY
 uint64_t __llvm_profile_get_num_data(const __llvm_profile_data *Begin,
                                      const __llvm_profile_data *End) {
   intptr_t BeginI = (intptr_t)Begin, EndI = (intptr_t)End;
-  // `sizeof(__llvm_profile_data) - 1` is required in the numerator when
-  // [Begin, End] represents an inclusive range.
-  // For ELF, [Begin, End) represents the address of linker-inserted
-  // symbols  `__start__<elf-section>` and `__stop_<elf-section>`.
-  // Thereby, `End` is one byte past the inclusive range, and
-  // `sizeof(__llvm_profile_data) - 1` is not necessary in the numerator to get
-  // the correct number of profile data.
-  // FIXME: Consider removing `sizeof(__llvm_profile_data) - 1` if this is true
-  // across platforms.
   return ((EndI + sizeof(__llvm_profile_data) - 1) - BeginI) /
----------------
whentojump wrote:

I also found the other related patch 120f6301ed5cd3e0c54e912d4b303281753462d3. From their commit message: "The Darwin linker shrinks sections containing aligned structures when padding is not explicitly added to the end of the structure." -- this explains, although I have no way of reproducing it, both

1. the rationale of this `sizeof(__llvm_profile_data) - 1`, and
2. why `__llvm_profile_get_data_size()` below is not as simple as `return (intptr_t)(End) - (intptr_t)(Begin);` 

> I think it's better not to break Darwins i386 for downstream users now we know D17623.

If someone is still using the setup, I think `__llvm_profile_get_num_vtable()` is already suffering from the same problem :((... In fact I was planning to unify all these functions (https://github.com/xlab-uiuc/llvm-mcdc/commit/b9e7da0fe04853cffa689cd7c549dc9954fdf931) but now I'm less sure. Perhaps yeah let's not touch it.

(Allow me to cc @vedantk if you are still online recently)


https://github.com/llvm/llvm-project/pull/83194


More information about the llvm-commits mailing list