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

[Kyriazis, George]

> -----Original Message-----
> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
> Behalf Of Emil Velikov
> Sent: Thursday, November 17, 2016 11:12 AM
> To: Kyriazis, George <george.kyriazis at intel.com>
> Cc: ML mesa-dev <mesa-dev at lists.freedesktop.org>
> Subject: Re: [Mesa-dev] [PATCH v3 06/10] swr: Windows-related changes
> 
> 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.
> 
This is needed for linux configure-based build.

> >  # 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.
> 
Ok, will do.

> >         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).
> 
Ok, replaced.

> 
> > -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.
> 
Ok, renaming backend entry points.

I'll resend another batch of changes for review, since there is order dependency and I can't send 6/10 ad 5.5/10 after the later ones.

Thanks,

george

> Thanks
> Emil
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev