[Mesa-dev] [PATCH v3 06/10] swr: Windows-related changes

[Emil Velikov]

Hi George,

Seems I was unclear as a few suggestions got missed.

On 16 November 2016 at 02:26, George Kyriazis <george.kyriazis at intel.com> wrote:
> - Handle dynamic library loading for windows
> - Implement swap for gdi
> - fix prototypes
> - update include paths on configure-based build for swr_loader.cpp
> ---
>  src/gallium/drivers/swr/Makefile.am    |  7 +++++++
>  src/gallium/drivers/swr/swr_loader.cpp | 28 +++++++++++++++++++++++++---
>  src/gallium/drivers/swr/swr_public.h   | 11 +++++++----
>  3 files changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/src/gallium/drivers/swr/Makefile.am b/src/gallium/drivers/swr/Makefile.am
> index dd1c2e6..305154f 100644
> --- a/src/gallium/drivers/swr/Makefile.am
> +++ b/src/gallium/drivers/swr/Makefile.am
> @@ -217,6 +217,12 @@ libswrAVX2_la_CXXFLAGS = \
>  libswrAVX2_la_SOURCES = \
>         $(COMMON_SOURCES)
>
> +# XXX: $(SWR_AVX_CXXFLAGS) should not be included, but we end up including
> +# simdintrin.h, which throws a warning if AVX is not enabled
> +libmesaswr_la_CXXFLAGS = \
> +       $(COMMON_CXXFLAGS) \
> +       $(SWR_AVX_CXXFLAGS)
> +
Drop this.

>  # XXX: Don't ship these generated sources for now, since they are specific
>  # to the LLVM version they are generated from. Thus a release tarball
>  # containing the said files, generated against eg. LLVM 3.8 will fail to build
> @@ -235,6 +241,7 @@ libswrAVX2_la_LDFLAGS = \
>  include $(top_srcdir)/install-gallium-links.mk
>
>  EXTRA_DIST = \
> +       SConscript \
This should be part of the patch which brings the file - namely 08/10.

>         rasterizer/archrast/events.proto \
>         rasterizer/jitter/scripts/gen_llvm_ir_macros.py \
>         rasterizer/jitter/scripts/gen_llvm_types.py \
> diff --git a/src/gallium/drivers/swr/swr_loader.cpp b/src/gallium/drivers/swr/swr_loader.cpp
> index 2113c37..4f3329e 100644
> --- a/src/gallium/drivers/swr/swr_loader.cpp
> +++ b/src/gallium/drivers/swr/swr_loader.cpp

> +#include "swr_screen.h"
> +#include "swr_resource.h"
> +
You only need p_screen.h here. Adding the swr ones is wrong (afaict).


> -struct pipe_screen *swr_create_screen(struct sw_winsys *winsys);
> +PUBLIC struct pipe_screen *swr_create_screen(struct sw_winsys *winsys);
>
Do you really need this ? As-it this will make the swr loader
swr_create_screen public which is bad.

Either way - [re]name the loader and backend entrypoints differently
and use separate declarations. You can squash it here, but a separate
patch (5.5/10) would be better.

Thanks
Emil