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

Mingming Liu via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 3 21:38:21 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) /
----------------
minglotus-6 wrote:

> I suppose this PR was intended to remove the sizeof(__llvm_profile_data) - 1 part? 

Hi @whentojump , yes, the intention is to remove `sizeof(__llvm_profile_data) - 1` and detect any users that depend on `sizeof(__llvm_profile_data) - 1` by the updated compiler-rt test.  Obviously I only removed the comment which is not intentional. 

>  It was originally added for i386 Darwin (https://reviews.llvm.org/D17623?id=49120)

thanks for the pointer!

>  and I guess that is retiring any way

The post-submit checks of https://github.com/llvm/llvm-project/commit/78647116d85b30c9b8b31a4690758c33f50c0550 doesn't have darwins i386. However, I think it's better not to break Darwins i386 for downstream users now we know D17623.

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


More information about the llvm-commits mailing list