[Mesa-dev] [PATCH 1/8] swr/rast: Split backend.cpp to improve compile time

Mon Jun 26 12:57:08 UTC 2017

Hi Tim,

On 22 June 2017 at 22:13, Tim Rowley <timothy.o.rowley at intel.com> wrote:
> Hardcode split to four files currently.  Decreases swr build
> time on a quad-core by ~10%.
> ---
>  src/gallium/drivers/swr/Makefile.am                |   26 +-
>  src/gallium/drivers/swr/Makefile.sources           |    4 +
>  src/gallium/drivers/swr/SConscript                 |   19 +-
>  .../drivers/swr/rasterizer/codegen/gen_backends.py |   38 +-
>  .../drivers/swr/rasterizer/codegen/gen_common.py   |    7 +
>  .../rasterizer/codegen/templates/gen_backend.cpp   |    1 +
>  .../codegen/templates/gen_header_init.hpp          |   43 +
>  src/gallium/drivers/swr/rasterizer/core/api.cpp    |    7 +-
>  .../drivers/swr/rasterizer/core/backend.cpp        |  809 +--------------
>  src/gallium/drivers/swr/rasterizer/core/backend.h  | 1033 +------------------
>  .../drivers/swr/rasterizer/core/backend_clear.cpp  |  281 ++++++
>  .../drivers/swr/rasterizer/core/backend_impl.h     | 1067 ++++++++++++++++++++
>  .../drivers/swr/rasterizer/core/backend_sample.cpp |  345 +++++++
>  .../swr/rasterizer/core/backend_singlesample.cpp   |  321 ++++++
>  14 files changed, 2160 insertions(+), 1841 deletions(-)
>  create mode 100644 src/gallium/drivers/swr/rasterizer/codegen/templates/gen_header_init.hpp
>  create mode 100644 src/gallium/drivers/swr/rasterizer/core/backend_clear.cpp
>  create mode 100644 src/gallium/drivers/swr/rasterizer/core/backend_impl.h
>  create mode 100644 src/gallium/drivers/swr/rasterizer/core/backend_sample.cpp
>  create mode 100644 src/gallium/drivers/swr/rasterizer/core/backend_singlesample.cpp
>
> diff --git a/src/gallium/drivers/swr/Makefile.am b/src/gallium/drivers/swr/Makefile.am
> index 6650abd..0daec90 100644
> --- a/src/gallium/drivers/swr/Makefile.am
> +++ b/src/gallium/drivers/swr/Makefile.am
> @@ -34,6 +34,7 @@ COMMON_CXXFLAGS = \
>         $(LLVM_CXXFLAGS) \
>         $(SWR_CXX11_CXXFLAGS) \
>         -I$(builddir)/rasterizer/codegen \
> +       -I$(builddir)/rasterizer/core \
>         -I$(builddir)/rasterizer/jitter \
>         -I$(builddir)/rasterizer/archrast \
>         -I$(srcdir)/rasterizer \
> @@ -62,7 +63,11 @@ BUILT_SOURCES = \
>         rasterizer/archrast/gen_ar_event.cpp \
>         rasterizer/archrast/gen_ar_eventhandler.hpp \
>         rasterizer/archrast/gen_ar_eventhandlerfile.hpp \
> -       rasterizer/core/gen_BackendPixelRate0.cpp
> +       rasterizer/core/backends/gen_BackendPixelRate0.cpp \
> +       rasterizer/core/backends/gen_BackendPixelRate1.cpp \
> +       rasterizer/core/backends/gen_BackendPixelRate2.cpp \
> +       rasterizer/core/backends/gen_BackendPixelRate3.cpp \
> +       rasterizer/core/backends/gen_BackendPixelRate.hpp
>
>  MKDIR_GEN = $(AM_V_at)$(MKDIR_P) $(@D)
>  PYTHON_GEN = $(AM_V_GEN)$(PYTHON2) $(PYTHON_FLAGS)
> @@ -140,20 +145,30 @@ rasterizer/archrast/gen_ar_eventhandlerfile.hpp: rasterizer/codegen/gen_archrast
>                 --output rasterizer/archrast/gen_ar_eventhandlerfile.hpp \
>                 --gen_eventhandlerfile_h
>
> +rasterizer/core/backends/gen_BackendPixelRate0.cpp \
> +rasterizer/core/backends/gen_BackendPixelRate1.cpp \
> +rasterizer/core/backends/gen_BackendPixelRate2.cpp \
> +rasterizer/core/backends/gen_BackendPixelRate3.cpp \
> +rasterizer/core/backends/gen_BackendPixelRate.hpp: \
> +backend.intermediate
> +
>  # 5 SWR_MULTISAMPLE_TYPE_COUNT
>  # 2 SWR_MSAA_SAMPLE_PATTERN_COUNT
>  # 3 SWR_INPUT_COVERAGE_COUNT
>  # 2 centroid
>  # 2 forcedSampleCount
>  # 2 canEarlyZ
> -rasterizer/core/gen_BackendPixelRate0.cpp: rasterizer/codegen/gen_backends.py rasterizer/codegen/templates/gen_backend.cpp
> +
> +.INTERMEDIATE: backend.intermediate
I have limited experience with .INTERMEDIATE and it didn't seem to
bring single/incremental build times improvements.
Have you seen any on your end? If not I'll just drop it.

