[Pixman] [PATCH 4/5] lowlevel-blt-bench: Parse test name strings in general case
Pekka Paalanen
ppaalanen at gmail.com
Mon Mar 16 07:33:47 PDT 2015
On Tue, 3 Mar 2015 15:24:19 +0000
Ben Avison <bavison at riscosopen.org> wrote:
> There are many types of composite operation that are useful to benchmark but
> which are omitted from the table. Continually having to add extra entries to
> the table is a nuisance and is prone to human error, so this patch adds the
> ability to break down unknow strings of the format
> <operation>_<src>[_<mask]<dst>[_ca]
> where bitmap formats are specified by number of bits of each component
> (assumed in ARGB order) or 'n' to indicate a solid source or mask.
>
> The only downside to this approach is that specifying 'all' instead of a
> specific operation will remain limited to the set of operations from the
> table.
> ---
> test/lowlevel-blt-bench.c | 131 ++++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 123 insertions(+), 8 deletions(-)
>
> diff --git a/test/lowlevel-blt-bench.c b/test/lowlevel-blt-bench.c
> index 3da094a..f05a663 100644
> --- a/test/lowlevel-blt-bench.c
> +++ b/test/lowlevel-blt-bench.c
> @@ -51,6 +51,12 @@
>
> #define EXCLUDE_OVERHEAD 1
>
> +typedef struct
> +{
> + const char *name;
> + uint32_t number;
> +} name_to_number_t;
> +
> uint32_t *dst;
> uint32_t *src;
> uint32_t *mask;
> @@ -367,14 +373,14 @@ bench_RT (pixman_op_t op,
> }
>
> void
> -bench_composite (char * testname,
> - int src_fmt,
> - int src_flags,
> - int op,
> - int mask_fmt,
> - int mask_flags,
> - int dst_fmt,
> - double npix)
> +bench_composite (const char * testname,
> + int src_fmt,
> + int src_flags,
> + int op,
> + int mask_fmt,
> + int mask_flags,
> + int dst_fmt,
> + double npix)
> {
> pixman_image_t * src_img;
> pixman_image_t * dst_img;
> @@ -719,12 +725,36 @@ tests_tbl[] =
> { "rpixbuf", PIXMAN_x8b8g8r8, 0, PIXMAN_OP_SRC, PIXMAN_a8b8g8r8, 0, PIXMAN_a8b8g8r8 },
> };
>
> +static uint32_t
> +lookup_name_to_number (const char **string, const name_to_number_t *array, size_t entries)
> +{
> + size_t entry, i;
> + for (entry = 0; entry < entries; entry++)
> + {
> + for (i = 0; (*string)[i] == array[entry].name[i]; i++)
> + {
> + if ((*string)[i] == 0)
> + {
> + *string += i;
> + return array[entry].number;
> + }
> + }
> + if ((*string)[i] == '_' && array[entry].name[i] == 0)
> + {
> + *string += i;
> + return array[entry].number;
> + }
> + }
> + return 0;
> +}
> +
> int
> main (int argc, char *argv[])
> {
> double x;
> int i;
> const char *pattern = NULL;
> + int matches = 0;
> for (i = 1; i < argc; i++)
> {
> if (argv[i][0] == '-')
> @@ -805,6 +835,7 @@ main (int argc, char *argv[])
> {
> if (strcmp (pattern, "all") == 0 || strcmp (tests_tbl[i].testname, pattern) == 0)
> {
> + ++matches;
> bench_composite (tests_tbl[i].testname,
> tests_tbl[i].src_fmt,
> tests_tbl[i].src_flags,
> @@ -816,6 +847,90 @@ main (int argc, char *argv[])
> }
> }
>
> + if (matches == 0)
> + {
> + /* Try parsing the string instead */
> + static const name_to_number_t combine_type[] = {
> + { "src", PIXMAN_OP_SRC },
> + { "over_reverse", PIXMAN_OP_OVER_REVERSE },
> + { "overrev", PIXMAN_OP_OVER_REVERSE },
> + { "over", PIXMAN_OP_OVER },
> + { "in_reverse", PIXMAN_OP_IN_REVERSE },
> + { "inrev", PIXMAN_OP_IN_REVERSE },
> + { "in", PIXMAN_OP_IN },
> + { "out_reverse", PIXMAN_OP_OUT_REVERSE },
> + { "outrev", PIXMAN_OP_OUT_REVERSE },
> + { "out", PIXMAN_OP_OUT },
> + { "atop_reverse", PIXMAN_OP_ATOP_REVERSE },
> + { "atoprev", PIXMAN_OP_ATOP_REVERSE },
> + { "atop", PIXMAN_OP_ATOP },
> + { "xor", PIXMAN_OP_XOR },
> + { "add", PIXMAN_OP_ADD }
> + };
> + static const name_to_number_t format[] = {
> + { "8888", PIXMAN_a8r8g8b8 },
> + { "x888", PIXMAN_x8r8g8b8 },
> + { "0565", PIXMAN_r5g6b5 },
> + { "1555", PIXMAN_a1r5g5b5 },
> + { "8", PIXMAN_a8 }
> + };
Hi,
reading Søren's comment:
> ==================================================================
> 0002: lowlevel parse Needs work
> 0003: affine benchmarker Needs work
> ==================================================================
>
> Both of these could benefit from taking the support for
> format/operator parsing and printing in check-format.c and moving it
> to utils.[ch], perhaps extended to deal with strings like 'none' etc.
I understood these arrays should have been replaced by
format_from_string/operator_from_string functions from patch 2 and
similar new functions doing lookups in the same arrays.
Hmm, but I see there are several alternative spellings for operators,
and the formats follow a whole new convention.
Maybe we could have aliases for name-to-operator lookups, and even for
name-to-format lookups. The parser could accept both "x888" and
"x8r8b8g8" kinds of format names. If these were added already in the
tables in test/utils.c, the lookup functions could be used directly
here. This would extend the way test cases can be expressed, because
instead of "8888" you could take "a8b8g8r8" or whatever that needs a
swizzle in the operation.
The name tables could be ordered such that functions like format_name
() can just return the first match.
Adding the "fake formats" to the tables shouldn't hurt either.
Dear maintainers, what do you think?
> + const char *p = pattern;
> + int src_fmt;
> + int src_flags = 0;
> + int mask_fmt;
> + int mask_flags = 0;
> + int dst_fmt;
> + int op = lookup_name_to_number(&p, combine_type, sizeof combine_type / sizeof *combine_type);
> + if (op != 0 && *p++ == '_')
> + {
> + if (p[0] == 'n' && p[1] == '_')
> + {
> + src_fmt = PIXMAN_a8r8g8b8;
> + src_flags = SOLID_FLAG;
> + ++p;
> + }
> + else
> + src_fmt = lookup_name_to_number(&p, format, sizeof format / sizeof *format);
> + if (src_fmt != 0 && *p++ == '_')
> + {
> + if (p[0] == 'n' && p[1] == '_')
> + {
> + mask_fmt = PIXMAN_a8r8g8b8;
> + mask_flags = SOLID_FLAG;
> + ++p;
> + }
> + else
> + mask_fmt = lookup_name_to_number(&p, format, sizeof format / sizeof *format);
> + if (mask_fmt != 0)
> + {
> + if (*p == 0)
> + {
> + dst_fmt = mask_fmt;
> + mask_fmt = PIXMAN_null;
> + }
> + else
> + {
> + ++p;
> + dst_fmt = lookup_name_to_number(&p, format, sizeof format / sizeof *format);
> + if (strcmp (p, "_ca") == 0)
> + mask_flags |= CA_FLAG;
> + else if (*p != 0)
> + dst_fmt = 0;
> + }
> + if (dst_fmt != 0)
> + bench_composite (pattern,
> + src_fmt,
> + src_flags,
> + op,
> + mask_fmt,
> + mask_flags,
> + dst_fmt,
> + bandwidth/8);
> + }
> + }
> + }
> + }
Personally I'm not really much of a fan of this much nesting,
especially where you have a number of parsing steps and each step nests
a bit more. I'd refactor this code into a function and use early (error)
returns instead of nesting.
Looks like there is also no complaint, if the given string is not a
valid test name.
> +
> free (src);
> return 0;
> }
Thanks,
pq
More information about the Pixman
mailing list