[PATCH] D47208: [profile] Support profiling runtime on Fuchsia

Vedant Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 22 10:58:46 PDT 2018


vsk added a comment.

This looks good at a high level. How is it tested?



================
Comment at: compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c:31
+COMPILER_RT_VISIBILITY
+void __llvm_profile_initialize_file(void) {
+  /* Unsupported */
----------------
phosek wrote:
> This is only needed because runtime registration invokes it: https://reviews.llvm.org/diffusion/L/browse/compiler-rt/trunk/lib/profile/InstrProfilingRuntime.cc;333007$24, if we did our own registration we could avoid this function altogether and we could also name `__llvm_profile_register_write_file_atexit` more appropriately (since there're no files involved) but I'm not sure whether it's worth the duplication?
Having an imperfect name sounds a bit better to me than copying the code for the static initializer.


================
Comment at: compiler-rt/lib/profile/InstrProfilingPlatformLinux.c:10
 
-#if defined(__linux__) || defined(__FreeBSD__) || \
+#if defined(__linux__) || defined(__FreeBSD__) || defined(__Fuchsia__) || \
     (defined(__sun__) && defined(__svr4__))
----------------
phosek wrote:
> This file seems more related to the file format and linker support than platform. Would there be any objections against renaming `InstrProfilingPlatformLinux.c` to `InstrProfilingPlatformELF.c` and similarly `InstrProfilingPlatformDarwin.c` to `InstrProfilingPlatformMachO.c`?
No objection here.


Repository:
  rL LLVM

https://reviews.llvm.org/D47208





More information about the llvm-commits mailing list