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