On 7 November 2016 at 22:32, George Kyriazis <george.kyria...@intel.com> wrote:
> - Added SConscript files
> - better handling of NOMINMAX for <windows.h> inclusion
> - Reorder header files in swr_context.cpp to handle NOMINMAX better, since
> mesa header files include windows.h before we get a chance to #define
> NOMINMAX
> - cleaner support for .dll and .so prefix/suffix across OSes
> - added PUBLIC for some protos
> - added swr_gdi_swap() which is call from libgl_gdi.c
> ---
> src/gallium/drivers/swr/Makefile.am | 8 ++
> src/gallium/drivers/swr/SConscript | 46 +++++++
> src/gallium/drivers/swr/SConscript-arch | 175
> +++++++++++++++++++++++++
> src/gallium/drivers/swr/rasterizer/common/os.h | 5 +-
> src/gallium/drivers/swr/swr_context.cpp | 16 +--
> src/gallium/drivers/swr/swr_context.h | 2 +
> src/gallium/drivers/swr/swr_loader.cpp | 37 +++++-
> src/gallium/drivers/swr/swr_public.h | 11 +-
> src/gallium/drivers/swr/swr_screen.cpp | 25 +---
> 9 files changed, 291 insertions(+), 34 deletions(-)
> create mode 100644 src/gallium/drivers/swr/SConscript
> create mode 100644 src/gallium/drivers/swr/SConscript-arch
>
Similar to 1/3 this patch does too many things. Please _don't_ do that.
Some ideas based on the above:
- source code fixes - one or multiple patches, depending on details.
- automake fixes - ^^
- introduce scons build (+ the EXTRA_DIST hunk)
Some misc comments below.
> +++ b/src/gallium/drivers/swr/SConscript
> @@ -0,0 +1,46 @@
> +Import('*')
> +
> +from sys import executable as python_cmd
> +import distutils.version
Seems unused. Maybe it was aimed for the llvm 3.9 requirement/check
mentioned in 1/3 ?
> +import os.path
> +
> +if not 'swr' in COMMAND_LINE_TARGETS:
> + Return()
> +
> +if not env['llvm']:
> + print 'warning: LLVM disabled: not building swr'
> + Return()
> +
> +env.MSVC2013Compat()
> +
> +swr_arch = 'avx'
> +VariantDir('avx', '.', duplicate=0)
> +SConscript('avx/SConscript-arch', exports='swr_arch')
> +
> +swr_arch = 'avx2'
> +VariantDir('avx2', '.', duplicate=0)
> +SConscript('avx2/SConscript-arch', exports='swr_arch')
> +
Afaict one can just fold the SConscript-arch here. Thus one won't need
to bother with the above nor the Depends hunk below.
Additionally with current approach one is generating [the] identical
source files twice. Far from ideal...
> +env = env.Clone()
> +
> +source = env.ParseSourceList('Makefile.sources', [
> + 'LOADER_SOURCES'
> +])
> +
> +env.Prepend(CPPPATH = [
> + 'rasterizer/scripts'
> + ])
> +
> +swr = env.ConvenienceLibrary(
> + target = 'swr',
> + source = source,
> + )
Keep the indentation to 4 spaces here and throughout the SConscripts.
That's a python requirement.
In general I'd encourage using .editorconfig and updating the section
for swr, if needed.
> +# remove headers, as scons thinks they are static objects for the .so
> +source = [x for x in source if not x.endswith(tuple(['.h','.hpp']))]
> +
Should be handled already. Otherwise please do so in scons/*
Quick grep suggests scons/custom.py
> +#ifdef _WIN32
> + prefix = "";
> + postfix = ".dll";
> +#else
> + prefix = "lib";
> + postfix = ".so";
> +#endif
> +
Quick grep suggests:
UTIL_DL_EXT
UTIL_DL_PREFIX
Regards,
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