> +backend.intermediate: rasterizer/codegen/gen_backends.py rasterizer/codegen/templates/gen_backend.cpp rasterizer/codegen/templates/gen_header_init.hpp
>         $(MKDIR_GEN)
>         $(PYTHON_GEN) \
>                 $(srcdir)/rasterizer/codegen/gen_backends.py \
> -               --outdir rasterizer/core \
> +               --outdir rasterizer/core/backends \
>                 --dim 5 2 3 2 2 2 \
> -               --split 0 \
> -               --cpp
> +               --numfiles 4 \
> +               --cpp \
> +               --hpp
>
Hardcoding file names in generator scripts tends to be a bad idea. One
example is the extra code needed to generate the cmake bits :-)
One could prune that, but it's not a priority AFAICT.

> --- a/src/gallium/drivers/swr/rasterizer/codegen/gen_backends.py
> +++ b/src/gallium/drivers/swr/rasterizer/codegen/gen_backends.py
> @@ -1,7 +1,7 @@
>  # Copyright (C) 2017 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"),
> +# 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
> @@ -11,7 +11,7 @@
>  # 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
> +# 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
> @@ -31,23 +31,28 @@ from gen_common import ArgumentParser, MakoTemplateWriter
>
>  def main(args=sys.argv[1:]):
>      thisDir = os.path.dirname(os.path.realpath(__file__))
> -    parser = ArgumentParser("Generate files and initialization functions for all permutuations of BackendPixelRate.")
> -    parser.add_argument('--dim', help="gBackendPixelRateTable array dimensions", nargs='+', type=int, required=True)
> -    parser.add_argument('--outdir', help="output directory", nargs='?', type=str, default=thisDir)
> -    parser.add_argument('--split', help="how many lines of initialization per file [0=no split]", nargs='?', type=int, default='512')
> -    parser.add_argument('--cpp', help="Generate cpp file(s)", action='store_true', default=False)
> -    parser.add_argument('--cmake', help="Generate cmake file", action='store_true', default=False)
> +    parser = ArgumentParser('Generate files and initialization functions for all permutuations of BackendPixelRate.')
> +    parser.add_argument('--dim', help='gBackendPixelRateTable array dimensions', nargs='+', type=int, required=True)
> +    parser.add_argument('--outdir', help='output directory', nargs='?', type=str, default=thisDir)
> +    parser.add_argument('--split', help='how many lines of initialization per file [0=no split]', nargs='?', type=int, default='512')
> +    parser.add_argument('--numfiles', help='how many output files to generate', nargs='?', type=int, default='0')
> +    parser.add_argument('--cpp', help='Generate cpp file(s)', action='store_true', default=False)
> +    parser.add_argument('--hpp', help='Generate hpp file', action='store_true', default=False)
> +    parser.add_argument('--cmake', help='Generate cmake file', action='store_true', default=False)
>
Most of the above seem unrelated cleanups/fixes. Please keep them separate.

> @@ -95,6 +102,19 @@ def main(args=sys.argv[1:]):
>                  fileNum=fileNum,
>                  funcList=chunkedList[fileNum])
>
> +    if args.hpp:
> +        baseHppName = os.path.join(args.outdir, backend.outHeaderName)
> +        templateHpp = os.path.join(thisDir, 'templates', backend.hpp_template)
> +
> +        #print('Generating', filename)
[snip]

>      # generate gen_backend.cmake file
>      if args.cmake:
>          templateCmake = os.path.join(thisDir, 'templates', 'gen_backend.cmake')
> @@ -108,7 +128,7 @@ def main(args=sys.argv[1:]):
>              numFiles=numFiles,
>              baseCppName='${RASTY_GEN_SRC_DIR}/backends/' + os.path.basename(baseCppName))
>
> -    #print("Generated %d template instantiations in %d files" % (len(output_list), numFiles))
> +    #print('Generated %d template instantiations in %d files' % (len(output_list), numFiles))
>
Uncomment these prints or drop them? They don't seem to bring much as-is.

With the .INTERMEDIATE commente (2-3 lines just above it) or dropped,
the build bits are
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

-Emil