Since there's been some churn on this, I'll try to post the updated patches
in a new thread (provided my git config is set right that is)

- Chuck

On Wed, Jan 17, 2018 at 8:56 AM, Chuck Atkins <[email protected]>
wrote:

> I wasn't happy with the redundancy either but at the time just didn't see
> how to refactor it more cleanly.  I'll take a second pass to consolidate
> and remove duplication.
>
> - Chuck
>
> On Tue, Jan 16, 2018 at 5:03 PM, Cherniak, Bruce <[email protected]
> > wrote:
>
>>
>> > On Jan 16, 2018, at 1:59 PM, Chuck Atkins <[email protected]>
>> wrote:
>> >
>> > Part 2 of 2 (part 1 is autoconf changes, part 2 is C++ changes)
>> >
>> > When only a single SWR architecture is being used, this allows that
>> > architecture to be builtin rather than as a separate libswrARCH.so that
>> > gets loaded via dlopen.  Since there are now several different code
>> > paths for each detected CPU architecture, the log output is also
>> > adjusted to convey where the backend is getting loaded from.
>> >
>> > This allows SWR to be used for static mesa builds which are still
>> > important for large HPC environments where shared libraries can impose
>> > unacceptable application startup times as hundreds of thousands of
>> copies
>> > of the libs are loaded from a shared parallel filesystem.
>> >
>> > Based on an initial implementation by Tim Rowley.
>> >
>> > Signed-off-by: Chuck Atkins <[email protected]>
>> > CC: Tim Rowley <[email protected]>
>> > CC: Bruce Cherniak <[email protected]>
>> > ---
>> > src/gallium/drivers/swr/swr_loader.cpp | 94
>> ++++++++++++++++++++++------------
>> > 1 file changed, 60 insertions(+), 34 deletions(-)
>> >
>> > diff --git a/src/gallium/drivers/swr/swr_loader.cpp
>> b/src/gallium/drivers/swr/swr_loader.cpp
>> > index 9d6f918e34..57f826fd59 100644
>> > --- a/src/gallium/drivers/swr/swr_loader.cpp
>> > +++ b/src/gallium/drivers/swr/swr_loader.cpp
>> > @@ -31,78 +31,104 @@
>> > struct pipe_screen *
>> > swr_create_screen(struct sw_winsys *winsys)
>> > {
>> > +   struct pipe_screen *screen = swr_create_screen_internal(winsys);
>> > +
>> > +   bool found = false;
>> > +   bool is_knl = false;
>> > +
>> > +#ifndef HAVE_SWR_BUILTIN
>> >    char filename[256] = { 0 };
>> > -   fprintf(stderr, "SWR detected ");
>> > +#endif
>> >
>> > -   util_dl_library *pLibrary = nullptr;
>> >
>> >    util_cpu_detect();
>> >
>> > -   bool is_knl = false;
>> > -
>> > -   if (!strlen(filename) &&
>> > -       util_cpu_caps.has_avx512f && util_cpu_caps.has_avx512er) {
>> > -#if HAVE_SWR_KNL
>> > -      fprintf(stderr, "KNL ");
>> > +   if (!found && util_cpu_caps.has_avx512f &&
>> util_cpu_caps.has_avx512er) {
>> > +      fprintf(stderr, "SWR detected KNL instruction support ");
>> > +#ifndef HAVE_SWR_KNL
>> > +      fprintf(stderr, "(skipping; not built).\n");
>> > +#else
>>
>> Each of the below #ifdef HAVE_SWR_BUILTIN clauses are identical, except
>> the filename.
>>
>> Could you instead just setup the filename, set found = true (and the one
>> is_knl), then do
>> the BUILTIN work all in one place...
>>
>> > +   #ifdef HAVE_SWR_BUILTIN
>> > +      swr_screen(screen)->pfnSwrGetInterface = SwrGetInterface;
>> > +      fprintf(stderr, "(using; builtin).\n");
>> > +   #else
>> >       sprintf(filename, "%s%s%s", UTIL_DL_PREFIX, "swrKNL",
>> UTIL_DL_EXT);
>> > +      fprintf(stderr, "(using; %s).\n", filename);
>> > +   #endif
>>
>> > +      found = true;
>> >       is_knl = true;
>> > -#else
>> > -      fprintf(stderr, "KNL (not built) ");
>> > #endif
>> >    }
>> >
>> > -   if (!strlen(filename) &&
>> > -       util_cpu_caps.has_avx512f && util_cpu_caps.has_avx512bw) {
>> > -#if HAVE_SWR_SKX
>> > -      fprintf(stderr, "SKX ");
>> > -      sprintf(filename, "%s%s%s", UTIL_DL_PREFIX, "swrSKX",
>> UTIL_DL_EXT);
>> > +   if (!found && util_cpu_caps.has_avx512f &&
>> util_cpu_caps.has_avx512bw) {
>> > +      fprintf(stderr, "SWR detected SKX instruction support ");
>> > +#ifndef HAVE_SWR_SKX
>> > +      fprintf(stderr, "(skipping; not built).\n");
>> > #else
>> > -      fprintf(stderr, "SKX (not built) ");
>>
>> > +   #ifdef HAVE_SWR_BUILTIN
>> > +      swr_screen(screen)->pfnSwrGetInterface = SwrGetInterface;
>> > +      fprintf(stderr, "(using; builtin).\n");
>> > +   #else
>> > +      sprintf(filename, "%s%s%s", UTIL_DL_PREFIX, "swrSKX",
>> UTIL_DL_EXT);
>> > +      fprintf(stderr, "(using; %s).\n", filename);
>> > +   #endif
>>
>> > +      found = true;
>> > #endif
>> >    }
>> >
>> > -   if (!strlen(filename) && util_cpu_caps.has_avx2) {
>> > -#if HAVE_SWR_AVX2
>> > -      fprintf(stderr, "AVX2 ");
>> > -      sprintf(filename, "%s%s%s", UTIL_DL_PREFIX, "swrAVX2",
>> UTIL_DL_EXT);
>> > +   if (!found && util_cpu_caps.has_avx2) {
>> > +      fprintf(stderr, "SWR detected AVX2 instruction support ");
>> > +#ifndef HAVE_SWR_AVX2
>> > +      fprintf(stderr, "(skipping; not built).\n");
>> > #else
>> > -      fprintf(stderr, "AVX2 (not built) ");
>>
>> > +   #ifdef HAVE_SWR_BUILTIN
>> > +      swr_screen(screen)->pfnSwrGetInterface = SwrGetInterface;
>> > +      fprintf(stderr, "(using; builtin).\n");
>> > +   #else
>> > +      sprintf(filename, "%s%s%s", UTIL_DL_PREFIX, "swrAVX2",
>> UTIL_DL_EXT);
>> > +      fprintf(stderr, "(using; %s).\n", filename);
>> > +   #endif
>>
>> > +      found = true;
>> > #endif
>> >    }
>> >
>> > -   if (!strlen(filename) && util_cpu_caps.has_avx) {
>> > -#if HAVE_SWR_AVX
>> > -      fprintf(stderr, "AVX ");
>> > -      sprintf(filename, "%s%s%s", UTIL_DL_PREFIX, "swrAVX",
>> UTIL_DL_EXT);
>> > +   if (!found && util_cpu_caps.has_avx) {
>> > +      fprintf(stderr, "SWR detected AVX instruction support ");
>> > +#ifndef HAVE_SWR_AVX
>> > +      fprintf(stderr, "(skipping; not built).\n");
>> > #else
>> > -      fprintf(stderr, "AVX (not built) ");
>>
>> > +   #ifdef HAVE_SWR_BUILTIN
>> > +      swr_screen(screen)->pfnSwrGetInterface = SwrGetInterface;
>> > +      fprintf(stderr, "(using; builtin).\n");
>> > +   #else
>> > +      sprintf(filename, "%s%s%s", UTIL_DL_PREFIX, "swrAVX",
>> UTIL_DL_EXT);
>> > +      fprintf(stderr, "(using; %s).\n", filename);
>> > +   #endif
>>
>> > +      found = true;
>> > #endif
>> >    }
>> >
>> > -   if (!strlen(filename)) {
>> > -      fprintf(stderr, "- no appropriate swr architecture library.
>> Aborting!\n");
>> > +   if (!found) {
>> > +      fprintf(stderr, "SWR could not detect a supported CPU
>> architecture.\n");
>> >       exit(-1);
>> > -   } else {
>> > -      fprintf(stderr, "\n");
>> >    }
>> >
>> > -   pLibrary = util_dl_open(filename);
>> > -
>>
>> Here, you can do the BUILTIN
>>       swr_screen(screen)->pfnSwrGetInterface = SwrGetInterface;
>>       fprintf(stderr, "(using; builtin).\n");
>>
>> or open the correct library and print which one we're using (
>> #else
>> > +#ifndef HAVE_SWR_BUILTIN
>> > +   util_dl_library *pLibrary = util_dl_open(filename);
>> >    if (!pLibrary) {
>> >       fprintf(stderr, "SWR library load failure: %s\n",
>> util_dl_error());
>> >       exit(-1);
>> >    }
>> >
>> >    util_dl_proc pApiProc = util_dl_get_proc_address(pLibrary,
>> "SwrGetInterface");
>> > -
>> >    if (!pApiProc) {
>> >       fprintf(stderr, "SWR library search failure: %s\n",
>> util_dl_error());
>> >       exit(-1);
>> >    }
>> >
>> > -   struct pipe_screen *screen = swr_create_screen_internal(winsys);
>> >    swr_screen(screen)->pfnSwrGetInterface =
>> (PFNSwrGetInterface)pApiProc;
>> >    swr_screen(screen)->is_knl = is_knl;
>> > +#endif
>> >
>> >    return screen;
>> > }
>> > --
>> > 2.14.3
>> >
>>
>>
>
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to