Re: [Mesa-dev] [PATCH] intel/tools: new i965_disasm tool
Thanks for reviewing the patch. I will make changes and send v2 accordingly.
On 08/20/2018 11:34 AM, Matt Turner wrote:
> On Thu, Aug 16, 2018 at 1:51 PM Sagar Ghuge wrote:
>>
>> Adds a new i965 instruction disassemble tool
>
> This looks very good. A few comments about the structure inline.
>
>> Signed-off-by: Sagar Ghuge
>> ---
>> src/intel/Makefile.tools.am | 15 +++
>> src/intel/tools/i965_disasm.c | 202 ++
>> src/intel/tools/i965_disasm.h | 46
>> src/intel/tools/meson.build | 11 ++
>> 4 files changed, 274 insertions(+)
>> create mode 100644 src/intel/tools/i965_disasm.c
>> create mode 100644 src/intel/tools/i965_disasm.h
>>
>> diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am
>> index 00624084e6..36a3a70a28 100644
>> --- a/src/intel/Makefile.tools.am
>> +++ b/src/intel/Makefile.tools.am
>> @@ -22,6 +22,7 @@
>> noinst_PROGRAMS += \
>> tools/aubinator \
>> tools/aubinator_error_decode \
>> + tools/i965_disasm \
>> tools/error2aub
>>
>>
>> @@ -62,6 +63,20 @@ tools_aubinator_error_decode_CFLAGS = \
>> $(AM_CFLAGS) \
>> $(ZLIB_CFLAGS)
>>
>> +tools_i965_disasm_SOURCES = \
>> + tools/i965_disasm.c \
>> + tools/i965_disasm.h
>> +
>> +tools_i965_disasm_LDADD = \
>> + common/libintel_common.la \
>> + compiler/libintel_compiler.la \
>> + dev/libintel_dev.la \
>> + $(top_builddir)/src/util/libmesautil.la \
>> + $(PTHREAD_LIBS)
>> +
>> +tools_i965_disasm_CFLAGS = \
>> + $(AM_CFLAGS)
>> +
>
> Looks good.
>
>> tools_error2aub_SOURCES = \
>> tools/gen_context.h \
>> diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c
>> new file mode 100644
>> index 00..c880559827
>> --- /dev/null
>> +++ b/src/intel/tools/i965_disasm.c
>> @@ -0,0 +1,202 @@
>> +/*
>> + * Copyright © 2018 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.
>> + */
>> +
>> +#include
>> +#include
>> +
>> +#include "compiler/brw_inst.h"
>> +#include "compiler/brw_eu.h"
>> +
>> +#include "i965_disasm.h"
>> +
>> +uint64_t INTEL_DEBUG;
>> +uint16_t pci_id = 0;
>> +FILE *outfile;
>> +
>> +struct i965_disasm {
>> +struct gen_device_info devinfo;
>> +};
>> +
>> +/* Return size of file in bytes pointed by fp */
>> +static size_t
>> +i965_disasm_get_file_size(FILE *fp)
>> +{
>> + size_t size = 0;
>
> No need for initialization.
>
>> +
>> + fseek(fp, 0L, SEEK_END);
>> + size = ftell(fp);
>> + fseek(fp, 0L, SEEK_SET);
>> +
>> + return size;
>> +}
>> +
>> +/* Return number of bytes read */
>> +static size_t
>> +i965_disasm_read_binary(FILE *fp, void **assembly)
>> +{
>> + size_t end = i965_disasm_get_file_size(fp);
>> + *assembly = malloc(end + 1);
>> + fread(*assembly, end, 1, fp);
>> + fclose(fp);
>> +
>> + return end;
>> +}
>> +
>> +static void
>> +print_help(const char *progname, FILE *file)
>> +{
>> + fprintf(file,
>> + "Usage: %s [OPTION]...\n"
>> + "Disassemble i965 instructions from binary file.\n\n"
>> + " --help display this help and exit\n"
>> + " --binary-path=PATH read binary file from binary file
>> PATH\n"
>> + " --gen=platform disassemble instructions for given \n"
>> + " platform (3 letter platform name)\n",
>> + progname);
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + FILE *fp = NULL;
>> + void *assembly = NULL;
>> + char *binary_path = NULL;
>> + size_t start = 0, end = 0;
>> + int c, i;
>> + struct i965_disasm *disasm;
>> +
>> + bool help = false;
>> + const struct option i965_disasm_opts[] = {
>> + { "help", no_argument, (int *) , true },
>> + { "binary-path",
Re: [Mesa-dev] [PATCH] intel/tools: new i965_disasm tool
On Thu, Aug 16, 2018 at 1:51 PM Sagar Ghuge wrote:
>
> Adds a new i965 instruction disassemble tool
This looks very good. A few comments about the structure inline.
> Signed-off-by: Sagar Ghuge
> ---
> src/intel/Makefile.tools.am | 15 +++
> src/intel/tools/i965_disasm.c | 202 ++
> src/intel/tools/i965_disasm.h | 46
> src/intel/tools/meson.build | 11 ++
> 4 files changed, 274 insertions(+)
> create mode 100644 src/intel/tools/i965_disasm.c
> create mode 100644 src/intel/tools/i965_disasm.h
>
> diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am
> index 00624084e6..36a3a70a28 100644
> --- a/src/intel/Makefile.tools.am
> +++ b/src/intel/Makefile.tools.am
> @@ -22,6 +22,7 @@
> noinst_PROGRAMS += \
> tools/aubinator \
> tools/aubinator_error_decode \
> + tools/i965_disasm \
> tools/error2aub
>
>
> @@ -62,6 +63,20 @@ tools_aubinator_error_decode_CFLAGS = \
> $(AM_CFLAGS) \
> $(ZLIB_CFLAGS)
>
> +tools_i965_disasm_SOURCES = \
> + tools/i965_disasm.c \
> + tools/i965_disasm.h
> +
> +tools_i965_disasm_LDADD = \
> + common/libintel_common.la \
> + compiler/libintel_compiler.la \
> + dev/libintel_dev.la \
> + $(top_builddir)/src/util/libmesautil.la \
> + $(PTHREAD_LIBS)
> +
> +tools_i965_disasm_CFLAGS = \
> + $(AM_CFLAGS)
> +
Looks good.
> tools_error2aub_SOURCES = \
> tools/gen_context.h \
> diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c
> new file mode 100644
> index 00..c880559827
> --- /dev/null
> +++ b/src/intel/tools/i965_disasm.c
> @@ -0,0 +1,202 @@
> +/*
> + * Copyright © 2018 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.
> + */
> +
> +#include
> +#include
> +
> +#include "compiler/brw_inst.h"
> +#include "compiler/brw_eu.h"
> +
> +#include "i965_disasm.h"
> +
> +uint64_t INTEL_DEBUG;
> +uint16_t pci_id = 0;
> +FILE *outfile;
> +
> +struct i965_disasm {
> +struct gen_device_info devinfo;
> +};
> +
> +/* Return size of file in bytes pointed by fp */
> +static size_t
> +i965_disasm_get_file_size(FILE *fp)
> +{
> + size_t size = 0;
No need for initialization.
> +
> + fseek(fp, 0L, SEEK_END);
> + size = ftell(fp);
> + fseek(fp, 0L, SEEK_SET);
> +
> + return size;
> +}
> +
> +/* Return number of bytes read */
> +static size_t
> +i965_disasm_read_binary(FILE *fp, void **assembly)
> +{
> + size_t end = i965_disasm_get_file_size(fp);
> + *assembly = malloc(end + 1);
> + fread(*assembly, end, 1, fp);
> + fclose(fp);
> +
> + return end;
> +}
> +
> +static void
> +print_help(const char *progname, FILE *file)
> +{
> + fprintf(file,
> + "Usage: %s [OPTION]...\n"
> + "Disassemble i965 instructions from binary file.\n\n"
> + " --help display this help and exit\n"
> + " --binary-path=PATH read binary file from binary file
> PATH\n"
> + " --gen=platform disassemble instructions for given \n"
> + " platform (3 letter platform name)\n",
> + progname);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + FILE *fp = NULL;
> + void *assembly = NULL;
> + char *binary_path = NULL;
> + size_t start = 0, end = 0;
> + int c, i;
> + struct i965_disasm *disasm;
> +
> + bool help = false;
> + const struct option i965_disasm_opts[] = {
> + { "help", no_argument, (int *) , true },
> + { "binary-path", required_argument, NULL, 'b' },
> + { "gen", required_argument, NULL, 'g'},
> + { NULL,0, NULL, 0 }
I think we're missing a single space before the last 0 here for
vertical alignment.
> + };
> +
> + outfile =
[Mesa-dev] [PATCH] intel/tools: new i965_disasm tool
Adds a new i965 instruction disassemble tool
Signed-off-by: Sagar Ghuge
---
src/intel/Makefile.tools.am | 15 +++
src/intel/tools/i965_disasm.c | 202 ++
src/intel/tools/i965_disasm.h | 46
src/intel/tools/meson.build | 11 ++
4 files changed, 274 insertions(+)
create mode 100644 src/intel/tools/i965_disasm.c
create mode 100644 src/intel/tools/i965_disasm.h
diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am
index 00624084e6..36a3a70a28 100644
--- a/src/intel/Makefile.tools.am
+++ b/src/intel/Makefile.tools.am
@@ -22,6 +22,7 @@
noinst_PROGRAMS += \
tools/aubinator \
tools/aubinator_error_decode \
+ tools/i965_disasm \
tools/error2aub
@@ -62,6 +63,20 @@ tools_aubinator_error_decode_CFLAGS = \
$(AM_CFLAGS) \
$(ZLIB_CFLAGS)
+tools_i965_disasm_SOURCES = \
+ tools/i965_disasm.c \
+ tools/i965_disasm.h
+
+tools_i965_disasm_LDADD = \
+ common/libintel_common.la \
+ compiler/libintel_compiler.la \
+ dev/libintel_dev.la \
+ $(top_builddir)/src/util/libmesautil.la \
+ $(PTHREAD_LIBS)
+
+tools_i965_disasm_CFLAGS = \
+ $(AM_CFLAGS)
+
tools_error2aub_SOURCES = \
tools/gen_context.h \
diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c
new file mode 100644
index 00..c880559827
--- /dev/null
+++ b/src/intel/tools/i965_disasm.c
@@ -0,0 +1,202 @@
+/*
+ * Copyright © 2018 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.
+ */
+
+#include
+#include
+
+#include "compiler/brw_inst.h"
+#include "compiler/brw_eu.h"
+
+#include "i965_disasm.h"
+
+uint64_t INTEL_DEBUG;
+uint16_t pci_id = 0;
+FILE *outfile;
+
+struct i965_disasm {
+struct gen_device_info devinfo;
+};
+
+/* Return size of file in bytes pointed by fp */
+static size_t
+i965_disasm_get_file_size(FILE *fp)
+{
+ size_t size = 0;
+
+ fseek(fp, 0L, SEEK_END);
+ size = ftell(fp);
+ fseek(fp, 0L, SEEK_SET);
+
+ return size;
+}
+
+/* Return number of bytes read */
+static size_t
+i965_disasm_read_binary(FILE *fp, void **assembly)
+{
+ size_t end = i965_disasm_get_file_size(fp);
+ *assembly = malloc(end + 1);
+ fread(*assembly, end, 1, fp);
+ fclose(fp);
+
+ return end;
+}
+
+static void
+print_help(const char *progname, FILE *file)
+{
+ fprintf(file,
+ "Usage: %s [OPTION]...\n"
+ "Disassemble i965 instructions from binary file.\n\n"
+ " --help display this help and exit\n"
+ " --binary-path=PATH read binary file from binary file PATH\n"
+ " --gen=platform disassemble instructions for given \n"
+ " platform (3 letter platform name)\n",
+ progname);
+}
+
+int main(int argc, char *argv[])
+{
+ FILE *fp = NULL;
+ void *assembly = NULL;
+ char *binary_path = NULL;
+ size_t start = 0, end = 0;
+ int c, i;
+ struct i965_disasm *disasm;
+
+ bool help = false;
+ const struct option i965_disasm_opts[] = {
+ { "help", no_argument, (int *) , true },
+ { "binary-path", required_argument, NULL, 'b' },
+ { "gen", required_argument, NULL, 'g'},
+ { NULL,0, NULL, 0 }
+ };
+
+ outfile = stderr;
+ i = 0;
+ while ((c = getopt_long(argc, argv, "", i965_disasm_opts, )) != -1) {
+ switch (c) {
+ case 'g': {
+ const int id = gen_device_name_to_pci_device_id(optarg);
+ if (id < 0) {
+fprintf(outfile, "can't parse gen: '%s', expected ivb, byt, hsw, "
+ "bdw, chv, skl, kbl or bxt\n", optarg);
+/* Clean up binary path if given pci id is wrong */
+if (binary_path) {
+ free(binary_path);
+ fclose(fp);
