subject:"\[Mesa\-dev\] \[PATCH 2\/8\] genxml\: Generate header genX

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

2017-03-22 Thread Chad Versace

On Wed 22 Mar 2017, Jason Ekstrand wrote:
> On Wed, Mar 22, 2017 at 2:02 PM, Chad Versace 
> wrote:
> 
> > On Wed 22 Mar 2017, Jason Ekstrand wrote:
> > > Wow... This is an impressive quantity of infastructure just to get the
> > > number of bits in a field...
> >
> > In the initial patch, I hand-wrote macros and functions. And I made
> > mistakes. That's when I decided to eliminate the mistakes with
> > generation. That's why we have the genxml, after all.
> >
> > > On Tue, Mar 21, 2017 at 4:02 PM, Chad Versace 
> > > wrote:
> >
> >
> > > > +REGEX = re.compile('^#define (?PGEN(?P[0-
> > > > 9]+)_(?P\w*Surface(Q?)Pitch)_bits).*$')
> > > >
> > >
> > > Can we please not use XML to generate a header and then use regex to
> > parse
> > > the header?  Let's just use XML for both.
> > >
> > > I hate to ask for a rewrite, but you should have seen that coming...
> >
> > Argh... yes, I knew it was coming. Yes, I knew I shouldn't have. But it
> > was S easy.
> >
> > Now that I've had time to ponder my sin, I no longer want to maintain
> > a giant pile of Python "just to get the number of bits in a field". Let
> > me propose an alternative: Keep the Python changes to gen_pack_header.py
> > that emits the GEN_*Pitch_bits macros, but hand-write the genX_bits.h
> > headers. I think that's good compromise. What do you think?
> >
> 
> Seems reasonable.  You can even do a #define like so:
> 
> #define GENX_BITS_DECLARE_LOOKUP(field) \
>static inline uint32_t \
>field##_bits(const struct gen_device_info *devinfo) \
>{ \
>   switch (devinfo->gen) {
>   case 4: return devinfo->is_g4x ? GEN4_##field##_bits :
> GEN45_##field##_bits;
>   case 5: return GEN5_##field##_bits;
>   case 6: return GEN6_##field##_bits;
>   case 7: return devinfo->is_haswell ? GEN75_##field##_bits :
> GEN7_##field##_bits;
>   case 8: return GEN8_##field##_bits;
>   case 9: return GEN9_##field##_bits;
>   }
>}
> 
> GENX_BITS_DECLARE_LOOKUP(RENDER_SURFACE_STATE_SurfacePitch)

For the sake of ctags and grep, I'm going to avoid the macro.

> Unfortunately, this won't be able to deal with field renames but I think
> the right way to deal with that is to just fix the XML.  Also, I didn't
> think you actually could include all of the genX_pack headers at the same
> time due to name conflicts.  Is this not true?

Right. The gen*_pack.h headers conflict with each other.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

2017-03-22 Thread Jason Ekstrand

On Wed, Mar 22, 2017 at 2:02 PM, Chad Versace 
wrote:

> On Wed 22 Mar 2017, Jason Ekstrand wrote:
> > Wow... This is an impressive quantity of infastructure just to get the
> > number of bits in a field...
>
> In the initial patch, I hand-wrote macros and functions. And I made
> mistakes. That's when I decided to eliminate the mistakes with
> generation. That's why we have the genxml, after all.
>
> > On Tue, Mar 21, 2017 at 4:02 PM, Chad Versace 
> > wrote:
>
>
> > > +REGEX = re.compile('^#define (?PGEN(?P[0-
> > > 9]+)_(?P\w*Surface(Q?)Pitch)_bits).*$')
> > >
> >
> > Can we please not use XML to generate a header and then use regex to
> parse
> > the header?  Let's just use XML for both.
> >
> > I hate to ask for a rewrite, but you should have seen that coming...
>
> Argh... yes, I knew it was coming. Yes, I knew I shouldn't have. But it
> was S easy.
>
> Now that I've had time to ponder my sin, I no longer want to maintain
> a giant pile of Python "just to get the number of bits in a field". Let
> me propose an alternative: Keep the Python changes to gen_pack_header.py
> that emits the GEN_*Pitch_bits macros, but hand-write the genX_bits.h
> headers. I think that's good compromise. What do you think?
>

