[Piglit] [PATCH 7/7] cl-program-tester: Add gentype option to config section

Aaron Watry awatry at gmail.com
Tue Oct 15 00:57:12 CEST 2013


On Mon, Sep 30, 2013 at 9:47 AM, Tom Stellard <tom at stellard.net> wrote:
> From: Tom Stellard <thomas.stellard at amd.com>
>
> The gentype option allows a test file to specify a list of types and
> vector sizes to be used for each test.  cl-program-tester will run each
> test once for each comibnation of type and vector size.
>
> This makes it possible to write a generic test that can be run on a
> number of different data types.  The macro _PIGLIT_GENTYPE is used to
> specify which types in the kernel will be generic and it will be replaced
> with the current type for each test run.
>
> This patch also adds the kernel-args.cl test which demonstrates how to
> use the new gentype option.
> ---
>  tests/cl/doc_program.cl                  |  11 +-
>  tests/cl/doc_program.program_test        |  10 ++
>  tests/cl/program/execute/kernel-args.cl  |  19 ++
>  tests/cl/program/program-tester.c        | 157 ++++++++++++++++-
>  tests/util/piglit-framework-cl-program.c | 288 +++++++++++++++++++------------
>  tests/util/piglit-framework-cl-program.h |  43 +++++
>  tests/util/piglit-framework-cl.c         |  12 ++
>  tests/util/piglit-util-cl.h              |   3 +-
>  8 files changed, 420 insertions(+), 123 deletions(-)
>  create mode 100644 tests/cl/program/execute/kernel-args.cl
>
> diff --git a/tests/cl/doc_program.cl b/tests/cl/doc_program.cl
> index 84e92ee..7f7fee2 100644
> --- a/tests/cl/doc_program.cl
> +++ b/tests/cl/doc_program.cl
> @@ -30,7 +30,16 @@ expect_test_fail: true       # Expect that tests will fail (arguments won't chec
>  dimensions: 3                # Number of dimensions for ND kernel (default: 1)
>  global_size: 10 10 10        # Global work size for ND kernel (default: 1 0 0)
>  local_size:   2  2  2        # Local work size for ND kernel (default: NULL)
> -
> +gentype: char uchar 2 3      # List of types to test with.  If set, the program test will
> +                             # run once for each combination of type and vector size and
> +                             # the macro _PIGLIT_GENTYPE will set to the current type being
> +                             # tested.
> +                             # The accepted types are: char, uchar, short, ushort, int, uint,
> +                             #                         long, ulong, float, double
> +                             # The accepted vector sizes are: 1, 2, 3, 4, 8, 16 and
> +                             # s -> Use only scalar sizes (1)
> +                             # v -> Use only vector sizes (2, 3, 4, 8, 16)
> +                             # n -> Use all sizes (1, 2, 3, 4, 8, 16)
>
>  # Execution tests #
>
> diff --git a/tests/cl/doc_program.program_test b/tests/cl/doc_program.program_test
> index 3ff67a6..5139255 100644
> --- a/tests/cl/doc_program.program_test
> +++ b/tests/cl/doc_program.program_test
> @@ -27,6 +27,16 @@ expect_test_fail: true       # Expect that tests will fail (arguments won't chec
>  dimensions: 3                # Number of dimensions for ND kernel (default: 1)
>  global_size: 10 10 10        # Global work size for ND kernel (default: 1 0 0)
>  local_size:   2  2  2        # Local work size for ND kernel (default: NULL)
> +gentype: char uchar 2 3      # List of types to test with.  If set, the program test will
> +                             # run once for each combination of type and vector size and
> +                             # the macro _PIGLIT_GENTYPE will set to the current type being
> +                             # tested.
> +                             # The accepted types are: char, uchar, short, ushort, int, uint,
> +                             #                         long, ulong, float, double
> +                             # The accepted vector sizes are: 1, 2, 3, 4, 8, 16 and
> +                             # s -> Use only scalar sizes (1)
> +                             # v -> Use only vector sizes (2, 3, 4, 8, 16)
> +                             # n -> Use all sizes (1, 2, 3, 4, 8, 16)
>
>  #program_source_file: "program.cl"  # Program source file path to use as source (relative to this file)
>  #program_binary_file: "program.bin" # Program binary file path to use as binary (relative to this file)
> diff --git a/tests/cl/program/execute/kernel-args.cl b/tests/cl/program/execute/kernel-args.cl
> new file mode 100644
> index 0000000..1923d3e
> --- /dev/null
> +++ b/tests/cl/program/execute/kernel-args.cl
> @@ -0,0 +1,19 @@
> +/*!
> +[config]
> +dimensions: 1
> +global_size: 1 0 0
> +kernel_name: test
> +gentype: char uchar short ushort int uint float n
> +
> +[test]
> +name: A
> +arg_out: 0 buffer gentype[2] 1 2
> +arg_in:  1 gentype 1
> +arg_in:  2 gentype 2
> +
> +!*/
> +
> +kernel void test(global _PIGLIT_GENTYPE *out, _PIGLIT_GENTYPE in, _PIGLIT_GENTYPE in2) {
> +       out[0] = in;
> +       out[1] = in2;
> +}
> diff --git a/tests/cl/program/program-tester.c b/tests/cl/program/program-tester.c
> index fbfeb7b..d65548c 100644
> --- a/tests/cl/program/program-tester.c
> +++ b/tests/cl/program/program-tester.c
> @@ -104,6 +104,7 @@
>  #define REGEX_TYPE_UINT       REGEX_DEFINE_TYPE("uint")
>  #define REGEX_TYPE_LONG       REGEX_DEFINE_TYPE("long")
>  #define REGEX_TYPE_ULONG      REGEX_DEFINE_TYPE("ulong")
> +#define REGEX_TYPE_GENTYPE    "gentype"
>  // half is defined as unsigned short and C can't read/write its value.
>  // Also half is only used as a storage format unless device supports
>  // cl_khr_fp16
> @@ -117,7 +118,7 @@
>  #define REGEX_TYPE  REGEX_TYPE_CHAR "|" REGEX_TYPE_UCHAR "|" \
>                      REGEX_TYPE_SHORT "|" REGEX_TYPE_USHORT "|" \
>                      REGEX_TYPE_INT "|" REGEX_TYPE_UINT "|" REGEX_TYPE_LONG "|" \
> -                    REGEX_TYPE_ULONG "|" REGEX_TYPE_FLOAT
> +                    REGEX_TYPE_ULONG "|" REGEX_TYPE_FLOAT "|" REGEX_TYPE_GENTYPE
>
>  /*
>   * Value argument:
> @@ -833,8 +834,35 @@ get_section_content(const char* src, char** content)
>         return size;
>  }
>
> +static void* vectorize_value(int* array, int type_size, int buffer_length,
> +                            int vec_elements)
> +{
> +       unsigned i;
> +       char* newarray;
> +       unsigned in_mem_vec_elements;
> +       /* Special case for vec3 buffer since they are aligned to 4 dwords
> +        * in memory.
> +        */
> +       if (buffer_length == 0) {
> +               in_mem_vec_elements = vec_elements;
> +       } else {
> +               in_mem_vec_elements = piglit_cl_get_num_mem_elements(vec_elements);
> +       }
> +       newarray = calloc(type_size, buffer_length * in_mem_vec_elements);
> +       for (i = 0; i < buffer_length; i++){
> +               int j;
> +               for (j = 0; j < vec_elements; j++) {
> +                       memcpy(newarray + (i * type_size * in_mem_vec_elements) + (j * type_size),
> +                       array + i, type_size);
> +               }
> +       }
> +       free(array);
> +       return newarray;
> +}
> +
>  void
> -get_test_arg_value(struct test_arg* test_arg, const char* value, size_t length)
> +get_test_arg_value(struct test_arg* test_arg, const char* value, size_t length,
> +                       unsigned gentype_size, unsigned gentype_elements)
>  {
>         size_t i; // index in array
>         size_t c; // component in element
> @@ -876,10 +904,16 @@ get_test_arg_value(struct test_arg* test_arg, const char* value, size_t length)
>                 CASE(PIGLIT_CL_TYPE_LONG,   cl_long,    get_int_array,    int_array)
>                 CASE(PIGLIT_CL_TYPE_ULONG,  cl_ulong,   get_uint_array,   uint_array)
>                 CASE(PIGLIT_CL_TYPE_FLOAT,  cl_float,   get_float_array,  float_array)
> +               CASE(PIGLIT_CL_TYPE_GENTYPE, cl_uint,   get_uint_array,   uint_array)
>         }
>
>  #undef CASE
>
> +       if (test_arg->cl_type == PIGLIT_CL_TYPE_GENTYPE) {
> +               test_arg->value = vectorize_value(test_arg->value, gentype_size,
> +                                       test_arg->length, gentype_elements);
> +       }
> +
>         free(int_array);
>         free(uint_array);
>         free(float_array);
> @@ -984,6 +1018,7 @@ get_test_arg(const char* src, struct test* test, bool arg_in)
>         ELSEIF(REGEX_TYPE_LONG,   PIGLIT_CL_TYPE_LONG)
>         ELSEIF(REGEX_TYPE_ULONG,  PIGLIT_CL_TYPE_ULONG)
>         ELSEIF(REGEX_TYPE_FLOAT,  PIGLIT_CL_TYPE_FLOAT)
> +       ELSEIF(REGEX_TYPE_GENTYPE, PIGLIT_CL_TYPE_GENTYPE)
>
>  #undef IF
>  #undef ELSEIF
> @@ -1084,6 +1119,72 @@ get_test_arg(const char* src, struct test* test, bool arg_in)
>         }
>  }
>
> +static uint32_t
> +parse_gentype(char *gentype_str)
> +{
> +       char *type;
> +       uint32_t gentype;
> +       while ((type = strtok(gentype_str, " "))) {
> +               char *endptr;
> +               int length;
> +               /* strtok requres that the first paramter be NULL for all

s/requres/requires
s/paramter/parameter

> +                * calls after the fist. */
> +               gentype_str = NULL;
> +               length = strtol(type, &endptr, 0);
> +               if (endptr != type) {
> +                       switch (length) {
> +                       case  1: gentype |= GENLEN_1;  break;
> +                       case  2: gentype |= GENLEN_2;  break;
> +                       case  3: gentype |= GENLEN_3;  break;
> +                       case  4: gentype |= GENLEN_4;  break;
> +                       case  8: gentype |= GENLEN_8;  break;
> +                       case 16: gentype |= GENLEN_16; break;
> +                       default:
> +                               fprintf(stderr, "%d is not a valid gentype length\n", length);
> +                               exit_report_result(PIGLIT_FAIL);
> +                               break;
> +                       }
> +                       continue;
> +               }
> +
> +               /* The token is not an integer, so we have a type name or
> +                * a special identifier.
> +                */
> +
> +               if (!strcmp(type, "char")) {
> +                       gentype |= GENTYPE_CHAR;
> +               } else if (!strcmp(type, "uchar")) {
> +                       gentype |= GENTYPE_UCHAR;
> +               } else if (!strcmp(type, "short")) {
> +                       gentype |= GENTYPE_SHORT;
> +               } else if (!strcmp(type, "ushort")) {
> +                       gentype |= GENTYPE_USHORT;
> +               } else if (!strcmp(type, "int")) {
> +                       gentype |= GENTYPE_INT;
> +               } else if (!strcmp(type, "uint")) {
> +                       gentype |= GENTYPE_UINT;
> +               } else if (!strcmp(type, "long")) {
> +                       gentype |= GENTYPE_LONG;
> +               } else if (!strcmp(type, "ulong")) {
> +                       gentype |= GENTYPE_ULONG;
> +               } else if (!strcmp(type, "float")) {
> +                       gentype |= GENTYPE_FLOAT;
> +               } else if (!strcmp(type, "double")) {
> +                       gentype |= GENTYPE_DOUBLE;
> +               } else if (!strcmp(type, "s")) {
> +                       gentype |= GENLEN_SCALAR;
> +               } else if (!strcmp(type, "v")) {
> +                       gentype |= GENLEN_VECTOR;
> +               } else if (!strcmp(type, "n")) {
> +                       gentype |= GENLEN_SCALAR | GENLEN_VECTOR;
> +               } else {
> +                       fprintf(stderr, "%s is not a valid gentype type\n");
> +                       exit_report_result(PIGLIT_FAIL);
> +               }
> +       }
> +       return gentype;
> +}
> +
>  /**
>   * Helper function for parsing a test name and checking for illegal characters.
>   */
> @@ -1316,6 +1417,11 @@ parse_config(const char* config_str,
>                                         } else {
>                                                 local_work_size_null = true;
>                                         }
> +                               } else if (regex_match(key, "^gentype$")) {
> +                                       if (!regex_match(value, REGEX_FULL_MATCH(REGEX_NULL))) {
> +                                               config->gentype = parse_gentype(value);
> +                                       }
> +
>                                 } else {
>                                         fprintf(stderr,
>                                                 "Invalid configuration, key '%s' does not belong to a [config] section: %s\n",
> @@ -1653,6 +1759,7 @@ check_test_arg_value(struct test_arg test_arg,
>                 CASEI(PIGLIT_CL_TYPE_LONG,   "long",   cl_long)
>                 CASEU(PIGLIT_CL_TYPE_ULONG,  "ulong",  cl_ulong)
>                 CASEF(PIGLIT_CL_TYPE_FLOAT,  "float",  cl_float)
> +               CASEI(PIGLIT_CL_TYPE_GENTYPE, "gentype", cl_int)

This is defined as cl_int here, but the CASE hunk in
get_test_arg_value seems to imply cl_uint...  Are these unrelated
usages, or is this a bug in disguise?

>         }
>
>  #undef CASEF
> @@ -1714,13 +1821,20 @@ test_kernel(const struct piglit_cl_program_test_config* config,
>         for(j = 0; j < test.num_args_in; j++) {
>                 bool arg_set = false;
>                 struct test_arg test_arg = test.args_in[j];
> +               unsigned arg_size = test_arg.vec_elements * piglit_cl_type_get_size(test_arg.cl_type);
> +               if (test_arg.cl_type == PIGLIT_CL_TYPE_GENTYPE) {
> +                       test_arg.size_in_mem = test_arg.length * env->gentype_size *
> +                                       piglit_cl_get_num_mem_elements(env->gentype_elements);
> +                       arg_size = test_arg.length * env->gentype_size * env->gentype_elements;
> +               }
>
>                 switch(test_arg.type) {
>                 case TEST_ARG_VALUE:
> -                       get_test_arg_value(&test_arg, test_arg.raw_value, test_arg.vec_elements);
> +                       get_test_arg_value(&test_arg, test_arg.raw_value, test_arg.vec_elements,
> +                                       env->gentype_size, env->gentype_elements);
>                         arg_set = piglit_cl_set_kernel_arg(kernel,
>                                                            test_arg.index,
> -                                                          test_arg.vec_elements * piglit_cl_type_get_size(test_arg.cl_type),
> +                                                          arg_size,

This line was already messed up, but could you fix the indentation on
this line?  There's mixed tabs/spaces here which does not agree with
the surrounding lines.

>                                                            test_arg.value);
>                         break;
>                 case TEST_ARG_BUFFER: {
> @@ -1737,11 +1851,13 @@ test_kernel(const struct piglit_cl_program_test_config* config,
>
>                                         get_test_arg_value(&test_arg,
>                                                            repeat_value_str,
> -                                                          get_array_length(repeat_value_str));
> +                                                          get_array_length(repeat_value_str),
> +                                                          env->gentype_size, env->gentype_elements);
>                                 } else {
>                                         get_test_arg_value(&test_arg,
>                                                            test_arg.raw_value,
> -                                                          test_arg.length * test_arg.vec_elements);
> +                                                          test_arg.length * test_arg.vec_elements,
> +                                                          env->gentype_size, env->gentype_elements);
>                                 }
>                                 buffer_arg.buffer = piglit_cl_create_buffer(env->context,
>                                                                             CL_MEM_READ_WRITE,
> @@ -1787,6 +1903,12 @@ test_kernel(const struct piglit_cl_program_test_config* config,
>         for(j = 0; j < test.num_args_out; j++) {
>                 bool arg_set = false;
>                 struct test_arg test_arg = test.args_out[j];
> +               if (test_arg.cl_type == PIGLIT_CL_TYPE_GENTYPE) {
> +                       test_arg.size_in_mem = (test_arg.length * env->gentype_size * env->gentype_elements);
> +                       test_arg.size_in_mem = test_arg.length * env->gentype_size *
> +                               (env->gentype_elements == 3 ? 4 : env->gentype_elements);

We have a helper function for this now... piglit_cl_get_num_mem_elements()

> +               }
> +                                       printf("test_arg.size is %u\n", test_arg.size_in_mem);

This indentation looks off.

>
>                 switch(test_arg.type) {
>                 case TEST_ARG_VALUE:
> @@ -1864,6 +1986,10 @@ test_kernel(const struct piglit_cl_program_test_config* config,
>                 int k;
>                 bool arg_valid = false;
>                 struct test_arg test_arg = test.args_out[j];
> +               if (test_arg.cl_type == PIGLIT_CL_TYPE_GENTYPE) {
> +                       test_arg.size_in_mem = test_arg.length * env->gentype_size *
> +                               piglit_cl_get_num_mem_elements(env->gentype_elements);
> +               }
>
>                 switch(test_arg.type) {
>                 case TEST_ARG_VALUE:
> @@ -1871,6 +1997,7 @@ test_kernel(const struct piglit_cl_program_test_config* config,
>                         break;
>                 case TEST_ARG_BUFFER: {
>                         struct buffer_arg buffer_arg;
> +                       void* read_value;
>
>                         /* Find the right buffer */
>                         for(k = 0; k < num_buffer_args; k++) {
> @@ -1889,19 +2016,31 @@ test_kernel(const struct piglit_cl_program_test_config* config,
>
>                                         get_test_arg_value(&test_arg,
>                                                            repeat_value_str,
> -                                                          get_array_length(repeat_value_str));
> +                                                          get_array_length(repeat_value_str),
> +                                                          env->gentype_size, env->gentype_elements);
>                                 } else {
>                                         get_test_arg_value(&test_arg,
>                                                            test_arg.raw_value,
> -                                                          test_arg.length * test_arg.vec_elements);
> +                                                          test_arg.length * test_arg.vec_elements,
> +                                                          env->gentype_size, env->gentype_elements);
> +                               }
> +                               if (test_arg.cl_type == PIGLIT_CL_TYPE_GENTYPE) {
> +                                       /* GENTYPE comparisons always assume an integer array,
> +                                        * so we need to makre sure the output pointer

s/makre/make/

> +                                        * is aligned to 4 bytes and also initialized to
> +                                        * zero.
> +                                        */
> +                                       read_value = calloc(1, test_arg.size_in_mem + (4 - (test_arg.size_in_mem %4)));
> +                               } else {
> +                                       read_value = malloc(test_arg.size_in_mem);
>                                 }
> -                               void* read_value = malloc(test_arg.size_in_mem);
>
>                                 if(piglit_cl_read_buffer(env->context->command_queues[0],
>                                                              buffer_arg.buffer,
>                                                              0,
>                                                              test_arg.size_in_mem,
>                                                              read_value)) {
> +                                       unsigned i;
>                                         arg_valid = true;
>                                         if(check_test_arg_value(test_arg, read_value)) {
>                                                 printf(" Argument %u: PASS%s\n",
> diff --git a/tests/util/piglit-framework-cl-program.c b/tests/util/piglit-framework-cl-program.c
> index 417405d..24badae 100644
> --- a/tests/util/piglit-framework-cl-program.c
> +++ b/tests/util/piglit-framework-cl-program.c
> @@ -119,6 +119,131 @@ void piglit_cl_program_test_init(const int argc,
>         }
>  }
>
> +enum piglit_result build_and_execute_program(
> +       const int argc,
> +       const char **argv,
> +       struct piglit_cl_program_test_env *env,
> +       struct piglit_cl_program_test_config* config,
> +       char *build_options)
> +{
> +       enum piglit_result result;
> +       unsigned i;
> +       printf("#   Build options: %s\n", build_options);
> +
> +       /* Create and build program */
> +       if(config->program_source != NULL) {
> +               if(!config->expect_build_fail) {
> +                       env->program = piglit_cl_build_program_with_source(env->context,
> +                                                                         1,
> +                                                                         &config->program_source,
> +                                                                         build_options);
> +               } else {
> +                       env->program = piglit_cl_fail_build_program_with_source(env->context,
> +                                                                              1,
> +                                                                              &config->program_source,
> +                                                                              build_options);
> +               }
> +       } else if(config->program_source_file != NULL) {
> +               unsigned int size;
> +               char* program_source;
> +
> +               program_source = piglit_load_text_file(config->program_source_file, &size);
> +               if(program_source != NULL && size > 0) {
> +                       if(!config->expect_build_fail) {
> +                               env->program = piglit_cl_build_program_with_source(env->context,
> +                                                                                 1,
> +                                                                                 &program_source,
> +                                                                                 build_options);
> +                       } else {
> +                               env->program = piglit_cl_fail_build_program_with_source(env->context,
> +                                                                                      1,
> +                                                                                      &program_source,
> +                                                                                      build_options);
> +                       }
> +               } else {
> +                       fprintf(stderr, "Program source file %s does not exists or is empty\n",
> +                               config->program_source_file);
> +                       return PIGLIT_WARN;
> +               }
> +               free(program_source);

Instead of duplicating most of the above if/else-if block, could we
instead just check for source/source-file and then attempt to build
the program, followed by freeing the program_source if necessary?  I
see by looking below that this is mostly just moving code around, but
I wouldn't mind seeing this simplified a bit.

> +       } else if(config->program_binary != NULL) {
> +               size_t length = strlen((char*)config->program_binary);
> +
> +               if(!config->expect_build_fail) {
> +                       env->program = piglit_cl_build_program_with_binary(env->context,
> +                                                                         &length,
> +                                                                         &config->program_binary,
> +                                                                         build_options);
> +               } else {
> +                       env->program = piglit_cl_fail_build_program_with_binary(env->context,
> +                                                                              &length,
> +                                                                              &config->program_binary,
> +                                                                              build_options);
> +               }
> +       } else if(config->program_binary_file != NULL) {
> +               unsigned int length;
> +               size_t* lengths = malloc(sizeof(size_t) * env->context->num_devices);
> +               unsigned char** program_binaries = malloc(sizeof(unsigned char**) * env->context->num_devices);
> +
> +               ((char**)program_binaries)[0] =
> +                       piglit_load_text_file(config->program_binary_file, &length);
> +               lengths[0] = length;
> +               for(i = 1; i < env->context->num_devices; i++) {
> +                       lengths[i] = lengths[0];
> +                       program_binaries[i] = program_binaries[0];
> +               }
> +
> +               if(((char**)program_binaries)[0] != NULL && length > 0) {
> +                       if(!config->expect_build_fail) {
> +                               env->program = piglit_cl_build_program_with_binary(env->context,
> +                                                                                 lengths,
> +                                                                                 program_binaries,
> +                                                                                 build_options);
> +                       } else {
> +                               env->program = piglit_cl_fail_build_program_with_binary(env->context,
> +                                                                                      lengths,
> +                                                                                      program_binaries,
> +                                                                                      build_options);
> +                       }
> +               } else {
> +                       fprintf(stderr, "Program binary file %s does not exists or is empty\n",
> +                               config->program_source_file);
> +                       return PIGLIT_WARN;
> +               }
> +
> +               free(program_binaries[0]);
> +               free(program_binaries);
> +               free(lengths);
> +       }
> +
> +       if(env->program == NULL) {
> +               return PIGLIT_FAIL;
> +       }
> +
> +       /* Create kernel(s) */
> +       if(config->kernel_name != NULL) {
> +               env->kernel = piglit_cl_create_kernel(env->program, config->kernel_name);
> +
> +               if(env->kernel == NULL) {
> +                       return PIGLIT_FAIL;
> +               }
> +       }
> +
> +       /* Run the actual test */
> +       result = config->_program_test(argc, argv, config, env);
> +
> +
> +       /* Release kernel(s) */
> +       if(env->kernel != NULL) {
> +               clReleaseKernel(env->kernel);
> +       }
> +
> +       /* Release program */
> +       clReleaseProgram(env->program);
> +
> +       return result;
> +}
> +
>  /* Run by piglit_cl_framework_run() */
>  enum piglit_result
>  piglit_cl_program_test_run(const int argc,
> @@ -128,7 +253,7 @@ piglit_cl_program_test_run(const int argc,
>                             cl_platform_id platform_id,
>                             cl_device_id device_id)
>  {
> -       enum piglit_result result;
> +       enum piglit_result result = PIGLIT_PASS;
>
>         struct piglit_cl_program_test_config* config = void_config;
>         struct piglit_cl_program_test_env env = {
> @@ -143,6 +268,9 @@ piglit_cl_program_test_run(const int argc,
>                 .program = NULL,
>
>                 .kernel = NULL,
> +
> +               .gentype_size = 0,
> +               .gentype_elements = 0
>         };
>
>         int i;
> @@ -231,106 +359,55 @@ piglit_cl_program_test_run(const int argc,
>                 }
>         }
>
> -       printf("#   Build options: %s\n", build_options);
> -
> -       /* Create and build program */
> -       if(config->program_source != NULL) {
> -               if(!config->expect_build_fail) {
> -                       env.program = piglit_cl_build_program_with_source(env.context,
> -                                                                         1,
> -                                                                         &config->program_source,
> -                                                                         build_options);
> -               } else {
> -                       env.program = piglit_cl_fail_build_program_with_source(env.context,
> -                                                                              1,
> -                                                                              &config->program_source,
> -                                                                              build_options);
> -               }
> -       } else if(config->program_source_file != NULL) {
> -               unsigned int size;
> -               char* program_source;
> -
> -               program_source = piglit_load_text_file(config->program_source_file, &size);
> -               if(program_source != NULL && size > 0) {
> -                       if(!config->expect_build_fail) {
> -                               env.program = piglit_cl_build_program_with_source(env.context,
> -                                                                                 1,
> -                                                                                 &program_source,
> -                                                                                 build_options);
> -                       } else {
> -                               env.program = piglit_cl_fail_build_program_with_source(env.context,
> -                                                                                      1,
> -                                                                                      &program_source,
> -                                                                                      build_options);
> +       if (!config->gentype) {
> +               result = build_and_execute_program(argc, argv, &env, config, build_options);
> +       } else {
> +               char define[50];
> +               const char *type_str;
> +               const char *len_str;
> +               unsigned type_i, len_i;
> +               for (type_i = GENTYPE_START; type_i <= GENTYPE_END; ++type_i) {
> +                       unsigned gentype_size, gentype_elements;
> +                       unsigned type_bit = (1 << type_i);
> +                       if (!(config->gentype & type_bit)) {
> +                               continue;
>                         }
> -               } else {
> -                       fprintf(stderr, "Program source file %s does not exists or is empty\n",
> -                               config->program_source_file);
> -                       return PIGLIT_WARN;
> -               }
> -               free(program_source);
> -       } else if(config->program_binary != NULL) {
> -               size_t length = strlen((char*)config->program_binary);
> -
> -               if(!config->expect_build_fail) {
> -                       env.program = piglit_cl_build_program_with_binary(env.context,
> -                                                                         &length,
> -                                                                         &config->program_binary,
> -                                                                         build_options);
> -               } else {
> -                       env.program = piglit_cl_fail_build_program_with_binary(env.context,
> -                                                                              &length,
> -                                                                              &config->program_binary,
> -                                                                              build_options);
> -               }
> -       } else if(config->program_binary_file != NULL) {
> -               unsigned int length;
> -               size_t* lengths = malloc(sizeof(size_t) * env.context->num_devices);
> -               unsigned char** program_binaries = malloc(sizeof(unsigned char**) * env.context->num_devices);
> -
> -               ((char**)program_binaries)[0] =
> -                       piglit_load_text_file(config->program_binary_file, &length);
> -               lengths[0] = length;
> -               for(i = 1; i < env.context->num_devices; i++) {
> -                       lengths[i] = lengths[0];
> -                       program_binaries[i] = program_binaries[0];
> -               }
> -
> -               if(((char**)program_binaries)[0] != NULL && length > 0) {
> -                       if(!config->expect_build_fail) {
> -                               env.program = piglit_cl_build_program_with_binary(env.context,
> -                                                                                 lengths,
> -                                                                                 program_binaries,
> -                                                                                 build_options);
> -                       } else {
> -                               env.program = piglit_cl_fail_build_program_with_binary(env.context,
> -                                                                                      lengths,
> -                                                                                      program_binaries,
> -                                                                                      build_options);
> +                       switch(type_bit) {
> +                       case GENTYPE_CHAR: type_str = "char"; gentype_size = 1; break;
> +                       case GENTYPE_UCHAR: type_str = "uchar"; gentype_size = 1; break;
> +                       case GENTYPE_SHORT: type_str = "short"; gentype_size = 2; break;
> +                       case GENTYPE_USHORT: type_str = "ushort"; gentype_size = 2; break;
> +                       case GENTYPE_INT: type_str = "int"; gentype_size = 4; break;
> +                       case GENTYPE_UINT: type_str = "uint"; gentype_size = 4; break;
> +                       case GENTYPE_LONG: type_str = "long"; gentype_size = 8; break;
> +                       case GENTYPE_ULONG: type_str = "ulong"; gentype_size = 8; break;
> +                       case GENTYPE_FLOAT: type_str = "float"; gentype_size = 4; break;
> +                       case GENTYPE_DOUBLE: type_str = "double"; gentype_size = 8; break;

I know that it's an optional feature for now, but eventually we'll
need to add GENTYPE_HALF as well.  I'm also a little sad that we can't
make use of the piglit_cl_type_get_size helper here due to different
definitions of GENTYPE_* and PIGLIT_CL_TYPE_*

> +                       default: assert(!"Unkown type."); break;

s/Unkown/Unknown/

> +                       }
> +                       for (len_i = GENLEN_START; len_i <= GENLEN_END; ++len_i) {
> +                               unsigned len_bit = (1 << len_i);
> +                               char *new_build_options;
> +                               if (!(config->gentype & len_bit)) {
> +                                       continue;
> +                               }
> +                               switch(len_bit) {
> +                               default: len_str = ""; gentype_elements = 1; break;
> +                               case GENLEN_2: len_str = "2"; gentype_elements = 2; break;
> +                               case GENLEN_3: len_str = "3"; gentype_elements = 3; break;
> +                               case GENLEN_4: len_str = "4"; gentype_elements = 4; break;
> +                               case GENLEN_8: len_str = "8"; gentype_elements = 8; break;
> +                               case GENLEN_16: len_str = "16"; gentype_elements = 16; break;
> +                               }
> +                               sprintf(define, " -D_PIGLIT_GENTYPE=%s%s",
> +                                                       type_str, len_str);
> +                               new_build_options = malloc(sizeof(define) + strlen(build_options) + 1);
> +                               sprintf(new_build_options, "%s%s", build_options, define);
> +                               env.gentype_size = gentype_size;
> +                               env.gentype_elements = gentype_elements;
> +                               piglit_merge_result(&result, build_and_execute_program(argc, argv, &env, config, new_build_options));
> +                               free(new_build_options);
>                         }
> -               } else {
> -                       fprintf(stderr, "Program binary file %s does not exists or is empty\n",
> -                               config->program_source_file);
> -                       return PIGLIT_WARN;
> -               }
> -
> -               free(program_binaries[0]);
> -               free(program_binaries);
> -               free(lengths);
> -       }
> -
> -       free(build_options);
> -
> -       if(env.program == NULL) {
> -               return PIGLIT_FAIL;
> -       }
> -
> -       /* Create kernel(s) */
> -       if(config->kernel_name != NULL) {
> -               env.kernel = piglit_cl_create_kernel(env.program, config->kernel_name);
> -
> -               if(env.kernel == NULL) {
> -                       return PIGLIT_FAIL;
>                 }
>         }
>
> @@ -339,19 +416,6 @@ piglit_cl_program_test_run(const int argc,
>                 free(device_ids);
>         }
>
> -
> -       /* Run the actual test */
> -       result = config->_program_test(argc, argv, config, &env);
> -
> -
> -       /* Release kernel(s) */
> -       if(env.kernel != NULL) {
> -               clReleaseKernel(env.kernel);
> -       }
> -
> -       /* Release program */
> -       clReleaseProgram(env.program);
> -
>         /* Release context */
>         piglit_cl_release_context(env.context);
>
> diff --git a/tests/util/piglit-framework-cl-program.h b/tests/util/piglit-framework-cl-program.h
> index bf15d3d..3131604 100644
> --- a/tests/util/piglit-framework-cl-program.h
> +++ b/tests/util/piglit-framework-cl-program.h
> @@ -27,6 +27,43 @@
>
>  #include "piglit-framework-cl.h"
>
> +#define GENTYPE_START  0
> +#define GENTYPE_END    9
> +#define GENLEN_START  10
> +#define GENLEN_END    15
> +
> +#define GENTYPE_CHAR   (1 << 0)
> +#define GENTYPE_UCHAR  (1 << 1)
> +#define GENTYPE_SHORT  (1 << 2)
> +#define GENTYPE_USHORT (1 << 3)
> +#define GENTYPE_INT    (1 << 4)
> +#define GENTYPE_UINT   (1 << 5)
> +#define GENTYPE_LONG   (1 << 6)
> +#define GENTYPE_ULONG  (1 << 7)
> +#define GENTYPE_FLOAT  (1 << 8)
> +#define GENTYPE_DOUBLE (1 << 9)
> +#define GENLEN_1      (1 << 10)
> +#define GENLEN_2      (1 << 11)
> +#define GENLEN_3      (1 << 12)
> +#define GENLEN_4      (1 << 13)
> +#define GENLEN_8      (1 << 14)
> +#define GENLEN_16     (1 << 15)
> +
> +#define GENTYPE_SIGNED_INTEGER \
> +  (GENTYPE_CHAR | GENTYPE_SHORT | GENTYPE_INT | GENTYPE_LONG)
> +#define GENTYPE_UNSIGNED_INTEGER \
> +  (GENTYPE_UCHAR | GENTYPE_USHORT | GENTYPE_UINT | GENTYPE_ULONG)
> +#define GENTYPE_INTEGER \
> +  (GENTYPE_SIGNED_INTEGER | GENTYPE_UNSIGNED_INTEGER)
> +
> +#define GENLEN_SCALAR \
> +  (GENLEN_1)
> +#define GENLEN_VECTOR \
> +  (GENLEN_2 | GENLEN_3 | GENLEN_4 | GENLEN_8 | GENLEN_16)
> +#define GENTYPE \
> +  (GENTYPE_INTEGER | GENTYPE_FLOAT | GENLEN_SCALAR)
> +#define GENTYPEN \
> +  (GENTYPE | GENLEN_VECTOR)
>
>  typedef const struct piglit_cl_program_test_config
>                       piglit_cl_program_test_config_t;
> @@ -87,6 +124,9 @@ PIGLIT_CL_DEFINE_TEST_CONFIG_BEGIN(struct piglit_cl_program_test_config)
>         char* kernel_name; /**< Create kernel(s) for program.
>                                 Conflicts with both \c expect_build_fail==TRUE and
>                                 \c build_only==TRUE. (optional) */
> +       uint32_t gentype; /**< Bitfield of types to test.  For each bit set the
> +                               program will be compiled with
> +                               -D_PIGLIT_GENTYPE=type and exucted. */

s/exucted/executed/

Also, the indentation looks a bit off.

The series is generally looking good.  I've made comments on a few of
the other patches, and I believe that I had an outstanding
comment/question on patch 6 from a week and a half back as well.

--Aaron

>
>  PIGLIT_CL_DEFINE_TEST_CONFIG_END
>
> @@ -153,6 +193,9 @@ struct piglit_cl_program_test_env {
>                                Holds valid kernel if \c kernel_name is a
>                                NULL-terminated string, \c run_per_device is \c true,
>                                and \c expect_build_fail is \c false.*/
> +
> +       unsigned gentype_size;
> +       unsigned gentype_elements;
>  };
>
>
> diff --git a/tests/util/piglit-framework-cl.c b/tests/util/piglit-framework-cl.c
> index 241dd42..2e887ce 100644
> --- a/tests/util/piglit-framework-cl.c
> +++ b/tests/util/piglit-framework-cl.c
> @@ -86,6 +86,12 @@ bool check_platform_extensions(cl_platform_id platform_id, char* extensions)
>  {
>         char* pch;
>
> +       /* Initial calling strtok with a NULL argument may have undefined results if
> +        * strtok() was previously used by this program.
> +        */
> +       if (!extensions) {
> +               return true;
> +       }
>         pch = strtok(extensions, " ");
>         while(pch != NULL) {
>                 if(   strlen(pch) > 0
> @@ -108,6 +114,12 @@ bool check_device_extensions(cl_device_id device_id, char* extensions)
>  {
>         char* pch;
>
> +       /* Initial calling strtok with a NULL argument may have undefined results if
> +        * strtok() was previously used by this program.
> +        */
> +       if (!extensions) {
> +               return true;
> +       }
>         pch = strtok(extensions, " ");
>         while(pch != NULL) {
>                 if(   strlen(pch) > 0
> diff --git a/tests/util/piglit-util-cl.h b/tests/util/piglit-util-cl.h
> index b392359..78d3229 100644
> --- a/tests/util/piglit-util-cl.h
> +++ b/tests/util/piglit-util-cl.h
> @@ -56,7 +56,8 @@ enum piglit_cl_type {
>         PIGLIT_CL_TYPE_LONG,
>         PIGLIT_CL_TYPE_ULONG,
>         PIGLIT_CL_TYPE_FLOAT,
> -       PIGLIT_CL_TYPE_DOUBLE
> +       PIGLIT_CL_TYPE_DOUBLE,
> +       PIGLIT_CL_TYPE_GENTYPE
>  };
>
>  /**
> --
> 1.7.11.4
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit


More information about the Piglit mailing list