aux/trace: print enum names instead of integer values in gallium traces
authorMatti Hamalainen <ccr@tnsp.org>
Thu, 9 Dec 2021 18:38:17 +0000 (20:38 +0200)
committerMarge Bot <emma+marge@anholt.net>
Wed, 19 Jan 2022 11:57:17 +0000 (11:57 +0000)
Having only magic constants instead of human-readable strings in
traces not only hinders readability, but also may affect trace
comparision of old and new traces if new enums have been added
or modified (thus possibly changing the values of existing ones.)

So we implement printing of enum names as strings instead.
In order to have those strings, we need to add some new helper
functions, which we will automatically generate from header file
src/gallium/include/pipe/p_defines.h via a new Python script
enums2names.py.

We also bolt this all into the Meson build system.

Link: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4609
Signed-off-by: Matti Hamalainen <ccr@tnsp.org>
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
Reviewed-By: Mike Blumenkrantz <michael.blumenkrantz@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14398>

src/gallium/auxiliary/driver_trace/enums2names.py [new file with mode: 0755]
src/gallium/auxiliary/driver_trace/tr_context.c
src/gallium/auxiliary/driver_trace/tr_dump.h
src/gallium/auxiliary/driver_trace/tr_screen.c
src/gallium/auxiliary/meson.build