Seems reasonable.  You can even do a #define like so:

#define GENX_BITS_DECLARE_LOOKUP(field) \
   static inline uint32_t \
   field##_bits(const struct gen_device_info *devinfo) \
   { \
  switch (devinfo->gen) {
  case 4: return devinfo->is_g4x ? GEN4_##field##_bits :
GEN45_##field##_bits;
  case 5: return GEN5_##field##_bits;
  case 6: return GEN6_##field##_bits;
  case 7: return devinfo->is_haswell ? GEN75_##field##_bits :
GEN7_##field##_bits;
  case 8: return GEN8_##field##_bits;
  case 9: return GEN9_##field##_bits;
  }
   }

GENX_BITS_DECLARE_LOOKUP(RENDER_SURFACE_STATE_SurfacePitch)

Unfortunately, this won't be able to deal with field renames but I think
the right way to deal with that is to just fix the XML.  Also, I didn't
think you actually could include all of the genX_pack headers at the same
time due to name conflicts.  Is this not true?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

2017-03-22 Thread Chad Versace

On Wed 22 Mar 2017, Emil Velikov wrote:
> Hi Chad,
> 
> Please keep Tapani in Cc for the next revision - I think he can manage
> the Android bits required.

Will do.

Also, I'm currently doing an initial build of android-ia, so I can stay
on top of the Android changes along with Tapani.

> There's a few small requests below, but don't bother if they're too
> time consuming.
> 
> On 21 March 2017 at 23:02, Chad Versace  wrote:
> 
> > +genxml/genX_bits.h: genxml/gen_bits_header.py
> > +genxml/genX_bits.h: $(GENXML_PACK_GENERATED_FILES)
> > +   $(MKDIR_GEN)
> > +   $(AM_V_GEN) $(PYTHON2) $(srcdir)/genxml/gen_bits_header.py \
> > +   $(GENXML_PACK_GENERATED_FILES) > $@ || ($(RM) $@; false)

> Please provide the file[name] directly to the script rather than
> piping stdout to file.

Sure. I can add an '-o' option. That is, if I don't discard the script
in v4 of the series.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

2017-03-22 Thread Chad Versace

On Wed 22 Mar 2017, Jason Ekstrand wrote:
> Wow... This is an impressive quantity of infastructure just to get the
> number of bits in a field...

In the initial patch, I hand-wrote macros and functions. And I made
mistakes. That's when I decided to eliminate the mistakes with
generation. That's why we have the genxml, after all.

> On Tue, Mar 21, 2017 at 4:02 PM, Chad Versace 
> wrote:

> > +REGEX = re.compile('^#define (?PGEN(?P[0-
> > 9]+)_(?P\w*Surface(Q?)Pitch)_bits).*$')
> >
> 
> Can we please not use XML to generate a header and then use regex to parse
> the header?  Let's just use XML for both.
> 
> I hate to ask for a rewrite, but you should have seen that coming...

Argh... yes, I knew it was coming. Yes, I knew I shouldn't have. But it
was S easy.

Now that I've had time to ponder my sin, I no longer want to maintain
a giant pile of Python "just to get the number of bits in a field". Let
me propose an alternative: Keep the Python changes to gen_pack_header.py
that emits the GEN_*Pitch_bits macros, but hand-write the genX_bits.h
headers. I think that's good compromise. What do you think?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

2017-03-22 Thread Emil Velikov

Hi Chad,

Please keep Tapani in Cc for the next revision - I think he can manage
the Android bits required.

There's a few small requests below, but don't bother if they're too
time consuming.

On 21 March 2017 at 23:02, Chad Versace  wrote:

> +genxml/genX_bits.h: genxml/gen_bits_header.py
> +genxml/genX_bits.h: $(GENXML_PACK_GENERATED_FILES)
> +   $(MKDIR_GEN)
> +   $(AM_V_GEN) $(PYTHON2) $(srcdir)/genxml/gen_bits_header.py \
> +   $(GENXML_PACK_GENERATED_FILES) > $@ || ($(RM) $@; false)
Please provide the file[name] directly to the script rather than
piping stdout to file.

