[Mesa-dev] [PATCH v5 1/5] genxml: New generated header genX_bits.h (v5)

[Chad Versace]

On Tue 28 Mar 2017, Emil Velikov wrote:
> 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



> > +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.

I didn't have any issues. I added the --cpp-guard option because it
allowed me to re-use the same idiom in Android.genxml.mk used by the
other generated headers. That is, the --cpp-guard option allowed me to
re-use the Makefile's `header-gen` function instead of adding a custom
rule just for genX_bits.h.

> For the future rather please, poke me/others for ideas/help. Be that
> on #dri-devel, #intel-gfx or #android-ia.

I don't understand. I tested the build on android-ia, and it worked. And
the hunk in Android.genxml.mk follows the same idioms used elsewhere in
the file.