[compiler-rt] r328987 - [profile] Fix value profile runtime merging issues

Rong Xu via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 2 14:14:30 PDT 2018


The failed tests are the newly added tests. I'm looking into this.

-Rong

On Mon, Apr 2, 2018 at 1:39 PM Mike Edwards <mike at sqlby.me> wrote:

> Hi,
> It appears the commit related to this review has cause a bot failure on
> Green Dragon.  This failure was masked by an earlier failure which was
> reverted a short time ago.  Please have a look at:
> http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/44030/ and
> either revert your commit or notify me if you will be submitting a patch
> within the next hour.
>
> Respectfully,
> Mike Edwards
>
>
> On Mon, Apr 2, 2018 at 9:57 AM, Rong Xu via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: xur
>> Date: Mon Apr  2 09:57:00 2018
>> New Revision: 328987
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=328987&view=rev
>> Log:
>> [profile] Fix value profile runtime merging issues
>>
>> This patch fixes the following issues:
>> (1) The strong definition of the merge hook function was not working which
>> breaks the online value profile merging. This patch removes the weak
>> attribute of VPMergeHook and assigns the value dynamically.
>> (2) Truncate the proifle file so that we don't have garbage data at the
>> end of
>> the file.
>> (3) Add new __llvm_profile_instrument_target_value() interface to do the
>> value
>> profile update in batch. This is needed as the original incremental by 1
>> in __llvm_profile_instrument_target() is too slow for online merge.
>>
>> Differential Revision: https://reviews.llvm.org/D44847
>>
>> Added:
>>     compiler-rt/trunk/test/profile/instrprof-value-merge.c
>> Modified:
>>     compiler-rt/trunk/lib/profile/InstrProfiling.h
>>     compiler-rt/trunk/lib/profile/InstrProfilingFile.c
>>     compiler-rt/trunk/lib/profile/InstrProfilingMerge.c
>>     compiler-rt/trunk/lib/profile/InstrProfilingMergeFile.c
>>     compiler-rt/trunk/lib/profile/InstrProfilingPort.h
>>     compiler-rt/trunk/lib/profile/InstrProfilingValue.c
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfiling.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfiling.h?rev=328987&r1=328986&r2=328987&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfiling.h (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfiling.h Mon Apr  2 09:57:00
>> 2018
>> @@ -106,6 +106,10 @@ void INSTR_PROF_VALUE_PROF_FUNC(
>>  #include "InstrProfData.inc"
>>      );
>>
>> +void __llvm_profile_instrument_target_value(uint64_t TargetValue, void
>> *Data,
>> +                                            uint32_t CounterIndex,
>> +                                            uint64_t CounterValue);
>> +
>>  /*!
>>   * \brief Write instrumentation data to the current file.
>>   *
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingFile.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingFile.c?rev=328987&r1=328986&r2=328987&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfilingFile.c (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfilingFile.c Mon Apr  2
>> 09:57:00 2018
>> @@ -183,8 +183,12 @@ static int doProfileMerging(FILE *Profil
>>
>>    /* Now start merging */
>>    __llvm_profile_merge_from_buffer(ProfileBuffer, ProfileFileSize);
>> -  (void)munmap(ProfileBuffer, ProfileFileSize);
>>
>> +  // Truncate the file in case merging of value profile did not happend
>> to
>> +  // prevent from leaving garbage data at the end of the profile file.
>> +  ftruncate(fileno(ProfileFile), __llvm_profile_get_size_for_buffer());
>> +
>> +  (void)munmap(ProfileBuffer, ProfileFileSize);
>>    *MergeDone = 1;
>>
>>    return 0;
>> @@ -234,6 +238,7 @@ static int writeFile(const char *OutputN
>>    FILE *OutputFile;
>>
>>    int MergeDone = 0;
>> +  VPMergeHook = &lprofMergeValueProfData;
>>    if (!doMerging())
>>      OutputFile = fopen(OutputName, "ab");
>>    else
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingMerge.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingMerge.c?rev=328987&r1=328986&r2=328987&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfilingMerge.c (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfilingMerge.c Mon Apr  2
>> 09:57:00 2018
>> @@ -17,8 +17,9 @@
>>  #define INSTR_PROF_VALUE_PROF_DATA
>>  #include "InstrProfData.inc"
>>
>> -COMPILER_RT_WEAK void (*VPMergeHook)(ValueProfData *,
>> -                                     __llvm_profile_data *) = NULL;
>> +COMPILER_RT_VISIBILITY
>> +void (*VPMergeHook)(ValueProfData *, __llvm_profile_data *);
>> +
>>  COMPILER_RT_VISIBILITY
>>  uint64_t lprofGetLoadModuleSignature() {
>>    /* A very fast way to compute a module signature.  */
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingMergeFile.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingMergeFile.c?rev=328987&r1=328986&r2=328987&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfilingMergeFile.c (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfilingMergeFile.c Mon Apr  2
>> 09:57:00 2018
>> @@ -17,24 +17,24 @@
>>  #define INSTR_PROF_VALUE_PROF_DATA
>>  #include "InstrProfData.inc"
>>
>> -void (*VPMergeHook)(ValueProfData *,
>> -                    __llvm_profile_data *) = &lprofMergeValueProfData;
>> -
>>  /* Merge value profile data pointed to by SrcValueProfData into
>>   * in-memory profile counters pointed by to DstData.  */
>>  void lprofMergeValueProfData(ValueProfData *SrcValueProfData,
>>                               __llvm_profile_data *DstData) {
>> -  unsigned I, S, V, C;
>> +  unsigned I, S, V, DstIndex = 0;
>>    InstrProfValueData *VData;
>>    ValueProfRecord *VR = getFirstValueProfRecord(SrcValueProfData);
>>    for (I = 0; I < SrcValueProfData->NumValueKinds; I++) {
>>      VData = getValueProfRecordValueData(VR);
>> +    unsigned SrcIndex = 0;
>>      for (S = 0; S < VR->NumValueSites; S++) {
>>        uint8_t NV = VR->SiteCountArray[S];
>>        for (V = 0; V < NV; V++) {
>> -        for (C = 0; C < VData[V].Count; C++)
>> -          __llvm_profile_instrument_target(VData[V].Value, DstData, S);
>> +        __llvm_profile_instrument_target_value(VData[SrcIndex].Value,
>> DstData,
>> +                                               DstIndex,
>> VData[SrcIndex].Count);
>> +        ++SrcIndex;
>>        }
>> +      ++DstIndex;
>>      }
>>      VR = getValueProfRecordNext(VR);
>>    }
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingPort.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingPort.h?rev=328987&r1=328986&r2=328987&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfilingPort.h (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfilingPort.h Mon Apr  2
>> 09:57:00 2018
>> @@ -22,12 +22,14 @@
>>  #define COMPILER_RT_ALLOCA _alloca
>>  /* Need to include <stdio.h> and <io.h> */
>>  #define COMPILER_RT_FTRUNCATE(f,l) _chsize(_fileno(f),l)
>> +#define COMPILER_RT_ALWAYS_INLINE __forceinline
>>  #elif __GNUC__
>>  #define COMPILER_RT_ALIGNAS(x) __attribute__((aligned(x)))
>>  #define COMPILER_RT_VISIBILITY __attribute__((visibility("hidden")))
>>  #define COMPILER_RT_WEAK __attribute__((weak))
>>  #define COMPILER_RT_ALLOCA __builtin_alloca
>>  #define COMPILER_RT_FTRUNCATE(f,l) ftruncate(fileno(f),l)
>> +#define COMPILER_RT_ALWAYS_INLINE inline __attribute((always_inline))
>>  #endif
>>
>>  #if defined(__APPLE__)
>>
>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingValue.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingValue.c?rev=328987&r1=328986&r2=328987&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/profile/InstrProfilingValue.c (original)
>> +++ compiler-rt/trunk/lib/profile/InstrProfilingValue.c Mon Apr  2
>> 09:57:00 2018
>> @@ -132,13 +132,14 @@ static ValueProfNode *allocateOneNode(__
>>    return Node;
>>  }
>>
>> -COMPILER_RT_VISIBILITY void
>> -__llvm_profile_instrument_target(uint64_t TargetValue, void *Data,
>> -                                 uint32_t CounterIndex) {
>> +static COMPILER_RT_ALWAYS_INLINE void
>> +instrumentTargetValueImpl(uint64_t TargetValue, void *Data,
>> +                          uint32_t CounterIndex, uint64_t CountValue) {
>>    __llvm_profile_data *PData = (__llvm_profile_data *)Data;
>>    if (!PData)
>>      return;
>> -
>> +  if (!CountValue)
>> +    return;
>>    if (!PData->Values) {
>>      if (!allocateValueProfileCounters(PData))
>>        return;
>> @@ -153,7 +154,7 @@ __llvm_profile_instrument_target(uint64_
>>    uint8_t VDataCount = 0;
>>    while (CurVNode) {
>>      if (TargetValue == CurVNode->Value) {
>> -      CurVNode->Count++;
>> +      CurVNode->Count += CountValue;
>>        return;
>>      }
>>      if (CurVNode->Count < MinCount) {
>> @@ -194,11 +195,13 @@ __llvm_profile_instrument_target(uint64_
>>       * the runtime can wipe out more than one lowest count entries
>>       * to give space for hot targets.
>>       */
>> -    if (!MinCountVNode->Count || !(--MinCountVNode->Count)) {
>> +    if (MinCountVNode->Count <= CountValue) {
>>        CurVNode = MinCountVNode;
>>        CurVNode->Value = TargetValue;
>> -      CurVNode->Count++;
>> -    }
>> +      CurVNode->Count = CountValue;
>> +    } else
>> +      MinCountVNode->Count -= CountValue;
>> +
>>      return;
>>    }
>>
>> @@ -206,7 +209,7 @@ __llvm_profile_instrument_target(uint64_
>>    if (!CurVNode)
>>      return;
>>    CurVNode->Value = TargetValue;
>> -  CurVNode->Count++;
>> +  CurVNode->Count += CountValue;
>>
>>    uint32_t Success = 0;
>>    if (!ValueCounters[CounterIndex])
>> @@ -221,6 +224,18 @@ __llvm_profile_instrument_target(uint64_
>>    }
>>  }
>>
>> +COMPILER_RT_VISIBILITY void
>> +__llvm_profile_instrument_target(uint64_t TargetValue, void *Data,
>> +                                 uint32_t CounterIndex) {
>> +  instrumentTargetValueImpl(TargetValue, Data, CounterIndex, 1);
>> +}
>> +COMPILER_RT_VISIBILITY void
>> +__llvm_profile_instrument_target_value(uint64_t TargetValue, void *Data,
>> +                                       uint32_t CounterIndex,
>> +                                       uint64_t CountValue) {
>> +  instrumentTargetValueImpl(TargetValue, Data, CounterIndex, CountValue);
>> +}
>> +
>>  /*
>>   * The target values are partitioned into multiple regions/ranges. There
>> is one
>>   * contiguous region which is precise -- every value in the range is
>> tracked
>>
>> Added: compiler-rt/trunk/test/profile/instrprof-value-merge.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/profile/instrprof-value-merge.c?rev=328987&view=auto
>>
>> ==============================================================================
>> --- compiler-rt/trunk/test/profile/instrprof-value-merge.c (added)
>> +++ compiler-rt/trunk/test/profile/instrprof-value-merge.c Mon Apr  2
>> 09:57:00 2018
>> @@ -0,0 +1,79 @@
>> +// RUN: %clang_pgogen -o %t -O3 %s
>> +// RUN: rm -rf %t.profdir
>> +// RUN: env LLVM_PROFILE_FILE=%t.profdir/default_%m.profraw %run %t
>> +// RUN: env LLVM_PROFILE_FILE=%t.profdir/default_%m.profraw %run %t
>> +// RUN: env LLVM_PROFILE_FILE=%t.profdir/default_%m.profraw %run %t
>> +// RUN: env LLVM_PROFILE_FILE=%t.profdir/default_%m.profraw %run %t 1
>> +// RUN: env LLVM_PROFILE_FILE=%t.profdir/default_%m.profraw %run %t 1
>> +// RUN: llvm-profdata show -counts -function=main -ic-targets
>> -memop-sizes %t.profdir/default_*.profraw | FileCheck %s
>> +
>> +#include <string.h>
>> +
>> +void (*f0)();
>> +void (*f1)();
>> +void (*f2)();
>> +
>> +char dst[200];
>> +char src[200];
>> +volatile int n;
>> +
>> +__attribute__((noinline)) void foo() {}
>> +
>> +__attribute__((noinline)) void bar() {
>> +  f0 = foo;
>> +  f1 = foo;
>> +  f2 = foo;
>> +  n = 4;
>> +}
>> +int main(int argc, char *argv[]) {
>> +  int i;
>> +  bar();
>> +  if (argc == 1) {
>> +    f0();
>> +    for (i = 0; i < 9; i++)
>> +      f1();
>> +    for (i = 0; i < 99; i++)
>> +      f2();
>> +  } else {
>> +    memcpy((void *)dst, (void *)src, n);
>> +    for (i = 0; i < 6; i++)
>> +      memcpy((void *)(dst + 2), (void *)src, n + 1);
>> +    for (i = 0; i < 66; i++)
>> +      memcpy((void *)(dst + 9), (void *)src, n + 2);
>> +  }
>> +}
>> +
>> +// CHECK: Counters:
>> +// CHECK:   main:
>> +// CHECK:     Hash: 0x00030012a7ab6e87
>> +// CHECK:     Counters: 6
>> +// CHECK:     Indirect Call Site Count: 3
>> +// CHECK:     Number of Memory Intrinsics Calls: 3
>> +// CHECK:     Block counts: [27, 297, 12, 132, 3, 2]
>> +// CHECK:     Indirect Target Results:
>> +// CHECK:         [ 0, foo, 3 ]
>> +// CHECK:         [ 1, foo, 27 ]
>> +// CHECK:         [ 2, foo, 297 ]
>> +// CHECK:     Memory Intrinsic Size Results:
>> +// CHECK:         [ 0, 4, 2 ]
>> +// CHECK:         [ 1, 5, 12 ]
>> +// CHECK:         [ 2, 6, 132 ]
>> +// CHECK: Instrumentation level: IR
>> +// CHECK: Functions shown: 1
>> +// CHECK: Total functions: 3
>> +// CHECK: Maximum function count: 327
>> +// CHECK: Maximum internal block count: 297
>> +// CHECK: Statistics for indirect call sites profile:
>> +// CHECK:   Total number of sites: 3
>> +// CHECK:   Total number of sites with values: 3
>> +// CHECK:   Total number of profiled values: 3
>> +// CHECK:   Value sites histogram:
>> +// CHECK:         NumTargets, SiteCount
>> +// CHECK:         1, 3
>> +// CHECK: Statistics for memory intrinsic calls sizes profile:
>> +// CHECK:   Total number of sites: 3
>> +// CHECK:   Total number of sites with values: 3
>> +// CHECK:   Total number of profiled values: 3
>> +// CHECK:   Value sites histogram:
>> +// CHECK:         NumTargets, SiteCount
>> +// CHECK:         1, 3
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180402/f79fcb92/attachment.html>


More information about the llvm-commits mailing list