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

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 28 18:25:11 PST 2016


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?

David



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


More information about the llvm-commits mailing list