> +/* THIS FILE HAS BEEN GENERATED, DO NOT HAND EDIT.
> + *
> + * Sizes of bitfields in genxml instructions, structures, and registers.
> + */
> +
> +#ifndef GENX_BITS_H
> +#define GENX_BITS_H
> +

> +
> +#endif /* GENX_BITS_H */
Here one can use the filename.

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

2017-03-22 Thread Jason Ekstrand

Wow... This is an impressive quantity of infastructure just to get the
number of bits in a field...

On Tue, Mar 21, 2017 at 4:02 PM, Chad Versace 
wrote:

> genX_bits.h contains the sizes of bitfields in genxml instructions,
> structures, and registers. It also defines some functions to query those
> sizes.
>
> Currently, the bitfields in genX_bits.h are those whose name matches
> /.*Surface(Q?)Pitch/.
>
> 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 macro in gen*_pack.h that whose name matches
> /.*Surface(Q?)Pitch_bits$/ is also in genX_bits.h.
>
>   - For each set of macros whose name, after stripping the GEN prefix,
> is the same, genX_bits.h contains a query function. See the examples
> below.
>
> The generated file is not committed, so here are some excerpts:
>
> Example: The GEN7 and GEN6 sections:
>
> #define GEN7_RENDER_SURFACE_STATE_SurfacePitch_bits  18
> #define GEN7_RENDER_SURFACE_STATE_MCSSurfacePitch_bits9
> #define GEN7_3DSTATE_DEPTH_BUFFER_SurfacePitch_bits  18
> #define GEN7_3DSTATE_HIER_DEPTH_BUFFER_SurfacePitch_bits 17
> #define GEN7_3DSTATE_STENCIL_BUFFER_SurfacePitch_bits17
>
> #define GEN6_RENDER_SURFACE_STATE_SurfacePitch_bits  17
> #define GEN6_3DSTATE_DEPTH_BUFFER_SurfacePitch_bits  17
> #define GEN6_3DSTATE_HIER_DEPTH_BUFFER_SurfacePitch_bits 17
> #define GEN6_3DSTATE_STENCIL_BUFFER_SurfacePitch_bits17
>
> Example: The SurfacePitch and AuxiliarySurfacePitch functions for
> RENDER_SURFACE_STATE:
>
>static inline uint32_t __attribute__((const))
>RENDER_SURFACE_STATE_SurfacePitch_bits(int gen_10x)
>{
>   switch (gen_10x) {
>   case 90: return GEN9_RENDER_SURFACE_STATE_SurfacePitch_bits;
>   case 80: return GEN8_RENDER_SURFACE_STATE_SurfacePitch_bits;
>   case 75: return GEN75_RENDER_SURFACE_STATE_SurfacePitch_bits;
>   case 70: return GEN7_RENDER_SURFACE_STATE_SurfacePitch_bits;
>   case 60: return GEN6_RENDER_SURFACE_STATE_SurfacePitch_bits;
>   case 50: return GEN5_RENDER_SURFACE_STATE_SurfacePitch_bits;
>   case 45: return GEN45_RENDER_SURFACE_STATE_SurfacePitch_bits;
>   case 40: return GEN4_RENDER_SURFACE_STATE_SurfacePitch_bits;
>   default: return 0;
>   }
>}
>
>static inline uint32_t __attribute__((const))
>RENDER_SURFACE_STATE_AuxiliarySurfacePitch_bits(int gen_10x)
>{
>   switch (gen_10x) {
>   case 90: return GEN9_RENDER_SURFACE_STATE_
> AuxiliarySurfacePitch_bits;
>   case 80: return GEN8_RENDER_SURFACE_STATE_
> AuxiliarySurfacePitch_bits;
>   case 75: return GEN75_RENDER_SURFACE_STATE_MCSSurfacePitch_bits;
>   case 70: return GEN7_RENDER_SURFACE_STATE_MCSSurfacePitch_bits;
>   default: return 0;
>   }
>}
> ---
>  src/intel/Makefile.genxml.am|   9 +-
>  src/intel/Makefile.sources  |   6 +-
>  src/intel/genxml/.gitignore |   1 +
>  src/intel/genxml/gen_bits_header.py | 164 ++
> ++
>  4 files changed, 178 insertions(+), 2 deletions(-)
>  create mode 100644 src/intel/genxml/gen_bits_header.py
>
> diff --git a/src/intel/Makefile.genxml.am b/src/intel/Makefile.genxml.am
> index bea0aab817f..586e551e79b 100644
> --- a/src/intel/Makefile.genxml.am
> +++ b/src/intel/Makefile.genxml.am
> @@ -29,7 +29,7 @@ EXTRA_DIST += \
>
>  SUFFIXES = _pack.h _xml.h .xml
>
> -$(GENXML_GENERATED_FILES): genxml/gen_pack_header.py
> +$(GENXML_PACK_GENERATED_FILES): genxml/gen_pack_header.py
>
>  .xml_pack.h:
> $(MKDIR_GEN)
> @@ -41,6 +41,12 @@ $(GENXML_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/genX_bits.h: $(GENXML_PACK_GENERATED_FILES)
> +   $(MKDIR_GEN)
> +   $(AM_V_GEN) $(PYTHON2) $(srcdir)/genxml/gen_bits_header.py \
> +   $(GENXML_PACK_GENERATED_FILES) > $@ || ($(RM) $@; false)
> +
>  EXTRA_DIST += \
> genxml/gen4.xml \
> genxml/gen45.xml \
> @@ -50,6 +56,7 @@ EXTRA_DIST += \
> genxml/gen75.xml \
> genxml/gen8.xml \
> genxml/gen9.xml \
> +   genxml/genX_bits.h \
> genxml/genX_pack.h \
> genxml/gen_macros.h \
> genxml/gen_pack_header.py \
> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> index 839ea47d752..af85d448eb8 100644
> --- a/src/intel/Makefile.sources
> +++ b/src/intel/Makefile.sources
> @@ -107,7 +107,7 @@ COMPILER_FILES = \
>  COMPILER_GENERATED_FILES = \
> compiler/brw_nir_trig_workarounds.c
>
> -GENXML_GENERATED_FILES = \
> +GENXML_PACK_GENERATED_FILES = \
> genxml/gen4_pack.h \
> genxml/gen45_pack.h \
>

[Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

2017-03-21 Thread Chad Versace

genX_bits.h contains the sizes of bitfields in genxml instructions,
structures, and registers. It also defines some functions to query those
sizes.

Currently, the bitfields in genX_bits.h are those whose name matches
/.*Surface(Q?)Pitch/.

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 macro in gen*_pack.h that whose name matches
/.*Surface(Q?)Pitch_bits$/ is also in genX_bits.h.

  - For each set of macros whose name, after stripping the GEN prefix,
is the same, genX_bits.h contains a query function. See the examples
below.

The generated file is not committed, so here are some excerpts:

Example: The GEN7 and GEN6 sections:

#define GEN7_RENDER_SURFACE_STATE_SurfacePitch_bits  18
#define GEN7_RENDER_SURFACE_STATE_MCSSurfacePitch_bits9
#define GEN7_3DSTATE_DEPTH_BUFFER_SurfacePitch_bits  18
#define GEN7_3DSTATE_HIER_DEPTH_BUFFER_SurfacePitch_bits 17
#define GEN7_3DSTATE_STENCIL_BUFFER_SurfacePitch_bits17

#define GEN6_RENDER_SURFACE_STATE_SurfacePitch_bits  17
#define GEN6_3DSTATE_DEPTH_BUFFER_SurfacePitch_bits  17
#define GEN6_3DSTATE_HIER_DEPTH_BUFFER_SurfacePitch_bits 17
#define GEN6_3DSTATE_STENCIL_BUFFER_SurfacePitch_bits17

Example: The SurfacePitch and AuxiliarySurfacePitch functions for
RENDER_SURFACE_STATE:

   static inline uint32_t __attribute__((const))
   RENDER_SURFACE_STATE_SurfacePitch_bits(int gen_10x)
   {
  switch (gen_10x) {
  case 90: return GEN9_RENDER_SURFACE_STATE_SurfacePitch_bits;
  case 80: return GEN8_RENDER_SURFACE_STATE_SurfacePitch_bits;
  case 75: return GEN75_RENDER_SURFACE_STATE_SurfacePitch_bits;
  case 70: return GEN7_RENDER_SURFACE_STATE_SurfacePitch_bits;
  case 60: return GEN6_RENDER_SURFACE_STATE_SurfacePitch_bits;
  case 50: return GEN5_RENDER_SURFACE_STATE_SurfacePitch_bits;
  case 45: return GEN45_RENDER_SURFACE_STATE_SurfacePitch_bits;
  case 40: return GEN4_RENDER_SURFACE_STATE_SurfacePitch_bits;
  default: return 0;
  }
   }

   static inline uint32_t __attribute__((const))
   RENDER_SURFACE_STATE_AuxiliarySurfacePitch_bits(int gen_10x)
   {
  switch (gen_10x) {
  case 90: return GEN9_RENDER_SURFACE_STATE_AuxiliarySurfacePitch_bits;
  case 80: return GEN8_RENDER_SURFACE_STATE_AuxiliarySurfacePitch_bits;
  case 75: return GEN75_RENDER_SURFACE_STATE_MCSSurfacePitch_bits;
  case 70: return GEN7_RENDER_SURFACE_STATE_MCSSurfacePitch_bits;
  default: return 0;
  }
   }
---
 src/intel/Makefile.genxml.am|   9 +-
 src/intel/Makefile.sources  |   6 +-
 src/intel/genxml/.gitignore |   1 +
 src/intel/genxml/gen_bits_header.py | 164 
 4 files changed, 178 insertions(+), 2 deletions(-)
 create mode 100644 src/intel/genxml/gen_bits_header.py

diff --git a/src/intel/Makefile.genxml.am b/src/intel/Makefile.genxml.am
index bea0aab817f..586e551e79b 100644
--- a/src/intel/Makefile.genxml.am
+++ b/src/intel/Makefile.genxml.am
@@ -29,7 +29,7 @@ EXTRA_DIST += \
 
 SUFFIXES = _pack.h _xml.h .xml
 
-$(GENXML_GENERATED_FILES): genxml/gen_pack_header.py
+$(GENXML_PACK_GENERATED_FILES): genxml/gen_pack_header.py
 
 .xml_pack.h:
$(MKDIR_GEN)
@@ -41,6 +41,12 @@ $(GENXML_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/genX_bits.h: $(GENXML_PACK_GENERATED_FILES)
+   $(MKDIR_GEN)
+   $(AM_V_GEN) $(PYTHON2) $(srcdir)/genxml/gen_bits_header.py \
+   $(GENXML_PACK_GENERATED_FILES) > $@ || ($(RM) $@; false)
+
 EXTRA_DIST += \
genxml/gen4.xml \
genxml/gen45.xml \
@@ -50,6 +56,7 @@ EXTRA_DIST += \
genxml/gen75.xml \
genxml/gen8.xml \
genxml/gen9.xml \
+   genxml/genX_bits.h \
genxml/genX_pack.h \
genxml/gen_macros.h \
genxml/gen_pack_header.py \
diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
index 839ea47d752..af85d448eb8 100644
--- a/src/intel/Makefile.sources
+++ b/src/intel/Makefile.sources
@@ -107,7 +107,7 @@ COMPILER_FILES = \
 COMPILER_GENERATED_FILES = \
compiler/brw_nir_trig_workarounds.c
 
-GENXML_GENERATED_FILES = \
+GENXML_PACK_GENERATED_FILES = \
genxml/gen4_pack.h \
genxml/gen45_pack.h \
genxml/gen5_pack.h \
@@ -117,6 +117,10 @@ GENXML_GENERATED_FILES = \
genxml/gen8_pack.h \
genxml/gen9_pack.h
 
+GENXML_GENERATED_FILES = \
+   $(GENXML_PACK_GENERATED_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

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

Re: [Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

[Mesa-dev] [PATCH 2/8] genxml: Generate header genX_bits.h

7 matches

Site Navigation

Mail list logo

Footer information