[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 20:23:20 PDT 2016
On Thu, Jun 23, 2016 at 8:15 PM, Vedant Kumar <vsk at apple.com> wrote:
> Yeah, this test will break if %run attempts remote execution.
>
> Here's an idea for a less brittle (and way less ambitious) test:
>
> ```
> // test-hostname-expansion.c
>
> // RUN: %cc_profgen test-hostname-expansion.c -o %t.exe
> // RUN: LLVM_PROFILE_FILE="%t.%h.profraw" %run %t.exe "%t.%h.profraw"
> // RUN: rm %t.`uname -n`.profraw
>
> // REQUIRES: shell
>
> int main(int argc, char **argv) {
> __llvm_profile_set_filename(argv[1]);
> return 0;
> }
> ```
>
> I.e, give up on the idea of actually testing remote execution, and just
> check that the hostname expansion is faithful.
>
Faithful to what? %h is fundamentally expanded on the target. `uname -n`
occurs on the host. They are not necessarily related in any way.
-- Sean Silva
>
> Unrelated to Sean's point: why is "%clang_profuse=%t.profdata" in the
> test? I don't think it adds anything.
>
> WDYT?
>
> thanks,
> vedant
>
> > On Jun 23, 2016, at 7:48 PM, Sean Silva <chisophugis at gmail.com> 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
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160623/68161ea3/attachment.html>
More information about the llvm-commits
mailing list