[compiler-rt] r259272 - [profile] Support hostname expansion in LLVM_PROFILE_FILE
Sean Silva via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 23 22:00:14 PDT 2016
On Thu, Jun 23, 2016 at 9:15 PM, Xinliang David Li <xinliangli at gmail.com>
wrote:
> Does the patch look fine?
>
> David
>
> Index: instrprof-hostname.c
> ===================================================================
> --- instrprof-hostname.c (revision 273505)
> +++ instrprof-hostname.c (working copy)
> @@ -1,6 +1,7 @@
> // RUN: %clang_profgen -o %t -O3 %s
> // RUN: env LLVM_PROFILE_FILE=%h.%t-%h.profraw_%h %run %t
> -// RUN: llvm-profdata merge -o %t.profdata `uname -n`.%t-`uname
> -n`.profraw_`uname -n`
> +// RUN: %run uname -n > %t.n
> +// RUN: llvm-profdata merge -o %t.profdata `cat %t.n`.%t-`cat
> %t.n`.profraw_`cat %t.n`
> // RUN: %clang_profuse=%t.profdata -o - -S -emit-llvm %s | FileCheck %s
> // REQUIRES: shell
>
This LGTM for general UNIX-y target devices (nice solution!). Unfortunately
PS4 does not have any of the standard UNIX utilities available for running
on the target. In fact, there is no shell on the PS4 target, no /bin, no
/usr/bin, etc. :(
-- Sean Silva
>
>
> On Thu, Jun 23, 2016 at 8:51 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>>
>>
>> On Thu, Jun 23, 2016 at 8:30 PM, Xinliang David Li <xinliangli at gmail.com>
>> wrote:
>>
>>> The test case can be modified to dump hostname to a file. At the host
>>> side, re-create the file name and make sure it exists. Does that work?
>>>
>>
>> That would work I think; I don't have an environment where I can
>> effectively fix/test it though. On PS4 we don't support %h expansion (it
>> would bring in a heavyweight dependency) so I have this XFAIL'd locally (it
>> is not urgent to fix for us).
>>
>> The `REQUIRES: shell` had been masking the issue for us until recently
>> and so I'm just now commenting on this patch.
>>
>> -- Sean Silva
>>
>>
>>>
>>> David
>>>
>>> On Thu, Jun 23, 2016 at 7:48 PM, Sean Silva via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Hi Vedant, Daniel,
>>>>
>>>> The test case in this patch is fundamentally flawed: it runs `uname -n`
>>>> on the host, while `%h` expands on the target. No relation between them can
>>>> be assumed.
>>>>
>>>> Unfortunately, I don't really have a concrete suggestion for improving
>>>> the test case: we would have to have some sort of special knowledge of what
>>>> %run is actually configured to do in order to "verify" anything about the
>>>> filename.
>>>>
>>>> -- Sean Silva
>>>>
>>>> On Fri, Jan 29, 2016 at 3:52 PM, Vedant Kumar via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: vedantk
>>>>> Date: Fri Jan 29 17:52:11 2016
>>>>> New Revision: 259272
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=259272&view=rev
>>>>> Log:
>>>>> [profile] Support hostname expansion in LLVM_PROFILE_FILE
>>>>>
>>>>> This patch adds support for expanding "%h" out to the machine hostname
>>>>> in the LLVM_PROFILE_FILE environment variable.
>>>>>
>>>>> Patch by Daniel Waters!
>>>>>
>>>>> Differential Revision: http://reviews.llvm.org/D16371
>>>>>
>>>>> Added:
>>>>> compiler-rt/trunk/test/profile/instrprof-hostname.c
>>>>> Modified:
>>>>> compiler-rt/trunk/lib/profile/InstrProfilingFile.c
>>>>> compiler-rt/trunk/lib/profile/InstrProfilingPort.h
>>>>>
>>>>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingFile.c
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingFile.c?rev=259272&r1=259271&r2=259272&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- compiler-rt/trunk/lib/profile/InstrProfilingFile.c (original)
>>>>> +++ compiler-rt/trunk/lib/profile/InstrProfilingFile.c Fri Jan 29
>>>>> 17:52:11 2016
>>>>> @@ -14,9 +14,22 @@
>>>>> #include <stdio.h>
>>>>> #include <stdlib.h>
>>>>> #include <string.h>
>>>>> +#ifdef COMPILER_RT_HAS_UNAME
>>>>> +#include <sys/utsname.h>
>>>>> +#endif
>>>>>
>>>>> #define UNCONST(ptr) ((void *)(uintptr_t)(ptr))
>>>>>
>>>>> +#ifdef COMPILER_RT_HAS_UNAME
>>>>> +int GetHostName(char *Name, int Len) {
>>>>> + struct utsname N;
>>>>> + int R;
>>>>> + if (!(R = uname(&N)))
>>>>> + strncpy(Name, N.nodename, Len);
>>>>> + return R;
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>> /* Return 1 if there is an error, otherwise return 0. */
>>>>> static uint32_t fileWriter(ProfDataIOVec *IOVecs, uint32_t NumIOVecs,
>>>>> void **WriterCtx) {
>>>>> @@ -114,9 +127,10 @@ int getpid(void);
>>>>> static int setFilenamePossiblyWithPid(const char *Filename) {
>>>>> #define MAX_PID_SIZE 16
>>>>> char PidChars[MAX_PID_SIZE] = {0};
>>>>> - int NumPids = 0, PidLength = 0;
>>>>> + int NumPids = 0, PidLength = 0, NumHosts = 0, HostNameLength = 0;
>>>>> char *Allocated;
>>>>> int I, J;
>>>>> + char Hostname[COMPILER_RT_MAX_HOSTLEN];
>>>>>
>>>>> /* Reset filename on NULL, except with env var which is checked by
>>>>> caller. */
>>>>> if (!Filename) {
>>>>> @@ -126,19 +140,29 @@ static int setFilenamePossiblyWithPid(co
>>>>>
>>>>> /* Check the filename for "%p", which indicates a pid-substitution.
>>>>> */
>>>>> for (I = 0; Filename[I]; ++I)
>>>>> - if (Filename[I] == '%' && Filename[++I] == 'p')
>>>>> - if (!NumPids++) {
>>>>> - PidLength = snprintf(PidChars, MAX_PID_SIZE, "%d", getpid());
>>>>> - if (PidLength <= 0)
>>>>> - return -1;
>>>>> + if (Filename[I] == '%') {
>>>>> + if (Filename[++I] == 'p') {
>>>>> + if (!NumPids++) {
>>>>> + PidLength = snprintf(PidChars, MAX_PID_SIZE, "%d",
>>>>> getpid());
>>>>> + if (PidLength <= 0)
>>>>> + return -1;
>>>>> + }
>>>>> + } else if (Filename[I] == 'h') {
>>>>> + if (!NumHosts++)
>>>>> + if (COMPILER_RT_GETHOSTNAME(Hostname,
>>>>> COMPILER_RT_MAX_HOSTLEN))
>>>>> + return -1;
>>>>> + HostNameLength = strlen(Hostname);
>>>>> }
>>>>> - if (!NumPids) {
>>>>> + }
>>>>> +
>>>>> + if (!(NumPids || NumHosts)) {
>>>>> setFilename(Filename, 0);
>>>>> return 0;
>>>>> }
>>>>>
>>>>> /* Allocate enough space for the substituted filename. */
>>>>> - Allocated = malloc(I + NumPids*(PidLength - 2) + 1);
>>>>> + Allocated = malloc(I + NumPids*(PidLength - 2) +
>>>>> + NumHosts*(HostNameLength - 2) + 1);
>>>>> if (!Allocated)
>>>>> return -1;
>>>>>
>>>>> @@ -149,6 +173,10 @@ static int setFilenamePossiblyWithPid(co
>>>>> memcpy(Allocated + J, PidChars, PidLength);
>>>>> J += PidLength;
>>>>> }
>>>>> + else if (Filename[I] == 'h') {
>>>>> + memcpy(Allocated + J, Hostname, HostNameLength);
>>>>> + J += HostNameLength;
>>>>> + }
>>>>> /* Drop any unknown substitutions. */
>>>>> } else
>>>>> Allocated[J++] = Filename[I];
>>>>>
>>>>> Modified: compiler-rt/trunk/lib/profile/InstrProfilingPort.h
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingPort.h?rev=259272&r1=259271&r2=259272&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- compiler-rt/trunk/lib/profile/InstrProfilingPort.h (original)
>>>>> +++ compiler-rt/trunk/lib/profile/InstrProfilingPort.h Fri Jan 29
>>>>> 17:52:11 2016
>>>>> @@ -22,6 +22,14 @@
>>>>>
>>>>> #define COMPILER_RT_SECTION(Sect) __attribute__((section(Sect)))
>>>>>
>>>>> +#define COMPILER_RT_MAX_HOSTLEN 128
>>>>> +#ifdef _MSC_VER
>>>>> +#define COMPILER_RT_GETHOSTNAME(Name, Len) gethostname(Name, Len)
>>>>> +#else
>>>>> +#define COMPILER_RT_GETHOSTNAME(Name, Len) GetHostName(Name, Len)
>>>>> +#define COMPILER_RT_HAS_UNAME 1
>>>>> +#endif
>>>>> +
>>>>> #if COMPILER_RT_HAS_ATOMICS == 1
>>>>> #ifdef _MSC_VER
>>>>> #include <windows.h>
>>>>>
>>>>> Added: compiler-rt/trunk/test/profile/instrprof-hostname.c
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/profile/instrprof-hostname.c?rev=259272&view=auto
>>>>>
>>>>> ==============================================================================
>>>>> --- compiler-rt/trunk/test/profile/instrprof-hostname.c (added)
>>>>> +++ compiler-rt/trunk/test/profile/instrprof-hostname.c Fri Jan 29
>>>>> 17:52:11 2016
>>>>> @@ -0,0 +1,13 @@
>>>>> +// RUN: %clang_profgen -o %t -O3 %s
>>>>> +// RUN: env LLVM_PROFILE_FILE=%h.%t-%h.profraw_%h %run %t
>>>>> +// RUN: llvm-profdata merge -o %t.profdata `uname -n`.%t-`uname
>>>>> -n`.profraw_`uname -n`
>>>>> +// RUN: %clang_profuse=%t.profdata -o - -S -emit-llvm %s | FileCheck
>>>>> %s
>>>>> +// REQUIRES: shell
>>>>> +
>>>>> +int main(int argc, const char *argv[]) {
>>>>> + // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof
>>>>> ![[PD1:[0-9]+]]
>>>>> + if (argc > 2)
>>>>> + return 1;
>>>>> + return 0;
>>>>> +}
>>>>> +// CHECK: ![[PD1]] = !{!"branch_weights", i32 1, i32 2}
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> 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/20160623/0133b0dc/attachment.html>
More information about the llvm-commits
mailing list