[PATCH] D16371: [compiler-rt/profile] Added Hostname to .profdata file

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "Xinliang David Li" <davidxl at google.com>
> To: reviews+D16371+public+5c64e187738065e7 at reviews.llvm.org
> Cc: dwatersg at gmail.com, "Eric Christopher" <echristo at gmail.com>, "Duncan P. N. Exon Smith" <dexonsmith at apple.com>,
> "Vedant Kumar" <vsk at apple.com>, "Hal Finkel" <hfinkel at anl.gov>, "llvm-commits" <llvm-commits at lists.llvm.org>
> Sent: Thursday, January 28, 2016 8:25:11 PM
> Subject: Re: [PATCH] D16371: [compiler-rt/profile] Added Hostname to .profdata file
> 
> On Thu, Jan 28, 2016 at 6:10 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> > hfinkel added a comment.
> >
> > In http://reviews.llvm.org/D16371#338901, @vsk wrote:
> >
> >> To clarify, my objection to using guards for this feature wasn't
> >> cosmetic: is there anything fundamental about MSVC that inhibits
> >> this feature?
> >
> >
> > There's no uname() (or sys/utsname.h) on Windows. That's a POSIX
> > function.
> >
> >> I see that your lit test tests this patch, but (1) am concerned
> >> that the usage of uname might break Windows bots, and
> >
> >
> > It would, except that the '// REQUIRES: shell' should cause the
> > Windows bots to skip the test.
> >
> >> (2) wonder whether uname can be used by your scripts directly
> >> instead of relying on %h-expansion.
> >
> >
> > It is not clear how else to test that the host-name expansion
> > actually works.
> 
> I guess Vedant's point is that %h expansion is not strictly needed as
> the name expansion does not need to be delayed until instrumented
> binary run -- it can be set ahead of time before the run with $(uname
> -n) output being put into the name string.  What is the use case in
> which this would not work?

This does not work for cluster systems, supercomputers, etc. where each executable receives the same input environment (even though they're run on different hosts).

Thanks again,
Hal

> 
> David
> 
> 
> 
> >
> >
> > http://reviews.llvm.org/D16371
> >
> >
> >
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory