[Mesa-dev] [PATCH 4/4] OpenSWR build changes

Emil Velikov emil.l.velikov at gmail.com
Mon Feb 22 18:53:25 UTC 2016


On 18 February 2016 at 01:53, Tim Rowley <timothy.o.rowley at intel.com> wrote:

Don't be shy to mention something in the commit message - which of the
tree targets has been tested. Do they all build/work on linux,
windows, BSD, other. Pretty much anything that you believe it's
important. You might also include something that is known to not build
properly (be that explicitly disabled atm or not).

> ---
>  configure.ac                                  |  18 ++++
>  scons/custom.py                               |   2 +-
>  scons/llvm.py                                 |   8 +-
>  src/gallium/Makefile.am                       |   6 ++
>  src/gallium/SConscript                        |   3 +
>  src/gallium/drivers/swr/.clang-format         |  64 ++++++++++++++
>  src/gallium/drivers/swr/Automake.inc          |  30 +++++++
>  src/gallium/drivers/swr/Makefile.am           |  37 ++++++++
>  src/gallium/drivers/swr/Makefile.sources      |  23 +++++
>  src/gallium/drivers/swr/Makefile.sources-arch | 111 ++++++++++++++++++++++++
>  src/gallium/drivers/swr/SConscript            |  31 +++++++
>  src/gallium/drivers/swr/SConscript-arch       | 117 ++++++++++++++++++++++++++
>  src/gallium/drivers/swr/avx/Makefile.am       | 112 ++++++++++++++++++++++++
>  src/gallium/drivers/swr/avx2/Makefile.am      | 112 ++++++++++++++++++++++++
>  src/gallium/targets/libgl-gdi/SConscript      |   8 +-
>  src/gallium/targets/libgl-xlib/Makefile.am    |   5 ++
>  src/gallium/targets/libgl-xlib/SConscript     |   4 +
>  src/gallium/targets/osmesa/Makefile.am        |   6 ++
>  src/gallium/targets/osmesa/SConscript         |   4 +
>  19 files changed, 696 insertions(+), 5 deletions(-)
>  create mode 100644 src/gallium/drivers/swr/.clang-format
>  create mode 100644 src/gallium/drivers/swr/Automake.inc
>  create mode 100644 src/gallium/drivers/swr/Makefile.am
>  create mode 100644 src/gallium/drivers/swr/Makefile.sources
>  create mode 100644 src/gallium/drivers/swr/Makefile.sources-arch
>  create mode 100644 src/gallium/drivers/swr/SConscript
>  create mode 100644 src/gallium/drivers/swr/SConscript-arch
>  create mode 100644 src/gallium/drivers/swr/avx/Makefile.am
>  create mode 100644 src/gallium/drivers/swr/avx2/Makefile.am
>
> diff --git a/configure.ac b/configure.ac
> index 57330cb..1c8aaaa 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2191,6 +2191,16 @@ radeon_llvm_check() {
>      fi
>  }
>
> +swr_llvm_check() {
> +    gallium_require_llvm $1
> +    if test ${LLVM_VERSION_INT} -lt 306; then
> +        AC_MSG_ERROR([LLVM version 3.6 or later required when building $1])
> +    fi
> +    if test "x$enable_gallium_llvm" != "xyes"; then
I'm leaning that forcing this will get us more than what we need. We
can introduce a "NEED_GALLIUM_LLVM" conditional variable, which if set
builds the common llvm code. We can also use a lowercase named one,
set as enable-* equals to yes, and the one checked at the very end of
configure.ac

if test "x$enable_gallium_llvm" = xyes; then
    if test -n "$llvm_prefix"; then
        AC_PATH_TOOL([LLVM_CONFIG], [llvm-config], [no], ["$llvm_prefix/bin"])

Thinking about it the above might end up too messy. Please give it a
go and if things start exploding everywhere, just ignore my suggestion
and keep it as is.

> +        AC_MSG_ERROR([--enable-gallium-llvm is required when building $1])
> +    fi
> +}
> +
>  dnl Duplicates in GALLIUM_DRIVERS_DIRS are removed by sorting it after this block
>  if test -n "$with_gallium_drivers"; then
>      gallium_drivers=`IFS=', '; echo $with_gallium_drivers`
> @@ -2263,6 +2273,10 @@ if test -n "$with_gallium_drivers"; then
>                  HAVE_GALLIUM_LLVMPIPE=yes
>              fi
>              ;;
> +        xswr)
> +            swr_llvm_check "swr"
> +            HAVE_GALLIUM_SWR=yes
We want some form of testing here. Namely we should check that the
compiler will be fine with std=c++11 or at the very least that it can
build avx and avx2 code. Otherwise someone with 4+ yo compiler
shipping with their stable distro will wonder why things fail in crazy
ways.

> +            ;;
>          xvc4)
>              HAVE_GALLIUM_VC4=yes
>              gallium_require_drm "vc4"
> @@ -2352,6 +2366,7 @@ AM_CONDITIONAL(HAVE_GALLIUM_NOUVEAU, test "x$HAVE_GALLIUM_NOUVEAU" = xyes)
>  AM_CONDITIONAL(HAVE_GALLIUM_FREEDRENO, test "x$HAVE_GALLIUM_FREEDRENO" = xyes)
>  AM_CONDITIONAL(HAVE_GALLIUM_SOFTPIPE, test "x$HAVE_GALLIUM_SOFTPIPE" = xyes)
>  AM_CONDITIONAL(HAVE_GALLIUM_LLVMPIPE, test "x$HAVE_GALLIUM_LLVMPIPE" = xyes)
> +AM_CONDITIONAL(HAVE_GALLIUM_SWR, test "x$HAVE_GALLIUM_SWR" = xyes)
>  AM_CONDITIONAL(HAVE_GALLIUM_VC4, test "x$HAVE_GALLIUM_VC4" = xyes)
>  AM_CONDITIONAL(HAVE_GALLIUM_VIRGL, test "x$HAVE_GALLIUM_VIRGL" = xyes)
>
> @@ -2461,6 +2476,9 @@ AC_CONFIG_FILES([Makefile
>                 src/gallium/drivers/rbug/Makefile
>                 src/gallium/drivers/softpipe/Makefile
>                 src/gallium/drivers/svga/Makefile
> +               src/gallium/drivers/swr/Makefile
> +               src/gallium/drivers/swr/avx/Makefile
> +               src/gallium/drivers/swr/avx2/Makefile
>                 src/gallium/drivers/trace/Makefile
>                 src/gallium/drivers/vc4/Makefile
>                 src/gallium/drivers/virgl/Makefile
> diff --git a/scons/custom.py b/scons/custom.py
> index 043793b..a5a3410 100644
> --- a/scons/custom.py
> +++ b/scons/custom.py
> @@ -132,7 +132,7 @@ def code_generate(env, script, target, source, command):
>      script_src = env.File(script).srcnode()
>
>      # This command creates generated code *in the build directory*.
> -    command = command.replace('$SCRIPT', script_src.path)
> +    command = command.replace('$SCRIPT', script_src.rstr())
Looks like unrelated change which I'd split out in separate patch.

>      action = SCons.Action.Action(command, "$CODEGENCOMSTR")
>      code = env.Command(target, source, action)
>
> diff --git a/scons/llvm.py b/scons/llvm.py
> index 1fc8a3f..30742ff 100644
> --- a/scons/llvm.py
> +++ b/scons/llvm.py

> @@ -128,7 +129,8 @@ def generate(env):
>                  'LLVMX86Info', 'LLVMX86AsmPrinter', 'LLVMX86Utils',
>                  'LLVMMCJIT', 'LLVMTarget', 'LLVMExecutionEngine',
>                  'LLVMRuntimeDyld', 'LLVMObject', 'LLVMMCParser',
> -                'LLVMBitReader', 'LLVMMC', 'LLVMCore', 'LLVMSupport'
> +                'LLVMBitReader', 'LLVMMC', 'LLVMCore', 'LLVMSupport',
> +                'LLVMIRReader', 'LLVMAsmParser', 'LLVMX86AsmParser'
I'm thinking that we'll need these (and perhaps the irreader below)
for the automake/conf build as well. Set --disable-llvm-shared-libs
and watch things burn ;-)

>              ])
>          elif llvm_version >= distutils.version.LooseVersion('3.5'):
>              env.Prepend(LIBS = [
> @@ -198,12 +200,14 @@ def generate(env):
>                  pass
>              env.MergeFlags(cppflags)
>
> +            env['llvm_includedir'] = env.backtick('llvm-config --includedir').rstrip()
> +
>              # Match llvm --fno-rtti flag
>              cxxflags = env.backtick('llvm-config --cxxflags').split()
>              if '-fno-rtti' in cxxflags:
>                  env.Append(CXXFLAGS = ['-fno-rtti'])
>
> -            components = ['engine', 'mcjit', 'bitwriter', 'x86asmprinter', 'mcdisassembler']
> +            components = ['engine', 'mcjit', 'bitwriter', 'x86asmprinter', 'mcdisassembler', 'irreader']
>


> --- a/src/gallium/SConscript
> +++ b/src/gallium/SConscript
> @@ -20,6 +20,9 @@ SConscript([
>      'drivers/trace/SConscript',
>  ])
>
> +if env['platform'] != 'windows':
> +   SConscript('drivers/swr/SConscript')
> +
I take it that you're planning to change this at some point in the
future ? Some of the scon files below explicitly check against msvc.

> --- /dev/null
> +++ b/src/gallium/drivers/swr/.clang-format
Not a buildsystem file/change, but it's fine ;-)

> --- /dev/null
> +++ b/src/gallium/drivers/swr/Automake.inc
> @@ -0,0 +1,30 @@
> +# Copyright (C) 2015 Intel Corporation.   All Rights Reserved.
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> +# IN THE SOFTWARE.
> +
> +if HAVE_GALLIUM_SWR
> +
> +TARGET_CPPFLAGS += -DGALLIUM_SWR
> +TARGET_LIB_DEPS += \
> +       $(top_builddir)/src/gallium/drivers/swr/libmesaswr.la
> +       $(top_builddir)/src/gallium/drivers/swr/avx/libswrAVX.la
> +       $(top_builddir)/src/gallium/drivers/swr/avx2/libswrAVX2.la
> +
> +endif

Don't think you need this file at all. It (and TARGET_LIB_DEPS) is
used for the targets which work with more than software based
renderers - dri, vdapu ...

> diff --git a/src/gallium/drivers/swr/Makefile.am b/src/gallium/drivers/swr/Makefile.am
> new file mode 100644
> index 0000000..f3a4321
> --- /dev/null
> +++ b/src/gallium/drivers/swr/Makefile.am
> @@ -0,0 +1,37 @@
> +# Copyright (C) 2015 Intel Corporation.   All Rights Reserved.
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> +# IN THE SOFTWARE.
> +
> +AUTOMAKE_OPTIONS = subdir-objects
> +
We don't need this. It's already enabled globally.

> +include Makefile.sources
> +include $(top_srcdir)/src/gallium/Automake.inc
> +
> +AM_CXXFLAGS = \
> +       $(GALLIUM_DRIVER_CFLAGS) \
> +       -std=c++11 -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS
> +
I believe we already set the latter two in configure.ac, right ?

> +noinst_LTLIBRARIES = libmesaswr.la
> +
> +libmesaswr_la_SOURCES = $(CXX_SOURCES)
> +
> +libmesaswr_la_LDFLAGS =
> +
You can drop this line.

> +EXTRA_DIST = SConscript SConscript-arch Makefile.sources-arch
> diff --git a/src/gallium/drivers/swr/Makefile.sources b/src/gallium/drivers/swr/Makefile.sources
> new file mode 100644
> index 0000000..4317a85
> --- /dev/null
> +++ b/src/gallium/drivers/swr/Makefile.sources
> @@ -0,0 +1,23 @@
> +# Copyright (C) 2015 Intel Corporation.   All Rights Reserved.
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> +# IN THE SOFTWARE.
> +
> +CXX_SOURCES := \
> +       swr_loader.cpp

I'd fold this and the Makefile.sources-arch into a single file.

> diff --git a/src/gallium/drivers/swr/Makefile.sources-arch b/src/gallium/drivers/swr/Makefile.sources-arch
> new file mode 100644
> index 0000000..7ed0faf
> --- /dev/null
> +++ b/src/gallium/drivers/swr/Makefile.sources-arch
> @@ -0,0 +1,111 @@
> +# Copyright (C) 2015 Intel Corporation.   All Rights Reserved.
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> +# IN THE SOFTWARE.
> +
> +CXX_SOURCES := \
> +       swr_clear.cpp \
> +       swr_context.cpp \
> +       swr_context.h \
> +       swr_context_llvm.h \
> +       swr_draw.cpp \
> +       swr_public.h \
> +       swr_resource.h \
> +       swr_screen.cpp \
> +       swr_screen.h \
> +       swr_state.cpp \
> +       swr_state.h \
> +       swr_tex_sample.cpp \
> +       swr_tex_sample.h \
> +       swr_scratch.h \
> +       swr_scratch.cpp \
> +       swr_shader.cpp \
> +       swr_memory.h \
> +       swr_fence.h \
> +       swr_fence.cpp \
> +       swr_query.h \
> +       swr_query.cpp
> +
> +COMMON_CXX_SOURCES := \
> +    rasterizer/common/containers.hpp \
> +    rasterizer/common/formats.cpp \
> +    rasterizer/common/formats.h \
> +    rasterizer/common/isa.hpp \
> +    rasterizer/common/os.h \
> +    rasterizer/common/rdtsc_buckets.cpp \
> +    rasterizer/common/rdtsc_buckets.h \
> +    rasterizer/common/rdtsc_buckets_shared.h \
> +    rasterizer/common/rdtsc_buckets_shared.h \
> +    rasterizer/common/simdintrin.h \
> +    rasterizer/common/swr_assert.cpp \
> +    rasterizer/common/swr_assert.h
> +
> +CORE_CXX_SOURCES := \
> +    rasterizer/core/api.cpp \
> +    rasterizer/core/api.h \
> +    rasterizer/core/arena.cpp \
> +    rasterizer/core/arena.h \
> +    rasterizer/core/backend.cpp \
> +    rasterizer/core/backend.h \
> +    rasterizer/core/blend.h \
> +    rasterizer/core/clip.cpp \
> +    rasterizer/core/clip.h \
> +    rasterizer/core/context.h \
> +    rasterizer/core/depthstencil.h \
> +    rasterizer/core/fifo.hpp \
> +    rasterizer/core/format_traits.h \
> +    rasterizer/core/format_types.h \
> +    rasterizer/core/frontend.cpp \
> +    rasterizer/core/frontend.h \
> +    rasterizer/core/knobs.h \
> +    rasterizer/core/knobs_init.h \
> +    rasterizer/core/multisample.cpp \
> +    rasterizer/core/multisample.h \
> +    rasterizer/core/pa_avx.cpp \
> +    rasterizer/core/pa.h \
> +    rasterizer/core/rasterizer.cpp \
> +    rasterizer/core/rasterizer.h \
> +    rasterizer/core/rdtsc_core.cpp \
> +    rasterizer/core/rdtsc_core.h \
> +    rasterizer/core/state.h \
> +    rasterizer/core/threads.cpp \
> +    rasterizer/core/threads.h \
> +    rasterizer/core/tilemgr.cpp \
> +    rasterizer/core/tilemgr.h \
> +    rasterizer/core/utils.cpp \
> +    rasterizer/core/utils.h
> +
> +JITTER_CXX_SOURCES := \
> +    rasterizer/jitter/blend_jit.cpp \
> +    rasterizer/jitter/blend_jit.h \
> +    rasterizer/jitter/builder.cpp \
> +    rasterizer/jitter/builder.h \
> +    rasterizer/jitter/builder_misc.cpp \
> +    rasterizer/jitter/builder_misc.h \
> +    rasterizer/jitter/fetch_jit.cpp \
> +    rasterizer/jitter/fetch_jit.h \
> +    rasterizer/jitter/JitManager.cpp \
> +    rasterizer/jitter/JitManager.h \
> +    rasterizer/jitter/streamout_jit.cpp \
> +    rasterizer/jitter/streamout_jit.h
> +
> +MEMORY_CXX_SOURCES := \
> +    rasterizer/memory/ClearTile.cpp \
> +    rasterizer/memory/LoadTile.cpp \
> +    rasterizer/memory/StoreTile.cpp
Seems that some of the above lines start with space, while others with
tabs. Please use only tabs - different versions/vendors of make tend
to be picky.

> diff --git a/src/gallium/drivers/swr/SConscript b/src/gallium/drivers/swr/SConscript
> new file mode 100644
> index 0000000..b688054
> --- /dev/null
> +++ b/src/gallium/drivers/swr/SConscript
> @@ -0,0 +1,31 @@
> +from sys import executable as python_cmd
> +import distutils.version
> +import os.path
> +
> +Import('*')
> +
> +if not env['llvm']:
> +    print 'warning: LLVM disabled: not building swr'
> +    Return()
> +
> +swr_arch = 'avx'
> +SConscript('SConscript-arch', variant_dir='avx', duplicate=0, exports='swr_arch')
> +swr_arch = 'avx2'
> +SConscript('SConscript-arch', variant_dir='avx2', duplicate=0, exports='swr_arch')
> +
> +env = env.Clone()
> +
> +env.MSVC2013Compat()
> +
> +source = env.ParseSourceList('Makefile.sources', [
> +    'CXX_SOURCES'
> +])
> +
> +swr = env.ConvenienceLibrary(
> +       target = 'swr',
> +       source = source,
> +       )
> +
Please use four spaces per indentation level in SConscript (python
scripts). If you've spotted somewhere in mesa where things are messed
up let us know.

> +env.Alias('swr', swr)
> +
> +Export('swr')
> diff --git a/src/gallium/drivers/swr/SConscript-arch b/src/gallium/drivers/swr/SConscript-arch
> new file mode 100644
> index 0000000..b20a1f8
> --- /dev/null
> +++ b/src/gallium/drivers/swr/SConscript-arch
> @@ -0,0 +1,117 @@
> +from sys import executable as python_cmd
> +import distutils.version
> +import os.path
> +
> +Import('*')
> +
> +if not env['llvm']:
> +    print 'warning: LLVM disabled: not building swr'
> +    Return()
> +
> +env = env.Clone()
> +
> +env.MSVC2013Compat()
> +
> +env.Append(CPPDEFINES = [
> +       '__STDC_CONSTANT_MACROS',
> +       '__STDC_LIMIT_MACROS',
> +       ])
> +
Those are already set in scons/llvm.py aren't they ?

> +if not env['msvc'] :
> +    env.Append(CCFLAGS = [
> +        '-std=c++11',
> +    ])
> +
> +env.Prepend(CPPPATH = [
> +    'rasterizer',
> +    'rasterizer/core',
> +    'rasterizer/jitter',
> +    'rasterizer/scripts',
> +    'rasterizer/jitter',
> +    ])
> +
> +env.CodeGenerate(
> +    target = 'rasterizer/scripts/gen_knobs.cpp',
> +    script = 'rasterizer/scripts/gen_knobs.py',
> +    source = [],
> +    command = python_cmd + ' $SCRIPT ' + Dir('rasterizer/scripts').abspath
> +)
> +
> +env.CodeGenerate(
> +    target = 'rasterizer/scripts/gen_knobs.h',
> +    script = 'rasterizer/scripts/gen_knobs.py',
> +    source = [],
> +    command = python_cmd + ' $SCRIPT ' + Dir('rasterizer/scripts').abspath
> +)
> +
> +env.CodeGenerate(
> +    target = 'rasterizer/jitter/state_llvm.h',
> +    script = 'rasterizer/jitter/scripts/gen_llvm_types.py',
> +    source = 'rasterizer/core/state.h',
> +    command = python_cmd + ' $SCRIPT --input $SOURCE --output $TARGET'
> +)
> +
> +env.CodeGenerate(
> +    target = 'rasterizer/jitter/builder_gen.h',
> +    script = 'rasterizer/jitter/scripts/gen_llvm_ir_macros.py',
> +    source = os.path.join(env['llvm_includedir'], 'llvm/IR/IRBuilder.h'),
> +    command = python_cmd + ' $SCRIPT --input $SOURCE --output $TARGET --gen_h'
> +)
> +
> +env.CodeGenerate(
> +    target = 'rasterizer/jitter/builder_gen.cpp',
> +    script = 'rasterizer/jitter/scripts/gen_llvm_ir_macros.py',
> +    source = os.path.join(env['llvm_includedir'], 'llvm/IR/IRBuilder.h'),
> +    command = python_cmd + ' $SCRIPT --input $SOURCE --output $TARGET --gen_cpp'
> +)
> +
> +env.CodeGenerate(
> +    target = 'rasterizer/jitter/builder_x86.h',
> +    script = 'rasterizer/jitter/scripts/gen_llvm_ir_macros.py',
> +    source = '',
> +    command = python_cmd + ' $SCRIPT --output $TARGET --gen_x86_h'
> +)
> +
> +env.CodeGenerate(
> +    target = 'rasterizer/jitter/builder_x86.cpp',
> +    script = 'rasterizer/jitter/scripts/gen_llvm_ir_macros.py',
> +    source = '',
> +    command = python_cmd + ' $SCRIPT --output $TARGET --gen_x86_cpp'
> +)
> +
> +source = [
> +       'rasterizer/scripts/gen_knobs.cpp', 'rasterizer/scripts/gen_knobs.h',
> +       'rasterizer/jitter/builder_gen.cpp', 'rasterizer/jitter/builder_gen.h',
> +       'rasterizer/jitter/builder_x86.cpp', 'rasterizer/jitter/builder_x86.h',
> +       ]
> +
> +source += env.ParseSourceList('../Makefile.sources-arch', [
> +    'CXX_SOURCES',
> +    'COMMON_CXX_SOURCES',
> +    'CORE_CXX_SOURCES',
> +    'JITTER_CXX_SOURCES',
> +    'MEMORY_CXX_SOURCES'
> +])
> +
> +# 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']))]
> +
.h are already handled - see commit d4a1f3fd270 "scons: do not include
headers from the sources lists". Please do a similar (and separate)
patch for .hpp files.

> +env.Prepend(LIBS = [ mesautil, mesa, gallium ])
> +
Please drop this. Linking should happen in the final stage (gallium/targets).

> +Import('swr_arch')
> +
> +if swr_arch == 'avx':
> +    env.Append(CPPDEFINES = ['KNOB_ARCH=KNOB_ARCH_AVX'])
> +    env.Append(CCFLAGS = ['-march=core-avx-i'])
> +    swrAVX = env.SharedLibrary(target = 'swrAVX',      source = source)
> +    env.Alias('swrAVX', swrAVX)
> +    Export('swrAVX')
> +elif swr_arch == 'avx2':
> +    env.Append(CPPDEFINES = ['KNOB_ARCH=KNOB_ARCH_AVX2'])
> +    env.Append(CCFLAGS = ['-march=core-avx2'])
> +    swrAVX2 = env.SharedLibrary(target = 'swrAVX2',    source = source)
> +    env.Alias('swrAVX2', swrAVX2)
> +    Export('swrAVX2')
> +else:
> +    print "unknown swr architecture"
> +    exit(1)
> diff --git a/src/gallium/drivers/swr/avx/Makefile.am b/src/gallium/drivers/swr/avx/Makefile.am
> new file mode 100644
> index 0000000..c09fb5a
> --- /dev/null
> +++ b/src/gallium/drivers/swr/avx/Makefile.am
> @@ -0,0 +1,112 @@
> +# Copyright (C) 2015 Intel Corporation.   All Rights Reserved.
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> +# IN THE SOFTWARE.
> +
> +AUTOMAKE_OPTIONS = subdir-objects
> +
Please nuke this.

> +include ../Makefile.sources-arch
> +include $(top_srcdir)/src/gallium/Automake.inc
> +
> +VPATH = $(srcdir) $(srcdir)/..
> +
Yuk why do we need this ?

> +AM_CXXFLAGS = \
> +       $(GALLIUM_DRIVER_CFLAGS) \
> +       -std=c++11 -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS \
We already set the latter two ?

> +       -march=core-avx-i -DKNOB_ARCH=KNOB_ARCH_AVX \
Please split these two on separate lines.

> +       $(LLVM_CFLAGS)
> +
> +lib_LTLIBRARIES = libswrAVX.la
> +
> +libswrAVX_la_SOURCES = $(CXX_SOURCES)
> +
> +libswrAVX_la_LDFLAGS =
> +
We don't need this.

> +BUILT_SOURCES = \
> +       rasterizer/scripts/gen_knobs.cpp \
> +       rasterizer/scripts/gen_knobs.h \
> +       rasterizer/jitter/state_llvm.h \
> +       rasterizer/jitter/builder_gen.h \
> +       rasterizer/jitter/builder_gen.cpp \
> +       rasterizer/jitter/builder_x86.h \
> +       rasterizer/jitter/builder_x86.cpp
> +
> +rasterizer/scripts/gen_knobs.cpp rasterizer/scripts/gen_knobs.h: rasterizer/scripts/gen_knobs.py rasterizer/scripts/knob_defs.py rasterizer/scripts/templates/knobs.template
> +       $(PYTHON2) $(PYTHON_FLAGS) \
> +               $(srcdir)/../rasterizer/scripts/gen_knobs.py \
> +               rasterizer/scripts
> +
> +rasterizer/jitter/state_llvm.h: rasterizer/jitter/scripts/gen_llvm_types.py rasterizer/core/state.h
> +       $(PYTHON2) $(PYTHON_FLAGS) \
> +               $(srcdir)/../rasterizer/jitter/scripts/gen_llvm_types.py \
> +               --input $(srcdir)/../rasterizer/core/state.h \
> +               --output rasterizer/jitter/state_llvm.h
> +
> +rasterizer/jitter/builder_gen.h: rasterizer/jitter/scripts/gen_llvm_ir_macros.py $(LLVM_INCLUDEDIR)/llvm/IR/IRBuilder.h
> +       $(PYTHON2) $(PYTHON_FLAGS) \
> +               $(srcdir)/../rasterizer/jitter/scripts/gen_llvm_ir_macros.py \
> +               --input $(LLVM_INCLUDEDIR)/llvm/IR/IRBuilder.h \
> +               --output rasterizer/jitter/builder_gen.h \
> +               --gen_h
> +
> +rasterizer/jitter/builder_gen.cpp: rasterizer/jitter/scripts/gen_llvm_ir_macros.py $(LLVM_INCLUDEDIR)/llvm/IR/IRBuilder.h
> +       $(PYTHON2) $(PYTHON_FLAGS) \
> +               $(srcdir)/../rasterizer/jitter/scripts/gen_llvm_ir_macros.py \
> +               --input $(LLVM_INCLUDEDIR)/llvm/IR/IRBuilder.h \
> +               --output rasterizer/jitter/builder_gen.cpp \
> +               --gen_cpp
> +
> +rasterizer/jitter/builder_x86.h: rasterizer/jitter/scripts/gen_llvm_ir_macros.py
> +       $(PYTHON2) $(PYTHON_FLAGS) \
> +               $(srcdir)/../rasterizer/jitter/scripts/gen_llvm_ir_macros.py \
> +               --output rasterizer/jitter/builder_x86.h \
> +               --gen_x86_h
> +
> +rasterizer/jitter/builder_x86.cpp: rasterizer/jitter/scripts/gen_llvm_ir_macros.py
> +       $(PYTHON2) $(PYTHON_FLAGS) \
> +               $(srcdir)/../rasterizer/jitter/scripts/gen_llvm_ir_macros.py \
> +               --output rasterizer/jitter/builder_x86.cpp \
> +               --gen_x86_cpp
> +
> +libswrAVX_la_SOURCES += \
> +       $(COMMON_CXX_SOURCES) \
> +       $(CORE_CXX_SOURCES) \
> +       $(JITTER_CXX_SOURCES) \
> +       $(MEMORY_CXX_SOURCES) \
> +       rasterizer/scripts/gen_knobs.cpp \
> +       rasterizer/scripts/gen_knobs.h \
> +       rasterizer/jitter/builder_gen.cpp \
> +       rasterizer/jitter/builder_gen.h \
> +       rasterizer/jitter/builder_x86.cpp \
> +       rasterizer/jitter/builder_x86.h
> +
Fold this with the initial libswrAVX_la_SOURCES ?

> +AM_CXXFLAGS += \
> +       -I$(srcdir)/../rasterizer \
> +       -I$(srcdir)/../rasterizer/core \
> +       -I$(srcdir)/../rasterizer/jitter \
> +       -I$(builddir)/rasterizer/scripts \
> +       -I$(builddir)/rasterizer/jitter
> +
builddir should be listed before their srcdir equivalent. Also please
fold this with the initial AM_CXXFLAGS above.

> +libswrAVX_la_LIBADD = \
> +       $(top_builddir)/src/gallium/auxiliary/libgallium.la \
> +       $(top_builddir)/src/mesa/libmesagallium.la
> +
Keep these in the final link stage.

> +include $(top_srcdir)/install-gallium-links.mk
> +
Those are used for compatibility reasons with some developers'
workflow. I doubt you want these.

> --- /dev/null
> +++ b/src/gallium/drivers/swr/avx2/Makefile.am

The avx comments apply here as well. Me is feeling like a sad panda
that we cannot do the clever scons trick in here somehow.


> --- a/src/gallium/targets/libgl-xlib/Makefile.am
> +++ b/src/gallium/targets/libgl-xlib/Makefile.am
> @@ -84,4 +84,9 @@ endif
>  EXTRA_lib at GL_LIB@_la_DEPENDENCIES = libgl-xlib.sym
>  EXTRA_DIST = SConscript libgl-xlib.sym
>
> +if HAVE_GALLIUM_SWR
> +lib at GL_LIB@_la_LIBADD += $(top_builddir)/src/gallium/drivers/swr/libmesaswr.la $(LLVM_LIBS)
> +AM_CPPFLAGS += -DGALLIUM_SWR
> +endif
> +
Please move this hunk up a few lines, just after the LLVM handling.


Thanks
Emil


More information about the mesa-dev mailing list