[Mesa-dev] [PATCH 1/2] intel: aubinator: generate a standalone binary

Kristian Høgsberg hoegsberg at gmail.com
Wed Oct 5 18:00:10 UTC 2016


On Tue, Oct 4, 2016 at 9:26 AM Ben Widawsky <ben at bwidawsk.net> wrote:

> On 16-10-04 15:38:52, Lionel Landwerlin wrote:
> >Embed the xml files into the binary, so aubinator can be used from any
> >location.
> >
> >Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> >Cc: Sirisha Gandikota <Sirisha.Gandikota at intel.com>
> >---
> > src/intel/Makefile.am           |  1 +
> > src/intel/Makefile.aubinator.am | 36 +++++++++++++++
> > src/intel/Makefile.sources      |  7 +++
> > src/intel/tools/.gitignore      |  5 +++
> > src/intel/tools/aubinator.c     | 97
> +++++++++++++++++------------------------
> > src/intel/tools/decoder.c       | 82 ++++++++++++++++++++--------------
> > src/intel/tools/decoder.h       |  4 +-
> > 7 files changed, 141 insertions(+), 91 deletions(-)
> > create mode 100644 src/intel/Makefile.aubinator.am
> >
> >diff --git a/src/intel/Makefile.am b/src/intel/Makefile.am
> >index 9186b5c..c3cb9fb 100644
> >--- a/src/intel/Makefile.am
> >+++ b/src/intel/Makefile.am
> >@@ -52,6 +52,7 @@ BUILT_SOURCES =
> > CLEANFILES =
> > EXTRA_DIST =
> >
> >+include Makefile.aubinator.am
> > include Makefile.blorp.am
> > include Makefile.common.am
> > include Makefile.genxml.am
> >diff --git a/src/intel/Makefile.aubinator.am b/src/intel/
> Makefile.aubinator.am
> >new file mode 100644
> >index 0000000..9772700
> >--- /dev/null
> >+++ b/src/intel/Makefile.aubinator.am
> >@@ -0,0 +1,36 @@
> >+# Copyright © 2016 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.
> >+
> >+BUILT_SOURCES += $(AUBINATOR_GENERATED_FILES)
> >+
> >+SUFFIXES = _aubinator_xml.h .xml
> >+
> >+tools/gen6_aubinator_xml.h: genxml/gen6.xml
> >+tools/gen7_aubinator_xml.h: genxml/gen7.xml
> >+tools/gen75_aubinator_xml.h: genxml/gen75.xml
> >+tools/gen8_aubinator_xml.h: genxml/gen8.xml
> >+tools/gen9_aubinator_xml.h: genxml/gen9.xml
> >+
> >+$(AUBINATOR_GENERATED_FILES): Makefile
> >+
> >+%_aubinator_xml.h:
> >+      $(MKDIR_GEN)
> >+      $(AM_V_GEN) xxd -i $< > $@
> >diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> >index 94073d2..a5c2bf0 100644
> >--- a/src/intel/Makefile.sources
> >+++ b/src/intel/Makefile.sources
> >@@ -1,3 +1,10 @@
> >+AUBINATOR_GENERATED_FILES = \
> >+      tools/gen6_aubinator_xml.h \
> >+      tools/gen7_aubinator_xml.h \
> >+      tools/gen75_aubinator_xml.h \
> >+      tools/gen8_aubinator_xml.h \
> >+      tools/gen9_aubinator_xml.h
> >+
> > BLORP_FILES = \
> >       blorp/blorp.c \
> >       blorp/blorp.h \
> >diff --git a/src/intel/tools/.gitignore b/src/intel/tools/.gitignore
> >index 0c80a6f..c4eebde 100644
> >--- a/src/intel/tools/.gitignore
> >+++ b/src/intel/tools/.gitignore
> >@@ -1 +1,6 @@
> > /aubinator
> >+gen6_aubinator_xml.h
> >+gen75_aubinator_xml.h
> >+gen7_aubinator_xml.h
> >+gen8_aubinator_xml.h
> >+gen9_aubinator_xml.h
> >diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
> >index a31dcb2..83328b5 100644
> >--- a/src/intel/tools/aubinator.c
> >+++ b/src/intel/tools/aubinator.c
> >@@ -35,6 +35,8 @@
> > #include <sys/wait.h>
> > #include <sys/mman.h>
> >
> >+#include "util/macros.h"
> >+
> > #include "decoder.h"
> > #include "intel_aub.h"
> > #include "gen_disasm.h"
> >@@ -1059,11 +1061,24 @@ int main(int argc, char *argv[])
> > {
> >    struct gen_spec *spec;
> >    struct aub_file *file;
> >-   int i, pci_id = 0;
> >+   int i;
> >    bool found_arg_gen = false, pager = true;
> >-   int gen_major, gen_minor;
> >-   const char *value;
> >-   char gen_file[256], gen_val[24];
> >+   const char *value, *input_file = NULL;
> >+   char gen_val[24];
> >+   const struct {
> >+      const char *name;
> >+      int pci_id;
> >+   } gens[] = {
> >+      { "ivb", 0x0166 }, /* Intel(R) Ivybridge Mobile GT2 */
> >+      { "hsw", 0x0416 }, /* Intel(R) Haswell Mobile GT2 */
> >+      { "byt", 0x0155 }, /* Intel(R) Bay Trail */
> >+      { "bdw", 0x1616 }, /* Intel(R) HD Graphics 5500 (Broadwell GT2) */
> >+      { "chv", 0x22B3 }, /* Intel(R) HD Graphics (Cherryview) */
> >+      { "skl", 0x1912 }, /* Intel(R) HD Graphics 530 (Skylake GT2) */
> >+      { "kbl", 0x591D }, /* Intel(R) Kabylake GT2 */
> >+      { "bxt", 0x0A84 }  /* Intel(R) HD Graphics (Broxton) */
> >+   }, *gen = NULL;
> >+   struct gen_device_info devinfo;
> >
> >    if (argc == 1) {
> >       print_help(argv[0], stderr);
> >@@ -1081,8 +1096,6 @@ int main(int argc, char *argv[])
> >             exit(EXIT_FAILURE);
> >          }
> >          found_arg_gen = true;
> >-         gen_major = 0;
> >-         gen_minor = 0;
> >          snprintf(gen_val, sizeof(gen_val), "%s", value);
> >       } else if (strcmp(argv[i], "--headers") == 0) {
> >          option_full_decode = false;
> >@@ -1105,6 +1118,7 @@ int main(int argc, char *argv[])
> >             fprintf(stderr, "unknown option %s\n", argv[i]);
> >             exit(EXIT_FAILURE);
> >          }
> >+         input_file = argv[i];
> >          break;
> >       }
> >    }
> >@@ -1114,52 +1128,26 @@ int main(int argc, char *argv[])
> >       exit(EXIT_FAILURE);
> >    }
> >
> >-   if (strstr(gen_val, "ivb") != NULL) {
> >-      /* Intel(R) Ivybridge Mobile GT2 */
> >-      pci_id = 0x0166;
> >-      gen_major = 7;
> >-      gen_minor = 0;
> >-   } else if (strstr(gen_val, "hsw") != NULL) {
> >-      /* Intel(R) Haswell Mobile GT2 */
> >-      pci_id = 0x0416;
> >-      gen_major = 7;
> >-      gen_minor = 5;
> >-   } else if (strstr(gen_val, "byt") != NULL) {
> >-      /* Intel(R) Bay Trail */
> >-      pci_id = 0x0155;
> >-      gen_major = 7;
> >-      gen_minor = 5;
> >-   } else if (strstr(gen_val, "bdw") != NULL) {
> >-      /* Intel(R) HD Graphics 5500 (Broadwell GT2) */
> >-      pci_id = 0x1616;
> >-      gen_major = 8;
> >-      gen_minor = 0;
> >-   }  else if (strstr(gen_val, "chv") != NULL) {
> >-      /* Intel(R) HD Graphics (Cherryview) */
> >-      pci_id = 0x22B3;
> >-      gen_major = 8;
> >-      gen_minor = 0;
> >-   } else if (strstr(gen_val, "skl") != NULL) {
> >-      /* Intel(R) HD Graphics 530 (Skylake GT2) */
> >-      pci_id = 0x1912;
> >-      gen_major = 9;
> >-      gen_minor = 0;
> >-   } else if (strstr(gen_val, "kbl") != NULL) {
> >-      /* Intel(R) Kabylake GT2 */
> >-      pci_id = 0x591D;
> >-      gen_major = 9;
> >-      gen_minor = 0;
> >-   } else if (strstr(gen_val, "bxt") != NULL) {
> >-      /* Intel(R) HD Graphics (Broxton) */
> >-      pci_id = 0x0A84;
> >-      gen_major = 9;
> >-      gen_minor = 0;
> >-   } else {
> >+   for (i = 0; i < ARRAY_SIZE(gens); i++) {
> >+      if (!strcmp(gen_val, gens[i].name)) {
> >+         gen = &gens[i];
> >+         break;
> >+      }
> >+   }
> >+
> >+   if (gen == NULL) {
>
> Since we're redoing this, I'm wondering if it makes sense to use "name" at
> all.
> Maybe just the actual gen major/minor would be better? Just a thought.
>
> >       fprintf(stderr, "can't parse gen: %s, expected ivb, byt, hsw, "
> >                              "bdw, chv, skl, kbl or bxt\n", gen_val);
> >       exit(EXIT_FAILURE);
> >    }
> >
> >+   if (!gen_get_device_info(gen->pci_id, &devinfo)) {
> >+      fprintf(stderr, "can't find device information: pci_id=0x%x
> name=%s\n",
> >+              gen->pci_id, gen->name);
> >+      exit(EXIT_FAILURE);
> >+   }
> >+
> >+
> >    /* Do this before we redirect stdout to pager. */
> >    if (option_color == COLOR_AUTO)
> >       option_color = isatty(1) ? COLOR_ALWAYS : COLOR_NEVER;
> >@@ -1167,21 +1155,14 @@ int main(int argc, char *argv[])
> >    if (isatty(1) && pager)
> >       setup_pager();
> >
> >-   if (gen_minor > 0) {
> >-      snprintf(gen_file, sizeof(gen_file), "../genxml/gen%d%d.xml",
> >-               gen_major, gen_minor);
> >-   } else {
> >-      snprintf(gen_file, sizeof(gen_file), "../genxml/gen%d.xml",
> gen_major);
> >-   }
> >-
> >-   spec = gen_spec_load(gen_file);
> >-   disasm = gen_disasm_create(pci_id);
> >+   spec = gen_spec_load(&devinfo);
> >+   disasm = gen_disasm_create(gen->pci_id);
> >
> >-   if (argv[i] == NULL) {
> >+   if (input_file == NULL) {
> >        print_help(argv[0], stderr);
> >        exit(EXIT_FAILURE);
> >    } else {
> >-       file = aub_file_open(argv[i]);
> >+       file = aub_file_open(input_file);
> >    }
> >
> >    while (aub_file_more_stuff(file))
> >diff --git a/src/intel/tools/decoder.c b/src/intel/tools/decoder.c
> >index be3558b..f3edc71 100644
> >--- a/src/intel/tools/decoder.c
> >+++ b/src/intel/tools/decoder.c
> >@@ -32,6 +32,12 @@
> >
> > #include "decoder.h"
> >
> >+#include "gen6_aubinator_xml.h"
> >+#include "gen7_aubinator_xml.h"
> >+#include "gen75_aubinator_xml.h"
> >+#include "gen8_aubinator_xml.h"
> >+#include "gen9_aubinator_xml.h"
> >+
> > #define XML_BUFFER_SIZE 4096
> >
> > #define MAKE_GEN(major, minor) ( ((major) << 8) | (minor) )
> >@@ -394,57 +400,69 @@ character_data(void *data, const XML_Char *s, int
> len)
> > {
> > }
> >
> >+static void *
> >+devinfo_to_xml_data(const struct gen_device_info *devinfo,
> >+                    uint32_t *data_length)
> >+{
> >+   switch (devinfo->gen) {
> >+   case 6:
> >+      *data_length = sizeof(genxml_gen6_xml);
> >+      return genxml_gen6_xml;
> >+   case 7:
> >+      if (devinfo->is_haswell) {
> >+         *data_length = sizeof(genxml_gen75_xml);
> >+         return genxml_gen75_xml;
> >+      }
> >+      *data_length = sizeof(genxml_gen7_xml);
> >+      return genxml_gen7_xml;
> >+   case 8:
> >+      *data_length = sizeof(genxml_gen8_xml);
> >+      return genxml_gen8_xml;
> >+   case 9:
> >+      *data_length = sizeof(genxml_gen9_xml);
> >+      return genxml_gen9_xml;
> >+   default:
> >+      return NULL;
> >+   }
> >+}
> >+
> > struct gen_spec *
> >-gen_spec_load(const char *filename)
> >+gen_spec_load(const struct gen_device_info *devinfo)
> > {
> >    struct parser_context ctx;
> >-   void *buf;
> >-   int len;
> >-   FILE *input;
> >-
> >-   input = fopen(filename, "r");
> >-   printf("xml filename = %s\n", filename);
> >-   if (input == NULL) {
> >-      fprintf(stderr, "failed to open xml description\n");
> >-      exit(EXIT_FAILURE);
> >-   }
> >+   void *buf, *data;
> >+   uint32_t data_length = 0;
> >
> >    memset(&ctx, 0, sizeof ctx);
> >    ctx.parser = XML_ParserCreate(NULL);
> >    XML_SetUserData(ctx.parser, &ctx);
> >    if (ctx.parser == NULL) {
> >       fprintf(stderr, "failed to create parser\n");
> >-      fclose(input);
> >       return NULL;
> >    }
> >
> >    XML_SetElementHandler(ctx.parser, start_element, end_element);
> >    XML_SetCharacterDataHandler(ctx.parser, character_data);
> >-   ctx.loc.filename = filename;
> >
> >    ctx.spec = xzalloc(sizeof(*ctx.spec));
> >
> >-   do {
> >-      buf = XML_GetBuffer(ctx.parser, XML_BUFFER_SIZE);
> >-      len = fread(buf, 1, XML_BUFFER_SIZE, input);
> >-      if (len < 0) {
> >-         fprintf(stderr, "fread: %m\n");
> >-         fclose(input);
> >-         return NULL;
> >-      }
> >-      if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) {
> >-         fprintf(stderr,
> >-                 "Error parsing XML at line %ld col %ld: %s\n",
> >-                 XML_GetCurrentLineNumber(ctx.parser),
> >-                 XML_GetCurrentColumnNumber(ctx.parser),
> >-                 XML_ErrorString(XML_GetErrorCode(ctx.parser)));
> >-         fclose(input);
> >-         return NULL;
> >-      }
> >-   } while (len > 0);
> >+
> >+   data = devinfo_to_xml_data(devinfo, &data_length);
> >+   buf = XML_GetBuffer(ctx.parser, data_length);
> >+
> >+   memcpy(buf, data, data_length);
> >+
> >+   if (XML_ParseBuffer(ctx.parser, data_length, data_length == 0) == 0) {
> >+      fprintf(stderr,
> >+              "Error parsing XML at line %ld col %ld: %s\n",
> >+              XML_GetCurrentLineNumber(ctx.parser),
> >+              XML_GetCurrentColumnNumber(ctx.parser),
> >+              XML_ErrorString(XML_GetErrorCode(ctx.parser)));
> >+      XML_ParserFree(ctx.parser);
> >+      return NULL;
> >+   }
> >
> >    XML_ParserFree(ctx.parser);
> >-   fclose(input);
> >
> >    return ctx.spec;
> > }
> >diff --git a/src/intel/tools/decoder.h b/src/intel/tools/decoder.h
> >index f688ba5..5bea9b9 100644
> >--- a/src/intel/tools/decoder.h
> >+++ b/src/intel/tools/decoder.h
> >@@ -26,6 +26,8 @@
> > #include <stdint.h>
> > #include <stdbool.h>
> >
> >+#include "common/gen_device_info.h"
> >+
> > struct gen_spec;
> > struct gen_group;
> > struct gen_field;
> >@@ -36,7 +38,7 @@ static inline uint32_t gen_make_gen(uint32_t major,
> uint32_t minor)
> > }
> >
> > struct gen_group *gen_spec_find_struct(struct gen_spec *spec, const char
> *name);
> >-struct gen_spec *gen_spec_load(const char *filename);
> >+struct gen_spec *gen_spec_load(const struct gen_device_info *devinfo);
> > uint32_t gen_spec_get_gen(struct gen_spec *spec);
> > struct gen_group *gen_spec_find_instruction(struct gen_spec *spec, const
> uint32_t *p);
> > struct gen_group *gen_spec_find_register(struct gen_spec *spec, uint32_t
> offset);
>
> LGTM. Might want to run it by Kristian in terms of moving away from the XML
> parsing. It's hard to tell if he did it because it seemed fun, or because
> it was
> for some use (honestly, being able to change XML without rebuilding does
> seem
> kind of nice to me - but I don't care much either way).
>

Embedding the XML works, but if we're moving away from parsing the XML from
disk, a better option may be to write another python parser to generate a
static const C data structure.

Kristian


> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161005/88c2bc86/attachment-0001.html>


More information about the mesa-dev mailing list