[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