diff --git a/src/gallium/auxiliary/driver_trace/enums2names.py b/src/gallium/auxiliary/driver_trace/enums2names.py
new file mode 100755 (executable)
index 0000000..dd34690
--- /dev/null
@@ -0,0 +1,291 @@
+#!/usr/bin/env python3
+# coding=utf-8
+##########################################################################
+#
+# enums2names - Parse and convert enums to translator code
+# (C) Copyright 2021 Matti 'ccr' Hämäläinen <ccr@tnsp.org>
+#
+# 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, sub license, 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 NON-INFRINGEMENT.
+# IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS 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.
+#
+##########################################################################
+
+import sys
+import os.path
+import re
+import signal
+import argparse
+import textwrap
+
+assert sys.version_info >= (3, 6)
+
+
+#
+# List of enums we wish to include in output.
+# NOTE: This needs to be updated if such enums are added.
+#
+lst_enum_include = [
+    "pipe_texture_target",
+    "pipe_shader_type",
+    "pipe_shader_cap",
+    "pipe_shader_ir",
+    "pipe_cap",
+    "pipe_capf",
+    "pipe_compute_cap",
+    "pipe_resource_param",
+    "pipe_fd_type",
+]
+
+
+###
+### Utility functions
+###
+## Fatal error handler
+def pkk_fatal(smsg):
+    print("ERROR: "+ smsg)
+    sys.exit(1)
+
+
+## Handler for SIGINT signals
+def pkk_signal_handler(signal, frame):
+    print("\nQuitting due to SIGINT / Ctrl+C!")
+    sys.exit(1)
+
+
+## Argument parser subclass
+class PKKArgumentParser(argparse.ArgumentParser):
+    def print_help(self):
+        print("enums2names - Parse and convert enums to translator code\n"
+        "(C) Copyright 2021 Matti 'ccr' Hämäläinen <ccr@tnsp.org>\n")
+        super().print_help()
+
+    def error(self, msg):
+        self.print_help()
+        print(f"\nERROR: {msg}", file=sys.stderr)
+        sys.exit(2)
+
+
+def pkk_get_argparser():
+    optparser = PKKArgumentParser(
+        usage="%(prog)s [options] <infile|->\n"
+        "example: %(prog)s ../../include/pipe/p_defines.h -C tr_util.c -H tr_util.h"
+        )
+
+    optparser.add_argument("in_file",
+        type=str,
+        metavar="infile",
+        help="path to input header file p_defines.h (or '-' for stdin)")
+
+    optparser.add_argument("-C",
+        type=str,
+        metavar="outfile",
+        dest="out_source",
+        help="output C source file")
+
+    optparser.add_argument("-H",
+        type=str,
+        metavar="outfile",
+        dest="out_header",
+        help="output C header file")
+
+    optparser.add_argument("-I",
+        type=str,
+        metavar="include",
+        dest="include_file",
+        help="include file / path used for C source output")
+
+    return optparser
+
+
+class PKKHeaderParser:
+
+    def __init__(self, nfilename):
+        self.filename = nfilename
+        self.enums = {}
+        self.state = 0
+        self.nline = 0
+        self.mdata = []
+        self.start = 0
+        self.name = None
+
+    def error(self, msg):
+        pkk_fatal(f"{self.filename}:{self.nline} : {msg}")
+
+    def parse_line(self, sline):
+        # A kingdom for Py3.8 := operator ...
+        smatch = re.match(r'^enum\s+([A-Za-z0-9_]+)\s+.*;', sline)
+        if smatch:
+            pass
+        else:
+            smatch = re.match(r'^enum\s+([A-Za-z0-9_]+)', sline)
+            if smatch:
+                stmp = smatch.group(1)
+
+                if self.state != 0:
+                    self.error(f"enum '{stmp}' starting inside another enum '{self.name}'")
+
+                self.name = stmp
+                self.state = 1
+                self.start = self.nline
+                self.mdata = []
+            else:
+                smatch = re.match(r'^}(\s*|\s*[A-Z][A-Z_]+\s*);', sline)
+                if smatch:
+                    if self.state == 1:
+                        if self.name in self.enums:
+                            self.error("duplicate enum definition '{}', lines {} - {} vs {} - {}".format(
+                            self.name, self.enums[self.name]["start"], self.enums[self.name]["end"],
+                            self.start, self.nline))
+
+                        self.enums[self.name] = {
+                            "data": self.mdata,
+                            "start": self.start,
+                            "end": self.nline
+                        }
+
+                    self.state = 0
+
+                elif self.state == 1:
+                    smatch = re.match(r'([A-Za-z0-9_]+)\s*=\s*(.+)\s*,?', sline)
+                    if smatch:
+                        self.mdata.append(smatch.group(1))
+                    else:
+                        smatch = re.match(r'([A-Za-z0-9_]+)\s*,?', sline)
+                        if smatch:
+                            self.mdata.append(smatch.group(1))
+
+    def parse_file(self, fh):
+        self.nline = 0
+        for line in fh:
+            self.nline += 1
+            self.parse_line(line.strip())
+
+        return self.enums
+
+
+def pkk_output_header(fh):
+    prototypes = [f"const char *\n"
+        f"tr_util_{name}_name(enum {name} value);\n" for name in lst_enum_include]
+
+    print(textwrap.dedent("""\
+        /*
+         * File generated with {program}, please do not edit manually.
+         */
+        #ifndef {include_header_guard}
+        #define {include_header_guard}
+
+
+        #include "pipe/p_defines.h"
+
+
+        #ifdef __cplusplus
+        extern "C" {{
+        #endif
+
+        {prototypes}
+
+        #ifdef __cplusplus
+        }}
+        #endif
+
+        #endif /* {include_header_guard} */\
+        """).format(
+            program=pkk_progname,
+            include_header_guard=re.sub(r'[^A-Z]', '_', os.path.basename(pkk_cfg.out_header).upper()),
+            prototypes="".join(prototypes)
+            ), file=fh)
+
+
+def pkk_output_source(fh):
+    if pkk_cfg.include_file == None:
+        pkk_fatal("Output C source enabled, but include file is not set (-I option).")
+
+    print(textwrap.dedent("""\
+        /*
+         * File generated with {program}, please do not edit manually.
+         */
+        #include "{include_file}"
+        """).format(
+            program=pkk_progname,
+            include_file=pkk_cfg.include_file,
+            ), file=fh)
+
+    for name in lst_enum_include:
+        cases = [f"      case {eid}: return \"{eid}\";\n"
+            for eid in enums[name]["data"]]
+
+        print(textwrap.dedent("""\
+
+            const char *
+            tr_util_{name}_name(enum {name} value)
+            {{
+               switch (value) {{
+            {cases}
+                  default: return "{ucname}_UNKNOWN";
+               }}
+            }}
+            """).format(
+                name=name,
+                ucname=name.upper(),
+                cases="".join(cases)
+                ), file=fh)
+
+###
+### Main program starts
+###
+if __name__ == "__main__":
+    signal.signal(signal.SIGINT, pkk_signal_handler)
+
+    ### Parse arguments
+    pkk_progname = sys.argv[0]
+    optparser = pkk_get_argparser()
+    pkk_cfg = optparser.parse_args()
+
+    ### Parse input
+    hdrparser = PKKHeaderParser(pkk_cfg.in_file)
+
+    try:
+        if pkk_cfg.in_file != "-":
+            with open(pkk_cfg.in_file, "r", encoding="UTF-8") as fh:
+                enums = hdrparser.parse_file(fh)
+        else:
+            enums = hdrparser.parse_file(sys.stdin)
+
+    except OSError as e:
+        pkk_fatal(str(e))
+
+    ### Check if any of the required enums are missing
+    errors = False
+    for name in lst_enum_include:
+        if name not in enums:
+            print(f"ERROR: Missing enum '{name}'!")
+            errors = True
+
+    if errors:
+        pkk_fatal(f"Errors in input. Edit this script ({pkk_progname}) to add/remove included enums.")
+
+    ### Perform output
+    if pkk_cfg.out_header:
+        with open(pkk_cfg.out_header, "w", encoding="UTF-8") as fh:
+            pkk_output_header(fh)
+
+    if pkk_cfg.out_source:
+        with open(pkk_cfg.out_source, "w", encoding="UTF-8") as fh:
+            pkk_output_source(fh)
index 66e6ce8..5c58b3f 100644 (file)
@@ -41,6 +41,7 @@
 #include "tr_screen.h"
 #include "tr_texture.h"
 #include "tr_context.h"
