<div dir="ltr">Good catch! Given this is i386 darwin specific failure, I should have caught it ..<div><br></div><div>David</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 9, 2016 at 12:25 PM, Vedant Kumar 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">The issue is this line:<br>
<br>
(uint64_t)(__llvm_profile_end_data() - __llvm_profile_begin_data())<br>
<br>
We introduced __llvm_profile_get_data_size() to handle the Darwin<br>
linker's aggressive approach to laying out the prof data section. So we<br>
would need to replace it with:<br>
<br>
__llvm_profile_get_data_size(__llvm_profile_begin_data(),<br>
__llvm_profile_end_data())<br>
<br>
vedant<br>
<span class=""><br>
<br>
> On Mar 9, 2016, at 12:17 PM, Xinliang David Li <<a href="mailto:davidxl@google.com">davidxl@google.com</a>> wrote:<br>
><br>
><br>
><br>
> On Wed, Mar 9, 2016 at 12:14 PM, Vedant Kumar <<a href="mailto:vsk@apple.com">vsk@apple.com</a>> wrote:<br>
> Hi David,<br>
><br>
> I reproduced the issue. Do you mind reverting until I find a fix?<br>
><br>
> Sure.<br>
><br>
><br>
> As a side note: Please CC me for time-sensitive emails, I'll be able to get to it much faster :).<br>
><br>
><br>
> I think I cced you :)<br>
<br>
</span>Yes you did, my mistake!<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> David<br>
><br>
><br>
> vedant<br>
><br>
><br>
> > On Mar 9, 2016, at 9:20 AM, Xinliang David Li <<a href="mailto:davidxl@google.com">davidxl@google.com</a>> wrote:<br>
> ><br>
> > Vedant, I could not reproduce the problem locally. Can you help with debugging and let me know where the failure is?<br>
> ><br>
> > thanks,<br>
> ><br>
> > David<br>
> ><br>
> > On Wed, Mar 9, 2016 at 2:38 AM, Kuba Brecka <<a href="mailto:jbrecka@apple.com">jbrecka@apple.com</a>> wrote:<br>
> > Hi David,<br>
> ><br>
> > since this commit, the OS X buildbot has been failing the Profile-i386/instrprof-merge-match.test lit test: <a href="http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA_check/10556/console" rel="noreferrer" target="_blank">http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA_check/10556/console</a><br>
> ><br>
> > Could you take a look at it?<br>
> ><br>
> > Thanks,<br>
> > Kuba<br>
> ><br>
> > > On 04 Mar 2016, at 19:58, Xinliang David Li via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> > ><br>
> > > Author: davidxl<br>
> > > Date: Fri Mar 4 12:58:30 2016<br>
> > > New Revision: 262736<br>
> > ><br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=262736&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=262736&view=rev</a><br>
> > > Log:<br>
> > > [PGO] Add API to check compatibility of profile data in buffer<br>
> > ><br>
> > > This is needed by client which uses in-process merge API.<br>
> > ><br>
> > ><br>
> > > Added:<br>
> > > compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match-lib.c<br>
> > > compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match.c<br>
> > > compiler-rt/trunk/test/profile/instrprof-merge-match.test<br>
> > > Modified:<br>
> > > compiler-rt/trunk/lib/profile/InstrProfiling.h<br>
> > > compiler-rt/trunk/lib/profile/InstrProfilingMerge.c<br>
> > ><br>
> > > Modified: compiler-rt/trunk/lib/profile/InstrProfiling.h<br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfiling.h?rev=262736&r1=262735&r2=262736&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfiling.h?rev=262736&r1=262735&r2=262736&view=diff</a><br>
> > > ==============================================================================<br>
> > > --- compiler-rt/trunk/lib/profile/InstrProfiling.h (original)<br>
> > > +++ compiler-rt/trunk/lib/profile/InstrProfiling.h Fri Mar 4 12:58:30 2016<br>
> > > @@ -63,7 +63,9 @@ uint64_t *__llvm_profile_end_counters(vo<br>
> > > void __llvm_profile_reset_counters(void);<br>
> > ><br>
> > > /*!<br>
> > > - * \brief Read profile data form buffer and merge with<br>
> > > + * \brief Merge profile data from buffer.<br>
> > > + *<br>
> > > + * Read profile data form buffer \p Profile and merge with<br>
> > > * in-process profile counters. The client is expected to<br>
> > > * have checked or already knows the profile data in the<br>
> > > * buffer matches the in-process counter structure before<br>
> > > @@ -71,6 +73,16 @@ void __llvm_profile_reset_counters(void)<br>
> > > */<br>
> > > void __llvm_profile_merge_from_buffer(const char *Profile, uint64_t Size);<br>
> > ><br>
> > > +/*! \brief Check if profile in buffer matches the current binary.<br>
> > > + *<br>
> > > + * Returns 0 (success) if the profile data in buffer \p Profile with size<br>
> > > + * \p Size was generated by the same binary and therefore matches<br>
> > > + * structurally the in-process counters. If the profile data in buffer is<br>
> > > + * not compatible, the interface returns 1 (failure).<br>
> > > + */<br>
> > > +int __llvm_profile_check_compatibility(const char *Profile,<br>
> > > + uint64_t Size);<br>
> > > +<br>
> > > /*!<br>
> > > * \brief Counts the number of times a target value is seen.<br>
> > > *<br>
> > ><br>
> > > Modified: compiler-rt/trunk/lib/profile/InstrProfilingMerge.c<br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingMerge.c?rev=262736&r1=262735&r2=262736&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingMerge.c?rev=262736&r1=262735&r2=262736&view=diff</a><br>
> > > ==============================================================================<br>
> > > --- compiler-rt/trunk/lib/profile/InstrProfilingMerge.c (original)<br>
> > > +++ compiler-rt/trunk/lib/profile/InstrProfilingMerge.c Fri Mar 4 12:58:30 2016<br>
> > > @@ -20,6 +20,48 @@<br>
> > > COMPILER_RT_WEAK void (*VPMergeHook)(ValueProfData *,<br>
> > > __llvm_profile_data *) = NULL;<br>
> > ><br>
> > > +/* Returns 1 if profile is not structurally compatible. */<br>
> > > +COMPILER_RT_VISIBILITY<br>
> > > +int __llvm_profile_check_compatibility(const char *ProfileData,<br>
> > > + uint64_t ProfileSize) {<br>
> > > + /* Check profile header only for now */<br>
> > > + __llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;<br>
> > > + __llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;<br>
> > > + SrcDataStart =<br>
> > > + (__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header));<br>
> > > + SrcDataEnd = SrcDataStart + Header->DataSize;<br>
> > > +<br>
> > > + /* Check the header first. */<br>
> > > + if (Header->Magic != __llvm_profile_get_magic() ||<br>
> > > + Header->Version != __llvm_profile_get_version() ||<br>
> > > + Header->DataSize !=<br>
> > > + (uint64_t)(__llvm_profile_end_data() - __llvm_profile_begin_data()) ||<br>
> > > + Header->CountersSize != (uint64_t)(__llvm_profile_end_counters() -<br>
> > > + __llvm_profile_begin_counters()) ||<br>
> > > + Header->NamesSize != (uint64_t)(__llvm_profile_end_names() -<br>
> > > + __llvm_profile_begin_names()) ||<br>
> > > + Header->ValueKindLast != IPVK_Last)<br>
> > > + return 1;<br>
> > > +<br>
> > > + if (ProfileSize < sizeof(__llvm_profile_header) +<br>
> > > + Header->DataSize * sizeof(__llvm_profile_data) +<br>
> > > + Header->NamesSize + Header->CountersSize +<br>
> > > + Header->ValueDataSize)<br>
> > > + return 1;<br>
> > > +<br>
> > > + for (SrcData = SrcDataStart,<br>
> > > + DstData = (__llvm_profile_data *)__llvm_profile_begin_data();<br>
> > > + SrcData < SrcDataEnd; ++SrcData, ++DstData) {<br>
> > > + if (SrcData->NameRef != DstData->NameRef ||<br>
> > > + SrcData->FuncHash != DstData->FuncHash ||<br>
> > > + SrcData->NumCounters != DstData->NumCounters)<br>
> > > + return 1;<br>
> > > + }<br>
> > > +<br>
> > > + /* Matched! */<br>
> > > + return 0;<br>
> > > +}<br>
> > > +<br>
> > > COMPILER_RT_VISIBILITY<br>
> > > void __llvm_profile_merge_from_buffer(const char *ProfileData,<br>
> > > uint64_t ProfileSize) {<br>
> > ><br>
> > > Added: compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match-lib.c<br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match-lib.c?rev=262736&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match-lib.c?rev=262736&view=auto</a><br>
> > > ==============================================================================<br>
> > > --- compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match-lib.c (added)<br>
> > > +++ compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match-lib.c Fri Mar 4 12:58:30 2016<br>
> > > @@ -0,0 +1,39 @@<br>
> > > +#include <stdint.h><br>
> > > +#include <stdio.h><br>
> > > +#include <stdlib.h><br>
> > > +<br>
> > > +int __llvm_profile_runtime = 0;<br>
> > > +uint64_t __llvm_profile_get_size_for_buffer(void);<br>
> > > +int __llvm_profile_write_buffer(char *);<br>
> > > +void __llvm_profile_reset_counters(void);<br>
> > > +int __llvm_profile_check_compatibility(const char *, uint64_t);<br>
> > > +<br>
> > > +int gg = 0;<br>
> > > +void bar(char c) {<br>
> > > + if (c == '1')<br>
> > > + gg++;<br>
> > > + else<br>
> > > + gg--;<br>
> > > +}<br>
> > > +<br>
> > > +/* Returns 0 (size) when an error occurs. */<br>
> > > +uint64_t libEntry(char *Buffer, uint64_t MaxSize) {<br>
> > > +<br>
> > > + uint64_t Size = __llvm_profile_get_size_for_buffer();<br>
> > > + if (Size > MaxSize)<br>
> > > + return 1;<br>
> > > +<br>
> > > + __llvm_profile_reset_counters();<br>
> > > +<br>
> > > + bar('1');<br>
> > > +<br>
> > > + if (__llvm_profile_write_buffer(Buffer))<br>
> > > + return 0;<br>
> > > +<br>
> > > + /* Now check compatibility. Should return 0. */<br>
> > > + if (__llvm_profile_check_compatibility(Buffer, Size))<br>
> > > + return 0;<br>
> > > +<br>
> > > + return Size;<br>
> > > +}<br>
> > > +<br>
> > ><br>
> > > Added: compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match.c<br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match.c?rev=262736&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match.c?rev=262736&view=auto</a><br>
> > > ==============================================================================<br>
> > > --- compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match.c (added)<br>
> > > +++ compiler-rt/trunk/test/profile/Inputs/instrprof-merge-match.c Fri Mar 4 12:58:30 2016<br>
> > > @@ -0,0 +1,54 @@<br>
> > > +#include <stdint.h><br>
> > > +#include <stdlib.h><br>
> > > +#include <string.h><br>
> > > +<br>
> > > +int __llvm_profile_runtime = 0;<br>
> > > +uint64_t __llvm_profile_get_size_for_buffer(void);<br>
> > > +int __llvm_profile_write_buffer(char *);<br>
> > > +void __llvm_profile_reset_counters(void);<br>
> > > +int __llvm_profile_check_compatibility(const char *, uint64_t);<br>
> > > +<br>
> > > +int g = 0;<br>
> > > +void foo(char c) {<br>
> > > + if (c == '1')<br>
> > > + g++;<br>
> > > + else<br>
> > > + g--;<br>
> > > +}<br>
> > > +<br>
> > > +extern uint64_t libEntry(char *Buffer, uint64_t MaxSize);<br>
> > > +<br>
> > > +int main(int argc, const char *argv[]) {<br>
> > > + const uint64_t MaxSize = 10000;<br>
> > > + static char Buffer[MaxSize];<br>
> > > +<br>
> > > + uint64_t Size = __llvm_profile_get_size_for_buffer();<br>
> > > + if (Size > MaxSize)<br>
> > > + return 1;<br>
> > > +<br>
> > > + __llvm_profile_reset_counters();<br>
> > > + foo('0');<br>
> > > +<br>
> > > + if (__llvm_profile_write_buffer(Buffer))<br>
> > > + return 1;<br>
> > > +<br>
> > > + /* Now check compatibility. Should return 0. */<br>
> > > + if (__llvm_profile_check_compatibility(Buffer, Size))<br>
> > > + return 1;<br>
> > > +<br>
> > > + /* Clear the buffer. */<br>
> > > + memset(Buffer, 0, MaxSize);<br>
> > > +<br>
> > > + /* Collect profile from shared library. */<br>
> > > + Size = libEntry(Buffer, MaxSize);<br>
> > > +<br>
> > > + if (!Size)<br>
> > > + return 1;<br>
> > > +<br>
> > > + /* Shared library's profile should not match main executable's. */<br>
> > > + if (!__llvm_profile_check_compatibility(Buffer, Size))<br>
> > > + return 1;<br>
> > > +<br>
> > > + return 0;<br>
> > > +}<br>
> > > +<br>
> > ><br>
> > > Added: compiler-rt/trunk/test/profile/instrprof-merge-match.test<br>
> > > URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/profile/instrprof-merge-match.test?rev=262736&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/profile/instrprof-merge-match.test?rev=262736&view=auto</a><br>
> > > ==============================================================================<br>
> > > --- compiler-rt/trunk/test/profile/instrprof-merge-match.test (added)<br>
> > > +++ compiler-rt/trunk/test/profile/instrprof-merge-match.test Fri Mar 4 12:58:30 2016<br>
> > > @@ -0,0 +1,5 @@<br>
> > > +// RUN: mkdir -p %t.d<br>
> > > +// RUN: %clang_profgen -o %t.d/libt.so -fPIC -shared %S/Inputs/instrprof-merge-match-lib.c<br>
> > > +// RUN: %clang_profgen -o %t -L %t.d -rpath %t.d %S/Inputs/instrprof-merge-match.c -lt<br>
> > > +// RUN: %run %t<br>
> > > +<br>
> > ><br>
> > ><br>
> > > _______________________________________________<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/mailman/listinfo/llvm-commits</a><br>
> ><br>
> ><br>
><br>
><br>
<br>
_______________________________________________<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/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>