[Piglit] [PATCH 2/2] generators/gen_vs_in_fp64: Use modules.types

Andres Gomez agomez at igalia.com
Mon Jun 13 16:37:12 UTC 2016


Other than my comment in the previous patch about (type.columns or 1)
and (type.rows or 1), consider this:

Reviewed-by: Andres Gomez <agomez at igalia.com>

On Fri, 2016-06-10 at 16:42 -0700, Dylan Baker wrote:
> This removes a significant amount of string parsing from both the
> module
> and the templates, and simplifies some statements. The result is that
> the generator is roughly 50% faster.
> 
> This string parsing comes from the rows and cols functions, which are
> present in both the generator and the templates. Each of these
> functions
> gets called roughly 40 million times. 40 million. The cols function
> in
> particular accounts for nearly 20 seconds of runtime (and the rows
> about
> 5). The replacements don't even show up in the profile.
> 
> When you consider that the runtime is about 70 seconds before this
> patch, and about 35 after, that's another big win, probably enough
> that
> it's not worth optimizing it further. I can't come up with any big
> enhancements, maybe a few little ones that might amount to a couple
> of
> seconds.
> 
> The remaining significant time sinks are the collections.deque (which
> is
> called from within mako and is part of the standard library, we can't
> optimize it) and the regular.shader_test.mako. While I can spot some
> micro optimizations that might get us a few tenths of a second here
> and
> there, I don't see anything that's going to win another 10 or 15
> seconds.
> 
> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
> ---
>  generated_tests/gen_vs_in_fp64.py                  | 75 ++++++++--
> ------------
>  .../gen_vs_in_fp64/columns.shader_test.mako        | 40 ++++--------
>  .../gen_vs_in_fp64/regular.shader_test.mako        | 40 ++++--------
>  3 files changed, 48 insertions(+), 107 deletions(-)
> 
> diff --git a/generated_tests/gen_vs_in_fp64.py
> b/generated_tests/gen_vs_in_fp64.py
> index fcc87f5..88c4952 100644
> --- a/generated_tests/gen_vs_in_fp64.py
> +++ b/generated_tests/gen_vs_in_fp64.py
> @@ -34,6 +34,7 @@ from six.moves import range
>  
>  from templates import template_dir
>  from modules import utils
> +from modules import types as glsltypes
>  
>  TEMPLATES =
> template_dir(os.path.basename(os.path.splitext(__file__)[0]))
>  
> @@ -71,29 +72,29 @@ DOUBLE_NORMAL_VALUES   = ['0xffefffffffffffff', #
> Negative maximum normalized
>                            '0x4b1e35ed24eb6496', # +7.23401345e+53
>                            '0x7fefffffffffffff'] # Positive maximum
> normalized
>  
> -DSCALAR_TYPES          = ['double']
> +DSCALAR_TYPES          = [glsltypes.DOUBLE]
>  
> -DVEC_TYPES             = ['dvec2', 'dvec3', 'dvec4']
> +DVEC_TYPES             = [glsltypes.DVEC2, glsltypes.DVEC3,
> glsltypes.DVEC4]
>  
> -DMAT_TYPES             = ['dmat2', 'dmat2x3', 'dmat2x4',
> -                          'dmat3x2', 'dmat3', 'dmat3x4',
> -                          'dmat4x2', 'dmat4x3', 'dmat4']
> +DMAT_TYPES             = [glsltypes.DMAT2, glsltypes.DMAT2X3,
> glsltypes.DMAT2X4,
> +                          glsltypes.DMAT3X2, glsltypes.DMAT3,
> glsltypes.DMAT3X4,
> +                          glsltypes.DMAT4X2, glsltypes.DMAT4X3,
> glsltypes.DMAT4]
>  
> -FSCALAR_TYPES          = ['float']
> +FSCALAR_TYPES          = [glsltypes.FLOAT]
>  
> -FVEC_TYPES             = ['vec2', 'vec3', 'vec4']
> +FVEC_TYPES             = [glsltypes.VEC2, glsltypes.VEC3,
> glsltypes.VEC4]
>  
> -FMAT_TYPES             = ['mat2', 'mat2x3', 'mat2x4',
> -                          'mat3x2', 'mat3', 'mat3x4',
> -                          'mat4x2', 'mat4x3', 'mat4']
> +FMAT_TYPES             = [glsltypes.MAT2, glsltypes.MAT2X3,
> glsltypes.MAT2X4,
> +                          glsltypes.MAT3X2, glsltypes.MAT3,
> glsltypes.MAT3X4,
> +                          glsltypes.MAT4X2, glsltypes.MAT4X3,
> glsltypes.MAT4]
>  
> -ISCALAR_TYPES          = ['int']
> +ISCALAR_TYPES          = [glsltypes.INT]
>  
> -IVEC_TYPES             = ['ivec2', 'ivec3', 'ivec4']
> +IVEC_TYPES             = [glsltypes.IVEC2, glsltypes.IVEC3,
> glsltypes.IVEC4]
>  
> -USCALAR_TYPES          = ['uint']
> +USCALAR_TYPES          = [glsltypes.UINT]
>  
> -UVEC_TYPES             = ['uvec2', 'uvec3', 'uvec4']
> +UVEC_TYPES             = [glsltypes.UVEC2, glsltypes.UVEC3,
> glsltypes.UVEC4]
>  
>  HEX_VALUES_32BIT       = ['0xc21620c5', # -3.7532        float,
> -1038737211 int, 3256230085 uint
>                            '0x75bc289b',
> #  4.7703e32     float,  1975265435 int, 1975265435 uint
> @@ -121,26 +122,6 @@ class TestTuple(object):
>      """A float64 derived and other type derived tuple to generate
> the
>         needed conversion tests.
>      """
> -
> -    @staticmethod
> -    def rows(in_type):
> -        """Calculates the amounts of rows in a basic GLSL type."""
> -        if 'vec' in in_type or 'mat' in in_type:
> -            return int(in_type[-1:])
> -        else:
> -            return 1
> -
> -    @staticmethod
> -    def cols(in_type):
> -        """Calculates the amounts of columns in a basic GLSL
> type."""
> -        if 'mat' in in_type:
> -            if 'x' in in_type:
> -                return int(in_type[-3:][:1])
> -            else:
> -                return int(in_type[-1:])
> -        else:
> -            return 1
> -
>      @staticmethod
>      def get_dir_name(ver):
>          """Returns the directory name to save tests given a GLSL
> version."""
> @@ -191,20 +172,17 @@ class RegularTestTuple(TestTuple):
>              for ver in glsl_vers:
>                  utils.safe_makedirs(TestTuple.get_dir_name(ver))
>  
> -        for in_types, position_order, arrays, ver in
> itertools.product(in_types_array,
> -                                                                    
>    position_orders,
> -                                                                    
>    arrays_array,
> -                                                                    
>    glsl_vers):
> +        for in_types, position_order, arrays, ver in
> itertools.product(
> +                in_types_array,
> +                position_orders,
> +                arrays_array,
> +                glsl_vers):
>              num_vs_in = 1 # We use an additional vec3 piglit_vertex
> input
>              for idx, in_type in enumerate(in_types):
> -                if ((in_type.startswith('dvec') or
> in_type.startswith('dmat'))
> -                    and (in_type.endswith('3') or
> in_type.endswith('4'))):
> -                    multiplier = 2
> -                else:
> -                    multiplier = 1
> -                num_vs_in += TestTuple.cols(in_type) * arrays[idx] *
> multiplier
> +                num_vs_in += (in_type.columns or 1) * arrays[idx] *
> \
> +                    (2 if in_type.type.name == 'double' and
> in_type.rows in [3, 4] else 1)
>                  # dvec* and dmat* didn't appear in GLSL until 4.20
> -                if (in_type.startswith('dvec') or
> in_type.startswith('dmat')) and ver == '410':
> +                if (in_type.type.name == 'double' and not
> in_type.scalar) and ver == '410':
>                      ver = '420'
>              # Skip the test if it needs too many inputs
>              if num_vs_in > MAX_VERTEX_ATTRIBS:
> @@ -277,13 +255,12 @@ class RegularTestTuple(TestTuple):
>  
>      def generate(self):
>          """Generate GLSL parser tests."""
> -
>          filename = os.path.join(TestTuple.get_dir_name(self._ver),
> 'vs-input')
>          for idx, in_type in enumerate(self._in_types):
>              if idx == self._position_order - 1:
>                  filename += '-position'
>              filename += '-{}{}'.format(
> -                in_type, '-array{}'.format(
> +                in_type.name, '-array{}'.format(
>                      self._arrays[idx]) if self._arrays[idx] - 1 else
> '')
>          if self._position_order > len(self._in_types):
>              filename += '-position'
> @@ -320,7 +297,7 @@ class ColumnsTestTuple(TestTuple):
>                  utils.safe_makedirs(TestTuple.get_dir_name(ver))
>  
>          for mat in DMAT_TYPES:
> -            for columns in itertools.product(range(2),
> repeat=TestTuple.cols(mat)):
> +            for columns in itertools.product(range(2),
> repeat=mat.columns):
>                  if (0 not in columns) or (1 not in columns):
>                      continue
>                  for ver in glsl_vers:
> @@ -337,7 +314,7 @@ class ColumnsTestTuple(TestTuple):
>          """Generate GLSL parser tests."""
>  
>          filename = os.path.join(TestTuple.get_dir_name(self._ver),
> -                                'vs-input-columns-
> {}'.format(self._mat))
> +                                'vs-input-columns-
> {}'.format(self._mat.name))
>          for idx, column in enumerate(self._columns):
>              if column == 1:
>                  filename += '-{}'.format(idx)
> diff --git
> a/generated_tests/templates/gen_vs_in_fp64/columns.shader_test.mako
> b/generated_tests/templates/gen_vs_in_fp64/columns.shader_test.mako
> index 0da2678..241c02b 100644
> ---
> a/generated_tests/templates/gen_vs_in_fp64/columns.shader_test.mako
> +++
> b/generated_tests/templates/gen_vs_in_fp64/columns.shader_test.mako
> @@ -31,24 +31,6 @@
>        glsl_version = '{}.{}'.format(glsl_version_int[0],
> glsl_version_int[1:3])
>  
>        return (glsl_version, glsl_version_int)
> -
> -
> -  def cols(in_type):
> -      if 'mat' in in_type:
> -          if 'x' in in_type:
> -              return int(in_type[-3:][:1])
> -          else:
> -              return int(in_type[-1:])
> -      else:
> -          return 1
> -
> -
> -  def rows(in_type):
> -      if 'vec' in in_type or 'mat' in in_type:
> -          return int(in_type[-1:])
> -      else:
> -          return 1
> -
>  %>
>  <% glsl, glsl_int = _version(ver) %>
>  
> @@ -66,9 +48,9 @@ GLSL >= ${glsl}
>    #extension GL_ARB_vertex_attrib_64bit : require
>  % endif
>  
> -uniform ${mat} expected;
> +uniform ${mat.name} expected;
>  
> -in ${mat} value;
> +in ${mat.name} value;
>  in vec3 piglit_vertex;
>  out vec4 fs_color;
>  
> @@ -103,16 +85,16 @@ void main()
>  
>  [vertex data]
>  piglit_vertex/vec3/3\
> -  % for i in range(cols(mat)):
> -   value/${mat}/${rows(mat)}${'/{}'.format(i) if cols(mat) > 1 else
> ''}\
> +  % for i in range(mat.columns):
> +   value/${mat.name}/${mat.rows}${'/{}'.format(i) if mat.columns > 1
> else ''}\
>    % endfor
>  
>  % for d in range(len(dvalues)):
>    % for vertex in ('-1.0 -1.0  0.0', ' 1.0 -1.0  0.0', '
> 1.0  1.0  0.0', '-1.0  1.0  0.0'):
>  ${vertex} \
> -    % for i in range(cols(mat)):
> -      % for j in range(rows(mat)):
> -${dvalues[(d + i * rows(mat) + j) % len(dvalues)]}  \
> +    % for i in range(mat.columns):
> +      % for j in range(mat.rows):
> +${dvalues[(d + i * mat.rows + j) % len(dvalues)]}  \
>        % endfor
>    \
>      % endfor
> @@ -123,10 +105,10 @@ ${dvalues[(d + i * rows(mat) + j) %
> len(dvalues)]}  \
>  [test]
>  % for d in range(len(dvalues)):
>  
> -  uniform ${mat} expected\
> -  % for i in range(cols(mat)):
> -    % for j in range(rows(mat)):
> -     ${dvalues[(d + i * rows(mat) + j) % len(dvalues)]}\
> +  uniform ${mat.name} expected\
> +  % for i in range(mat.columns):
> +    % for j in range(mat.rows):
> +     ${dvalues[(d + i * mat.rows + j) % len(dvalues)]}\
>      % endfor
>    % endfor
>  
> diff --git
> a/generated_tests/templates/gen_vs_in_fp64/regular.shader_test.mako
> b/generated_tests/templates/gen_vs_in_fp64/regular.shader_test.mako
> index b727b5e..1a76230 100644
> ---
> a/generated_tests/templates/gen_vs_in_fp64/regular.shader_test.mako
> +++
> b/generated_tests/templates/gen_vs_in_fp64/regular.shader_test.mako
> @@ -31,24 +31,6 @@
>        glsl_version = '{}.{}'.format(glsl_version_int[0],
> glsl_version_int[1:3])
>  
>        return (glsl_version, glsl_version_int)
> -
> -
> -  def cols(in_type):
> -      if 'mat' in in_type:
> -          if 'x' in in_type:
> -              return int(in_type[-3:][:1])
> -          else:
> -              return int(in_type[-1:])
> -      else:
> -          return 1
> -
> -
> -  def rows(in_type):
> -      if 'vec' in in_type or 'mat' in in_type:
> -          return int(in_type[-1:])
> -      else:
> -          return 1
> -
>  %>
>  <% glsl, glsl_int = _version(ver) %>
>  
> @@ -68,10 +50,10 @@ GL_MAX_VERTEX_ATTRIBS >= ${num_vs_in}
>  % endif
>  
>  % for idx, in_type in enumerate(in_types):
> -  uniform ${in_type} expected${idx}${'[{}]'.format(arrays[idx]) if
> arrays[idx] - 1 else ''};
> +  uniform ${in_type.name} expected${idx}${'[{}]'.format(arrays[idx])
> if arrays[idx] - 1 else ''};
>  % endfor
>  % for idx, in_type in enumerate(in_types):
> -  in ${in_type} value${idx}${'[{}]'.format(arrays[idx]) if
> arrays[idx] - 1 else ''};
> +  in ${in_type.name} value${idx}${'[{}]'.format(arrays[idx]) if
> arrays[idx] - 1 else ''};
>  % endfor
>  in vec3 piglit_vertex;
>  out vec4 fs_color;
> @@ -109,8 +91,8 @@ void main()
>      piglit_vertex/vec3/3 \
>    % endif
>    % for i in range(arrays[idx]):
> -    % for j in range(cols(in_type)):
> -    value${idx}${'[{}]'.format(i) if arrays[idx] > 1 else
> ''}/${in_type}/${rows(in_type)}${'/{}'.format(j) if cols(in_type) > 1
> else ''} \
> +    % for j in range(in_type.columns or 1):
> +    value${idx}${'[{}]'.format(i) if arrays[idx] > 1 else
> ''}/${in_type.name}/${(in_type.rows or 1)}${'/{}'.format(j) if
> (in_type.columns or 0) > 1 else ''} \
>      % endfor
>    % endfor
>  % endfor
> @@ -125,9 +107,9 @@ void main()
>          ${vertex}   \
>        % endif
>        % for i in range(arrays[idx]):
> -        % for j in range(cols(in_type)):
> -          % for k in range(rows(in_type)):
> -            ${dvalues[(d + (i * cols(in_type) + j) * rows(in_type) +
> k) % len(dvalues)] if in_type.startswith('d') else hvalues[(d + (i *
> cols(in_type) + j) * rows(in_type) + k) % len(hvalues)]}  \
> +        % for j in range(in_type.columns or 1):
> +          % for k in range(in_type.rows or 1):
> +            ${dvalues[(d + (i * (in_type.columns or 1) + j) *
> (in_type.rows or 1) + k) % len(dvalues)] if in_type.type.name ==
> 'double' else hvalues[(d + (i * (in_type.columns or 1) + j) *
> (in_type.rows or 1) + k) % len(hvalues)]}  \
>            % endfor
>           \
>          % endfor
> @@ -145,10 +127,10 @@ void main()
>  
>    % for idx, in_type in enumerate(in_types):
>      % for i in range(arrays[idx]):
> -      uniform ${in_type} expected${idx}${'[{}]'.format(i) if
> arrays[idx] > 1 else ''}\
> -      % for j in range(cols(in_type)):
> -        % for k in range(rows(in_type)):
> -         ${dvalues[(d + (i * cols(in_type) + j) * rows(in_type) + k)
> % len(dvalues)] if in_type.startswith('d') else hvalues[(d + (i *
> cols(in_type) + j) * rows(in_type) + k) % len(hvalues)]}\
> +      uniform ${in_type.name} expected${idx}${'[{}]'.format(i) if
> arrays[idx] > 1 else ''}\
> +      % for j in range(in_type.columns or 1):
> +        % for k in range(in_type.rows or 1):
> +         ${dvalues[(d + (i * (in_type.columns or 1) + j) *
> (in_type.rows or 1) + k) % len(dvalues)] if in_type.type.name ==
> 'double' else hvalues[(d + (i * (in_type.columns or 1) + j) *
> (in_type.rows or 1) + k) % len(hvalues)]}\
>          % endfor
>        % endfor
>  
-- 
Br,

Andres



More information about the Piglit mailing list