+#include "tr_util.h"
 
 
 struct trace_query
@@ -490,7 +491,7 @@ trace_context_bind_sampler_states(struct pipe_context *_pipe,
    trace_dump_call_begin("pipe_context", "bind_sampler_states");
 
    trace_dump_arg(ptr, pipe);
-   trace_dump_arg(uint, shader);
+   trace_dump_arg_enum(shader, tr_util_pipe_shader_type_name(shader));
    trace_dump_arg(uint, start);
    trace_dump_arg(uint, num_states);
    trace_dump_arg_array(ptr, states, num_states);
@@ -922,7 +923,7 @@ trace_context_set_constant_buffer(struct pipe_context *_pipe,
    trace_dump_call_begin("pipe_context", "set_constant_buffer");
 
    trace_dump_arg(ptr, pipe);
-   trace_dump_arg(uint, shader);
+   trace_dump_arg_enum(shader, tr_util_pipe_shader_type_name(shader));
    trace_dump_arg(uint, index);
    trace_dump_arg(bool, take_ownership);
    trace_dump_arg(constant_buffer, constant_buffer);
@@ -965,7 +966,7 @@ trace_context_set_inlinable_constants(struct pipe_context *_pipe, enum pipe_shad
    trace_dump_call_begin("pipe_context", "set_inlinable_constants");
 
    trace_dump_arg(ptr, pipe);
-   trace_dump_arg(uint, shader);
+   trace_dump_arg_enum(shader, tr_util_pipe_shader_type_name(shader));
    trace_dump_arg(uint, num_values);
    trace_dump_arg_array(uint, values, num_values);
 
@@ -1210,7 +1211,7 @@ trace_context_set_sampler_views(struct pipe_context *_pipe,
    trace_dump_call_begin("pipe_context", "set_sampler_views");
 
    trace_dump_arg(ptr, pipe);
-   trace_dump_arg(uint, shader);
+   trace_dump_arg_enum(shader, tr_util_pipe_shader_type_name(shader));
    trace_dump_arg(uint, start);
    trace_dump_arg(uint, num);
    trace_dump_arg(uint, unbind_num_trailing_slots);
@@ -1581,7 +1582,7 @@ trace_context_create_fence_fd(struct pipe_context *_pipe,
    trace_dump_call_begin("pipe_context", "create_fence_fd");
 
    trace_dump_arg(ptr, pipe);
-   trace_dump_arg(int, fd);
+   trace_dump_arg_enum(fd, tr_util_pipe_fd_type_name(fd));
    trace_dump_arg(uint, type);
 
    pipe->create_fence_fd(pipe, fence, fd, type);
index 9a4b294..d6b862d 100644 (file)
@@ -123,6 +123,13 @@ bool trace_dump_is_triggered(void);
       trace_dump_arg_end(); \
    } while(0)
 
+#define trace_dump_arg_enum(_arg, _value) \
+   do { \
+      trace_dump_arg_begin(#_arg); \
+      trace_dump_enum(_value); \
+      trace_dump_arg_end(); \
+   } while(0)
+
 #define trace_dump_arg_struct(_type, _arg) \
    do { \
       trace_dump_arg_begin(#_arg); \
index 839d899..4174fe6 100644 (file)
@@ -37,6 +37,7 @@
 #include "tr_context.h"
 #include "tr_screen.h"
 #include "tr_public.h"
+#include "tr_util.h"
 
 
 static bool trace = false;
@@ -148,7 +149,7 @@ trace_screen_get_param(struct pipe_screen *_screen,
    trace_dump_call_begin("pipe_screen", "get_param");
 
    trace_dump_arg(ptr, screen);
-   trace_dump_arg(int, param);
+   trace_dump_arg_enum(param, tr_util_pipe_cap_name(param));
 
    result = screen->get_param(screen, param);
 
@@ -172,8 +173,8 @@ trace_screen_get_shader_param(struct pipe_screen *_screen,
    trace_dump_call_begin("pipe_screen", "get_shader_param");
 
    trace_dump_arg(ptr, screen);
-   trace_dump_arg(uint, shader);
-   trace_dump_arg(int, param);
+   trace_dump_arg_enum(shader, tr_util_pipe_shader_type_name(shader));
+   trace_dump_arg_enum(param, tr_util_pipe_shader_cap_name(param));
 
    result = screen->get_shader_param(screen, shader, param);
 
@@ -196,7 +197,7 @@ trace_screen_get_paramf(struct pipe_screen *_screen,
    trace_dump_call_begin("pipe_screen", "get_paramf");
 
    trace_dump_arg(ptr, screen);
-   trace_dump_arg(int, param);
+   trace_dump_arg_enum(param, tr_util_pipe_capf_name(param));
 
    result = screen->get_paramf(screen, param);
 
@@ -220,8 +221,8 @@ trace_screen_get_compute_param(struct pipe_screen *_screen,
    trace_dump_call_begin("pipe_screen", "get_compute_param");
 
    trace_dump_arg(ptr, screen);
-   trace_dump_arg(int, ir_type);
-   trace_dump_arg(int, param);
+   trace_dump_arg_enum(ir_type, tr_util_pipe_shader_ir_name(ir_type));
+   trace_dump_arg_enum(param, tr_util_pipe_compute_cap_name(param));
    trace_dump_arg(ptr, data);
 
    result = screen->get_compute_param(screen, ir_type, param, data);
@@ -250,7 +251,7 @@ trace_screen_is_format_supported(struct pipe_screen *_screen,
 
    trace_dump_arg(ptr, screen);
    trace_dump_arg(format, format);
-   trace_dump_arg(int, target);
+   trace_dump_arg_enum(target, tr_util_pipe_texture_target_name(target));
    trace_dump_arg(uint, sample_count);
    trace_dump_arg(uint, storage_sample_count);
    trace_dump_arg(uint, tex_usage);
index 169d481..9da9318 100644 (file)
@@ -472,6 +472,15 @@ if with_dri2 and with_platform_x11
   endif
 endif
 
+prog_enums2names = files('driver_trace/enums2names.py')
+files_libgallium += custom_target(
+  'tr_util.[ch]',
+  input: '../include/pipe/p_defines.h',
+  output: ['tr_util.c', 'tr_util.h'],
+  command: [prog_python, prog_enums2names, '@INPUT@', '-C', '@OUTPUT0@', '-H', '@OUTPUT1@', '-I', 'tr_util.h'],
+  depend_files: prog_enums2names,
+)
+
 files_libgallium += custom_target(
   'u_tracepoints.c',
   input: 'util/u_tracepoints.py',