[PATCH] D17676: Add some minimal portability code paths for PS4.

[Xinliang David Li via llvm-commits]

On Fri, Feb 26, 2016 at 11:53 PM, Sean Silva <chisophugis at gmail.com> wrote:

> silvas created this revision.
> silvas added reviewers: davidxl, MaggieYi, phillip.power, filcab.
> silvas added subscribers: llvm-commits, probinson, slingn,
> simon.f.whittaker.
>
> Hi David, SCE folks,
>
> What is implemented in this patch is enough for the upstream libprofile to
> work for PGO with the PS4 game codebase I tested ("game7" for you SCE
> folks; this is with a standalone build of compiler-rt).
>
> The first change, which is simple, is to stub out gethostname. PS4
> doesn't have a simple analog for this that doesn't bring in extra
> OS libraries, so for now we do not support `%h` expansion.
> This is consistent with internal B#136272.
>
> The second change implies future work, but is a simple change at present.
> PS4 does not have `getenv`, so for now we will introduce a shim.
> This obviously makes it impossible for many of the tests to be run since
> they require setting `LLVM_PROFILE_FILE=`.
>
> I see two paths forward:
>
> 1. In the tests we are already wrapping execution with `%run` and so by
>    setting a PS4-specific expansion for `%run` we can pass the information
>    in another way We can adapt the getenv shim as appropriate.
>    We will need to experiment with this internally.
>    Maggie, Phillip, Filipe? Any ideas? Maybe ping me internally since we
>    may need to get into some PS4 vagaries. I'm thinking a fake getenv
>    library that uses some side channel for communication.
>
> 2. Another possibility which is more verbose is to use a separate clang
>    invocation with `-profile-generate=<filename>` to set the filename in
>    each test.
>    This might require redundant clang invocations though which may be
>    undesirable for upstream. David, thoughts?
>    Also, this is a fairly libprofile-specific workaround, so it e.g.
>    doesn't help Filipe's ASan work.
>    Overall, this approach sounds like a bit of a hack to me.
>

One possibility is detect this in cmake and in lit.cfg, set clang_profgen
to use -fprofile-instr-generate= form.

>
> Small detail:
> InstrProfilingPort.h seems like the natural place for the getenv shim,
> but GCDAProfiling.c needs it as well. InstrProfilingUtil.h is currently
> the only header common between InstrProfilingFile.c and GCDAProfiling.c.
> I can move the shim to InstrProfilingPort.h and add an include to
> GCDAProfiling.c as per your preference David.
>
> http://reviews.llvm.org/D17676
>
> Files:
>   lib/profile/InstrProfilingPort.h
>   lib/profile/InstrProfilingUtil.h
>
> Index: lib/profile/InstrProfilingUtil.h
> ===================================================================
> --- lib/profile/InstrProfilingUtil.h
> +++ lib/profile/InstrProfilingUtil.h
> @@ -13,4 +13,9 @@
>  /*! \brief Create a directory tree. */
>  void __llvm_profile_recursive_mkdir(char *Pathname);
>
> +/* PS4 doesn't have getenv. Define a shim. */
> +#if __PS4__
> +static inline char *getenv(const char *name) { return 0; }
> +#endif /* #if __PS4__ */
> +
>  #endif  /* PROFILE_INSTRPROFILINGUTIL_H */
> Index: lib/profile/InstrProfilingPort.h
> ===================================================================
> --- lib/profile/InstrProfilingPort.h
> +++ lib/profile/InstrProfilingPort.h
> @@ -25,6 +25,8 @@
>  #define COMPILER_RT_MAX_HOSTLEN 128
>  #ifdef _MSC_VER
>  #define COMPILER_RT_GETHOSTNAME(Name, Len) gethostname(Name, Len)
> +#elif defined(__PS4__)
> +#define COMPILER_RT_GETHOSTNAME(Name, Len) (-1)
>  #else
>  #define COMPILER_RT_GETHOSTNAME(Name, Len) GetHostName(Name, Len)
>  #define COMPILER_RT_HAS_UNAME 1
>
>
> This looks fine.

thanks,

David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160227/68f05045/attachment.html>