For context, without this, the library handle from dlopen never get's
closed, even under successful operation, and the swr_screen created never
get's freed on error. Also error conditions resulted in exit() rather than
NULL return.
- Chuck
On Mon, Jan 22, 2018 at 10:12 AM, Chuck Atkins <chuck.atk...@kitware.com>
wrote:
> Signed-off-by: Chuck Atkins <chuck.atk...@kitware.com>
> ---
> src/gallium/drivers/swr/swr_loader.cpp | 100
> +++++++++++++++++----------------
> src/gallium/drivers/swr/swr_public.h | 6 +-
> src/gallium/drivers/swr/swr_screen.cpp | 26 +++++++--
> src/gallium/drivers/swr/swr_screen.h | 3 +
> 4 files changed, 79 insertions(+), 56 deletions(-)
>
> diff --git a/src/gallium/drivers/swr/swr_loader.cpp
> b/src/gallium/drivers/swr/swr_loader.cpp
> index 7f28bdb536..01b9804646 100644
> --- a/src/gallium/drivers/swr/swr_loader.cpp
> +++ b/src/gallium/drivers/swr/swr_loader.cpp
> @@ -29,96 +29,98 @@
> #include <stdio.h>
>
> // Helper function to resolve the backend filename based on architecture
> -inline void get_swr_arch_filename(const char arch[], char filename[])
> +static bool
> +swr_initialize_screen_interface(struct swr_screen *screen, const char
> arch[])
> {
> #ifdef HAVE_SWR_BUILTIN
> - strcpy(filename , "builtin");
> + screen->pLibrary = NULL;
> + screen->pfnSwrGetInterface = SwrGetInterface;
> + fprintf(stderr, "(using: builtin).\n");
> #else
> + char filename[256] = { 0 };
> sprintf(filename, "%sswr%s%s", UTIL_DL_PREFIX, arch, UTIL_DL_EXT);
> +
> + screen->pLibrary = util_dl_open(filename);
> + if (!screen->pLibrary) {
> + fprintf(stderr, "(skipping: %s).\n", util_dl_error());
> + return false;
> + }
> +
> + util_dl_proc pApiProc = util_dl_get_proc_address(screen->pLibrary,
> + "SwrGetInterface");
> + if (!pApiProc) {
> + fprintf(stderr, "(skipping: %s).\n", util_dl_error());
> + util_dl_close(screen->pLibrary);
> + screen->pLibrary = NULL;
> + return false;
> + }
> +
> + screen->pfnSwrGetInterface = (PFNSwrGetInterface)pApiProc;
> + fprintf(stderr, "(using: %s).\n", filename);
> #endif
> + return true;
> }
>
> +
> struct pipe_screen *
> swr_create_screen(struct sw_winsys *winsys)
> {
> - char filename[256] = { 0 };
> - bool found = false;
> - bool is_knl = false;
> - PFNSwrGetInterface pfnSwrGetInterface = nullptr;
> + struct pipe_screen *p_screen = swr_create_screen_internal(winsys);
> + if (!p_screen) {
> + return NULL;
> + }
> +
> + struct swr_screen *screen = swr_screen(p_screen);
> + screen->is_knl = false;
>
> util_cpu_detect();
>
> - if (!found && util_cpu_caps.has_avx512f && util_cpu_caps.has_avx512er)
> {
> + if (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");
> + fprintf(stderr, "(skipping: not built).\n");
> #else
> - get_swr_arch_filename("KNL", filename);
> - found = true;
> - is_knl = true;
> + if (swr_initialize_screen_interface(screen, "KNL")) {
> + screen->is_knl = true;
> + return p_screen;
> + }
> #endif
> }
>
> - if (!found && util_cpu_caps.has_avx512f && util_cpu_caps.has_avx512bw)
> {
> + if (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
> - get_swr_arch_filename("SKX", filename);
> - found = true;
> + if (swr_initialize_screen_interface(screen, "SKX"))
> + return p_screen;
> #endif
> }
>
> - if (!found && util_cpu_caps.has_avx2) {
> + if (util_cpu_caps.has_avx2) {
> fprintf(stderr, "SWR detected AVX2 instruction support ");
> #ifndef HAVE_SWR_AVX2
> fprintf(stderr, "(skipping not built).\n");
> #else
> - get_swr_arch_filename("AVX2", filename);
> - found = true;
> + if (swr_initialize_screen_interface(screen, "AVX2"))
> + return p_screen;
> #endif
> }
>
> - if (!found && util_cpu_caps.has_avx) {
> + if (util_cpu_caps.has_avx) {
> fprintf(stderr, "SWR detected AVX instruction support ");
> #ifndef HAVE_SWR_AVX
> fprintf(stderr, "(skipping not built).\n");
> #else
> - get_swr_arch_filename("AVX", filename);
> - found = true;
> + if (swr_initialize_screen_interface(screen, "AVX"))
> + return p_screen;
> #endif
> }
>
> - if (!found) {
> - fprintf(stderr, "SWR could not detect a supported CPU
> architecture.\n");
> - exit(-1);
> - }
> -
> - fprintf(stderr, "(using %s).\n", filename);
> -
> -#ifdef HAVE_SWR_BUILTIN
> - pfnSwrGetInterface = SwrGetInterface;
> -#else
> - 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);
> - }
> -
> - pfnSwrGetInterface = (PFNSwrGetInterface)pApiProc;
> -#endif
> -
> - struct pipe_screen *screen = swr_create_screen_internal(winsys);
> - swr_screen(screen)->is_knl = is_knl;
> - swr_screen(screen)->pfnSwrGetInterface = pfnSwrGetInterface;
> + fprintf(stderr, "SWR could not initialize a supported CPU
> architecture.\n");
> + swr_destroy_screen_internal(&screen);
>
> - return screen;
> + return NULL;
> }
>
>
> diff --git a/src/gallium/drivers/swr/swr_public.h
> b/src/gallium/drivers/swr/swr_public.h
> index 4b150705cd..4351b3aae0 100644
> --- a/src/gallium/drivers/swr/swr_public.h
> +++ b/src/gallium/drivers/swr/swr_public.h
> @@ -25,8 +25,9 @@
> #define SWR_PUBLIC_H
>
> struct pipe_screen;
> -struct sw_winsys;
> struct sw_displaytarget;
> +struct sw_winsys;
> +struct swr_screen;
>
> #ifdef __cplusplus
> extern "C" {
> @@ -38,6 +39,9 @@ struct pipe_screen *swr_create_screen(struct sw_winsys
> *winsys);
> // arch-specific dll entry point
> PUBLIC struct pipe_screen *swr_create_screen_internal(struct sw_winsys
> *winsys);
>
> +// cleanup for failed screen creation
> +PUBLIC void swr_destroy_screen_internal(struct swr_screen **screen);
> +
> #ifdef _WIN32
> void swr_gdi_swap(struct pipe_screen *screen,
> struct pipe_resource *res,
> diff --git a/src/gallium/drivers/swr/swr_screen.cpp
> b/src/gallium/drivers/swr/swr_screen.cpp
> index b67ac25ac8..cbb2b844a9 100644
> --- a/src/gallium/drivers/swr/swr_screen.cpp
> +++ b/src/gallium/drivers/swr/swr_screen.cpp
> @@ -1052,6 +1052,24 @@ swr_flush_frontbuffer(struct pipe_screen *p_screen,
> }
>
>
> +PUBLIC void
> +swr_destroy_screen_internal(struct swr_screen **screen)
> +{
> + struct pipe_screen *p_screen = &(*screen)->base;
> +
> + swr_fence_finish(p_screen, NULL, (*screen)->flush_fence, 0);
> + swr_fence_reference(p_screen, &(*screen)->flush_fence, NULL);
> +
> + JitDestroyContext((*screen)->hJitMgr);
> +
> + if ((*screen)->pLibrary)
> + util_dl_close((*screen)->pLibrary);
> +
> + FREE(*screen);
> + *screen = NULL;
> +}
> +
> +
> static void
> swr_destroy_screen(struct pipe_screen *p_screen)
> {
> @@ -1060,15 +1078,10 @@ swr_destroy_screen(struct pipe_screen *p_screen)
>
> fprintf(stderr, "SWR destroy screen!\n");
>
> - swr_fence_finish(p_screen, NULL, screen->flush_fence, 0);
> - swr_fence_reference(p_screen, &screen->flush_fence, NULL);
> -
> - JitDestroyContext(screen->hJitMgr);
> -
> if (winsys->destroy)
> winsys->destroy(winsys);
>
> - FREE(screen);
> + swr_destroy_screen_internal(&screen);
> }
>
>
> @@ -1119,6 +1132,7 @@ struct pipe_screen *
> swr_create_screen_internal(struct sw_winsys *winsys)
> {
> struct swr_screen *screen = CALLOC_STRUCT(swr_screen);
> + memset(screen, 0, sizeof(struct swr_screen));
>
> if (!screen)
> return NULL;
> diff --git a/src/gallium/drivers/swr/swr_screen.h
> b/src/gallium/drivers/swr/swr_screen.h
> index 89faab182c..b80d8c70ec 100644
> --- a/src/gallium/drivers/swr/swr_screen.h
> +++ b/src/gallium/drivers/swr/swr_screen.h
> @@ -28,6 +28,7 @@
>
> #include "pipe/p_screen.h"
> #include "pipe/p_defines.h"
> +#include "util/u_dl.h"
> #include "util/u_format.h"
> #include "api.h"
>
> @@ -50,6 +51,8 @@ struct swr_screen {
>
> HANDLE hJitMgr;
>
> + /* Dynamic backend implementations */
> + util_dl_library *pLibrary;
> PFNSwrGetInterface pfnSwrGetInterface;
>
> /* Do we run on Xeon Phi? */
> --
> 2.14.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
