[Mesa-dev] [PATCH v5 1/5] genxml: New generated header genX_bits.h (v5)
Emil Velikov
emil.l.velikov at gmail.com
Tue Mar 28 16:48:43 UTC 2017
Hi Chad,
On 25 March 2017 at 02:28, Chad Versace <chadversary at chromium.org> wrote:
> genX_bits.h contains the sizes of bitfields in genxml instructions,
> structures, and registers. It also defines some functions to query those
> sizes.
>
> isl_surf_init() will use the new header to validate that requested
> pitches fit in their destination bitfields.
>
> What's currently in genX_bits.h:
>
> - Each CONTAINER::Field from gen*.xml that has a bitsize has a macro
> in genX_bits.h:
>
> #define GEN{N}_CONTAINER_Field_bits {bitsize}
>
> - For each set of macros whose name, after stripping the GEN prefix,
> is the same, genX_bits.h contains a query function:
>
> static inline uint32_t __attribute__((pure))
> CONTAINER_Field_bits(const struct gen_device_info *devinfo);
>
> v2 (Chad Versace):
> - Parse the XML instead of scraping the generated gen*_pack.h headers.
>
> v3 (Dylan Baker):
> - Port to Mako.
>
> v4 (Jason Ekstrand):
> - Make the _bits functions take a gen_device_info.
>
> v5 (Chad Versace):
> - Fix autotools out-of-tree build.
> - Fix Android build. Tested with git://github.com/android-ia/manifest.
> - Fix macro names. They were all missing the "_bits" suffix.
> - Fix macros names more. Remove all double-underscores.
> - Unindent all generated code. (It was floating in a sea of whitespace).
> - Reformat header to appear human-written not machine-generated.
> - Sort gens from high to low. Newest gens should come first because,
> when we read code, we likely want to read the gen8/9 code and ignore
> the gen4 code. So put the gen4 code at the bottom.
> - Replace 'const' attributes with 'pure', because the functions now
> have a pointer parameter.
> - Add --cpp-guard flag. Used by Android.
> - Kill class FieldCollection. After Jason's rewrite, it was just
> a dict.
>
> Co-authored-by: Dylan Baker <dylan at pnwbakers.com>
> Co-authored-by: Jason Ekstrand <jason at jlekstrand.net>
> ---
> src/intel/Android.genxml.mk | 9 +-
> src/intel/Makefile.genxml.am | 6 +-
> src/intel/Makefile.sources | 6 +-
> src/intel/genxml/.gitignore | 1 +
> src/intel/genxml/gen_bits_header.py | 281 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 300 insertions(+), 3 deletions(-)
> create mode 100644 src/intel/genxml/gen_bits_header.py
>
> diff --git a/src/intel/Android.genxml.mk b/src/intel/Android.genxml.mk
> index 79de7843801..842d0e13a33 100644
> --- a/src/intel/Android.genxml.mk
> +++ b/src/intel/Android.genxml.mk
> @@ -46,9 +46,16 @@ LOCAL_GENERATED_SOURCES += $(addprefix $(intermediates)/, $(GENXML_GENERATED_FIL
> define header-gen
> @mkdir -p $(dir $@)
> @echo "Gen Header: $(PRIVATE_MODULE) <= $(notdir $(@))"
> - $(hide) $(PRIVATE_SCRIPT) $(PRIVATE_XML) > $@
> + $(hide) $(PRIVATE_SCRIPT) $(PRIVATE_SCRIPT_FLAGS) $(PRIVATE_XML) > $@
> endef
>
> +$(intermediates)/genxml/genX_bits.h: PRIVATE_SCRIPT := $(MESA_PYTHON2) $(LOCAL_PATH)/genxml/gen_bits_header.py
> +$(intermediates)/genxml/genX_bits.h: PRIVATE_SCRIPT_FLAGS := --cpp-guard=GENX_BITS_H
> +$(intermediates)/genxml/genX_bits.h: PRIVATE_XML := $(addprefix $(LOCAL_PATH)/,$(GENXML_XML_FILES))
> +$(intermediates)/genxml/genX_bits.h: $(LOCAL_PATH)/genxml/gen_bits_header.py
> +$(intermediates)/genxml/genX_bits.h: $(addprefix $(LOCAL_PATH)/,$(GENXML_XML_FILES))
> + $(call header-gen)
> +
> $(intermediates)/genxml/gen4_pack.h: PRIVATE_SCRIPT := $(MESA_PYTHON2) $(LOCAL_PATH)/genxml/gen_pack_header.py
> $(intermediates)/genxml/gen4_pack.h: PRIVATE_XML := $(LOCAL_PATH)/genxml/gen4.xml
> $(intermediates)/genxml/gen4_pack.h: $(LOCAL_PATH)/genxml/gen4.xml $(LOCAL_PATH)/genxml/gen_pack_header.py
> diff --git a/src/intel/Makefile.genxml.am b/src/intel/Makefile.genxml.am
> index 01a02b63b44..474b751f5fd 100644
> --- a/src/intel/Makefile.genxml.am
> +++ b/src/intel/Makefile.genxml.am
> @@ -30,7 +30,7 @@ EXTRA_DIST += \
>
> SUFFIXES = _pack.h _xml.h .xml
>
> -$(GENXML_GENERATED_FILES): genxml/gen_pack_header.py
> +$(GENXML_GENERATED_PACK_FILES): genxml/gen_pack_header.py
>
> .xml_pack.h:
> $(MKDIR_GEN)
> @@ -42,6 +42,10 @@ $(AUBINATOR_GENERATED_FILES): genxml/gen_zipped_file.py
> $(MKDIR_GEN)
> $(AM_V_GEN) $(PYTHON2) $(srcdir)/genxml/gen_zipped_file.py $< > $@ || ($(RM) $@; false)
>
> +genxml/genX_bits.h: genxml/gen_bits_header.py $(GENXML_XML_FILES)
> + $(MKDIR_GEN)
> + $(PYTHON_GEN) $< -o $@ $(addprefix $(srcdir)/,$(GENXML_XML_FILES))
> +
> EXTRA_DIST += \
> genxml/genX_pack.h \
> genxml/gen_macros.h \
> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> index 88bcf60f6e3..c56891643ce 100644
> --- a/src/intel/Makefile.sources
> +++ b/src/intel/Makefile.sources
> @@ -119,7 +119,7 @@ GENXML_XML_FILES = \
> genxml/gen8.xml \
> genxml/gen9.xml
>
> -GENXML_GENERATED_FILES = \
> +GENXML_GENERATED_PACK_FILES = \
> genxml/gen4_pack.h \
> genxml/gen45_pack.h \
> genxml/gen5_pack.h \
> @@ -129,6 +129,10 @@ GENXML_GENERATED_FILES = \
> genxml/gen8_pack.h \
> genxml/gen9_pack.h
>
> +GENXML_GENERATED_FILES = \
> + $(GENXML_GENERATED_PACK_FILES) \
> + genxml/genX_bits.h
> +
> AUBINATOR_GENERATED_FILES = \
> genxml/gen6_xml.h \
> genxml/gen7_xml.h \
> diff --git a/src/intel/genxml/.gitignore b/src/intel/genxml/.gitignore
> index c5672b5595c..3e2f1cfa9f0 100644
> --- a/src/intel/genxml/.gitignore
> +++ b/src/intel/genxml/.gitignore
> @@ -1,2 +1,3 @@
> +gen*_bits.h
> gen*_pack.h
> gen*_xml.h
> diff --git a/src/intel/genxml/gen_bits_header.py b/src/intel/genxml/gen_bits_header.py
> new file mode 100644
> index 00000000000..a7006457794
> --- /dev/null
> +++ b/src/intel/genxml/gen_bits_header.py
> @@ -0,0 +1,281 @@
> +#encoding=utf-8
> +# Copyright © 2017 Intel Corporation
> +
> +# 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 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.
> +
> +from __future__ import (
> + absolute_import, division, print_function, unicode_literals
> +)
> +
> +import argparse
> +import os
> +import sys
> +import xml.parsers.expat
> +
> +from mako.template import Template
> +
> +TEMPLATE = Template("""\
> +<%!
> +from operator import itemgetter
> +%>\
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * 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.
> + */
> +
> +/* THIS FILE HAS BEEN GENERATED, DO NOT HAND EDIT.
> + *
> + * Sizes of bitfields in genxml instructions, structures, and registers.
> + */
> +
> +#ifndef ${guard}
> +#define ${guard}
> +
> +#include <stdint.h>
> +
> +#include "common/gen_device_info.h"
> +#include "util/macros.h"
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +% for _, field in sorted(fields.iteritems(), key=itemgetter(0)):
> +
> +/* ${field.container_name}::${field.name} */
> +% for gen, bits in sorted(field.bits_by_gen.iteritems(), reverse=True):
> +#define ${gen.prefix(field.token_name, padded=True)} ${bits}
> +% endfor
> +
> +static inline uint32_t ATTRIBUTE_PURE
> +${field.token_name}(const struct gen_device_info *devinfo)
> +{
> + switch (devinfo->gen) {
> + case 9: return ${field.bits(9)};
> + case 8: return ${field.bits(8)};
> + case 7:
> + if (devinfo->is_haswell) {
> + return ${field.bits(7.5)};
> + } else {
> + return ${field.bits(7)};
> + }
> + case 6: return ${field.bits(6)};
> + case 5: return ${field.bits(5)};
> + case 4:
> + if (devinfo->is_g4x) {
> + return ${field.bits(4.5)};
> + } else {
> + return ${field.bits(4)};
> + }
> + default:
> + unreachable("Invalid hardware generation");
> + }
> +}
> +% endfor
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* ${guard} */""", output_encoding='utf-8')
> +
> +def to_alphanum(name):
> + substitutions = {
> + ' ': '',
> + '/': '',
> + '[': '',
> + ']': '',
> + '(': '',
> + ')': '',
> + '-': '',
> + ':': '',
> + '.': '',
> + ',': '',
> + '=': '',
> + '>': '',
> + '#': '',
> + 'α': 'alpha',
> + '&': '',
> + '*': '',
> + '"': '',
> + '+': '',
> + '\'': '',
> + }
> +
> + for i, j in substitutions.items():
> + name = name.replace(i, j)
> +
> + return name
> +
> +def safe_name(name):
> + name = to_alphanum(name)
> + if not name[0].isalpha():
> + name = '_' + name
> + return name
> +
> +class Gen(object):
> +
> + def __init__(self, z):
> + # Convert potential "major.minor" string
> + z = float(z)
> + if z < 10:
> + z *= 10
> + self.tenx = int(z)
> +
> + def __lt__(self, other):
> + return self.tenx < other.tenx
> +
> + def __hash__(self):
> + return hash(self.tenx)
> +
> + def __eq__(self, other):
> + return self.tenx == other.tenx
> +
> + def prefix(self, token, padded=False):
> + gen = self.tenx
> + pad = ''
> +
> + if gen % 10 == 0:
> + gen //= 10
> + if padded:
> + pad = ' '
> +
> + if token[0] == '_':
> + token = token[1:]
> +
> + return 'GEN{}_{}{}'.format(gen, token, pad)
> +
> +class Field(object):
> +
> + def __init__(self, container_name, name):
> + self.container_name = container_name
> + self.name = name
> + self.token_name = safe_name('_'.join([self.container_name, self.name, 'bits']))
> + self.bits_by_gen = {}
> +
> + def add_gen(self, gen, xml_attrs):
> + assert isinstance(gen, Gen)
> + start = int(xml_attrs['start'])
> + end = int(xml_attrs['end'])
> + self.bits_by_gen[gen] = 1 + end - start
> +
> + def bits(self, gen):
> + if not isinstance(gen, Gen):
> + gen = Gen(gen)
> + return self.bits_by_gen.get(gen, 0)
> +
> +class XmlParser(object):
> +
> + def __init__(self, fields):
> + self.parser = xml.parsers.expat.ParserCreate()
> + self.parser.StartElementHandler = self.start_element
> + self.parser.EndElementHandler = self.end_element
> +
> + self.gen = None
> + self.container_name = None
> + self.fields = fields
> +
> + def parse(self, filename):
> + with open(filename) as f:
> + self.parser.ParseFile(f)
> +
> + def start_element(self, name, attrs):
> + if name == 'genxml':
> + self.gen = Gen(attrs['gen'])
> + elif name in ('instruction', 'struct', 'register'):
> + self.start_container(attrs)
> + elif name == 'field':
> + self.start_field(attrs)
> + else:
> + pass
> +
> + def end_element(self, name):
> + if name == 'genxml':
> + self.gen = None
> + elif name in ('instruction', 'struct', 'register'):
> + self.container_name = None
> + else:
> + pass
> +
> + def start_container(self, attrs):
> + assert self.container_name is None
> + self.container_name = attrs['name']
> +
> + def start_field(self, attrs):
> + if self.container_name is None:
> + return
> +
> + field_name = attrs.get('name', None)
> + if not field_name:
> + return
> +
> + key = (self.container_name, field_name)
> + if key not in self.fields.keys():
> + self.fields[key] = Field(self.container_name, field_name)
> + self.fields[key].add_gen(self.gen, attrs)
> +
> +def parse_args():
> + p = argparse.ArgumentParser()
> + p.add_argument('-o', '--output', type=str,
> + help="If OUTPUT is unset or '-', then it defaults to '/dev/stdout'")
> + p.add_argument('--cpp-guard', type=str,
> + help='If unset, then CPP_GUARD is derived from OUTPUT.')
Admittedly --output should be enough for everyone, but I'm not sure
what issues you're getting with Android-IA.
For the future rather please, poke me/others for ideas/help. Be that
on #dri-devel, #intel-gfx or #android-ia.
As-is, the patch is
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
-Emil
More information about the mesa-dev
mailing list