[PATCH] D13329: [PGO]: Eliminate __llvm_profile_register calls for Linux (compiler_rt)

[Xinliang David Li via llvm-commits]

Ping.

David

On Tue, Oct 6, 2015 at 11:34 AM, Xinliang David Li <davidxl at google.com> wrote:
> updated patch. PTAL.
>
> David
>
> On Tue, Oct 6, 2015 at 12:04 AM, Justin Bogner <mail at justinbogner.com> wrote:
>> David Li <davidxl at google.com> writes:
>>> davidxl updated this revision to Diff 36319.
>>> davidxl added a comment.
>>>
>>> This is the latest version that does not rely on linker script.
>>>
>>>
>>> http://reviews.llvm.org/D13329
>>>
>>> Files:
>>>   lib/profile/CMakeLists.txt
>>>   lib/profile/InstrProfilingPlatformLinux.c
>>>
>>> Index: lib/profile/InstrProfilingPlatformLinux.c
>>> ===================================================================
>>> --- /dev/null
>>> +++ lib/profile/InstrProfilingPlatformLinux.c
>>> @@ -0,0 +1,48 @@
>>> +/*===- InstrProfilingPlatformLinux.c - Profile data Linux platform ------===*\
>>> +|*
>>> +|*                     The LLVM Compiler Infrastructure
>>> +|*
>>> +|* This file is distributed under the University of Illinois Open Source
>>> +|* License. See LICENSE.TXT for details.
>>> +|*
>>> +\*===----------------------------------------------------------------------===*/
>>> +
>>> +#include "InstrProfiling.h"
>>> +
>>> +#if !defined(__APPLE__)
>>> +#include <stdlib.h>
>>> +
>>> +extern __llvm_profile_data __start___llvm_prf_data
>>> +    __attribute__((visibility("hidden")));
>>> +extern __llvm_profile_data __stop___llvm_prf_data
>>> +    __attribute__((visibility("hidden")));
>>> +extern uint64_t __start___llvm_prf_cnts __attribute__((visibility("hidden")));
>>> +extern uint64_t __stop___llvm_prf_cnts __attribute__((visibility("hidden")));
>>> +extern char __start___llvm_prf_names __attribute__((visibility("hidden")));
>>> +extern char __stop___llvm_prf_names __attribute__((visibility("hidden")));
>>> +
>>> +__attribute__((visibility("hidden"))) const __llvm_profile_data *
>>> +__llvm_profile_begin_data(void) {
>>> +  return &__start___llvm_prf_data;
>>> +}
>>> +__attribute__((visibility("hidden"))) const __llvm_profile_data *
>>> +__llvm_profile_end_data(void) {
>>> +  return &__stop___llvm_prf_data;
>>> +}
>>> +__attribute__((visibility("hidden"))) const char *__llvm_profile_begin_names(
>>> +    void) {
>>> +  return &__start___llvm_prf_names;
>>> +}
>>> +__attribute__((visibility("hidden"))) const char *__llvm_profile_end_names(
>>> +    void) {
>>> +  return &__stop___llvm_prf_names;
>>> +}
>>> +__attribute__((visibility("hidden"))) uint64_t *__llvm_profile_begin_counters(
>>> +    void) {
>>> +  return &__start___llvm_prf_cnts;
>>> +}
>>> +__attribute__((visibility("hidden"))) uint64_t *__llvm_profile_end_counters(
>>> +    void) {
>>> +  return &__stop___llvm_prf_cnts;
>>> +}
>>> +#endif
>>> Index: lib/profile/CMakeLists.txt
>>> ===================================================================
>>> --- lib/profile/CMakeLists.txt
>>> +++ lib/profile/CMakeLists.txt
>>> @@ -1,23 +1,35 @@
>>>  add_custom_target(profile)
>>>
>>> -set(PROFILE_SOURCES
>>> +set(COMMON_SOURCES
>>>    GCDAProfiling.c
>>>    InstrProfiling.c
>>>    InstrProfilingBuffer.c
>>>    InstrProfilingFile.c
>>> -  InstrProfilingPlatformDarwin.c
>>> -  InstrProfilingPlatformOther.c
>>>    InstrProfilingRuntime.cc
>>>    InstrProfilingUtil.c)
>>>
>>>  if(APPLE)
>>> +  set(PROFILE_SOURCES
>>> +      ${COMMON_SOURCES}
>>> +      InstrProfilingPlatformDarwin.c)
>>> +
>>>    add_compiler_rt_runtime(clang_rt.profile
>>>      STATIC
>>>      OS osx
>>>      ARCHS ${PROFILE_SUPPORTED_ARCH}
>>>      SOURCES ${PROFILE_SOURCES}
>>>      PARENT_TARGET profile)
>>>  else()
>>> +  if(UNIX)
>>> +    # Assume Linux
>>
>> I don't think that's a reasonable assumption. What is the actual
>> commonality where this works? ELF? GNU tools? Notably, if this works on
>> the various BSDs this comment is confusing, and if it doesn't the check
>> is wrong.
>>
>>> +    set(PROFILE_SOURCES
>>> +        ${COMMON_SOURCES}
>>> +        InstrProfilingPlatformLinux.c)
>>
>> If this does generalize to ELF or GNU we should call it PlatformELF or
>> PlatformGNU or whatever, so as not to confuse people.
>>
>>> +  else()
>>> +    set(PROFILE_SOURCES
>>> +        ${COMMON_SOURCES}
>>> +        InstrProfilingPlatformOther.c)
>>> +  endif()
>>>    add_compiler_rt_runtime(clang_rt.profile
>>>      STATIC
>>>      ARCHS ${PROFILE_SUPPORTED_ARCH}
>>>
>>>