[Piglit] [PATCH 1/7] cl-program-tester: Use more appropriate member names for struct test_arg
Aaron Watry
awatry at gmail.com
Tue Oct 1 15:03:16 PDT 2013
The main downside to this plan that I see is that combining the tests
for all data types into the same program run leads to something like
the following:
Using the current vload-int.cl as an example:
Currently my pitcairn fails to run the test at all due to issues with
the vload3, vload8, and vload16 tests. If I remove the int3/8/16
tests, everything passes. Currently, we abort the entire test program
if one of the data sizes triggers an LLVM-based crash. So instead of
seeing int2/int4 working and the rest failing, I just see a crash when
it first tries to compile the int3 kernel (which happens when it tries
to execute the first int2 test).
If we split each data type and vector size into their own files or
test processes, we lose a bit/lot on piglit run speeds, but we would
get full results from the test run. I.e. if int3 fails but everything
else is valid, I'd love to just see crashes for the int3 tests, but
passes for all of the other tests, instead of a truncated test run.
It doesn't sound like you're headed in that direction, but it's also
the same result that we currently have, so it's not really a
regression in functionality either. I do like that we'll get
additional test coverage and simplified test writing out of these
changes. If we wanted to split each gentype into its own separate
process, that can probably be done at a future time.
--Aaron
On Tue, Oct 1, 2013 at 9:24 AM, Tom Stellard <tom at stellard.net> wrote:
> On Mon, Sep 30, 2013 at 05:09:46PM -0500, Aaron Watry wrote:
>> I hope to have time to dig into the actual changes themselves in more
>> detail (my first skim through looked fairly good besides my comment on
>> patch 6). If I don't get time to do a full review of the changes, I
>> just wanted to mention that I tested this on my CEDAR without any
>> regressions to existing tests... although the new kernel_args test
>> doesn't pass (but maybe that's expected).
>>
>
> Thanks for testing. The kernel_args test fails for me too. I'm still
> working on fix for it.
>
> My long term plan with the gentype feature is to make all the python
> generated tests use gentype. This will give us more test cases for
> the gentype feature and will hopefully reduce piglit run times by reducing
> the number of test files.
>
> -Tom
>
>>
>> On Mon, Sep 30, 2013 at 9:47 AM, Tom Stellard <tom at stellard.net> wrote:
>> > From: Tom Stellard <thomas.stellard at amd.com>
>> >
>> > cl_size has been renamed to vec_elements and cl_mem_size has been
>> > renamed to vec_mem_elements. This use of size is confusing, because
>> > size generally means number of bytes when used to describe a type, and
>> > in this case the value being stored was the number of elements in the
>> > vector.
>> > ---
>> > tests/cl/program/program-tester.c | 48 +++++++++++++++++++--------------------
>> > 1 file changed, 24 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/tests/cl/program/program-tester.c b/tests/cl/program/program-tester.c
>> > index be6bc20..1524db0 100644
>> > --- a/tests/cl/program/program-tester.c
>> > +++ b/tests/cl/program/program-tester.c
>> > @@ -216,8 +216,8 @@ struct test_arg {
>> > enum test_arg_type type;
>> >
>> > enum cl_type cl_type;
>> > - size_t cl_size; // 1 for int, 3 for int3
>> > - size_t cl_mem_size; // 1 for int, 4 for int3
>> > + size_t vec_elements; // 1 for int, 3 for int3
>> > + size_t vec_mem_elements; // 1 for int, 4 for int3
>> > size_t length; // for buffers
>> >
>> > /* kernel arg data */
>> > @@ -237,8 +237,8 @@ struct test_arg create_test_arg()
>> > .type = TEST_ARG_VALUE,
>> >
>> > .cl_type = TYPE_CHAR,
>> > - .cl_size = 1,
>> > - .cl_mem_size = 1,
>> > + .vec_elements = 1,
>> > + .vec_mem_elements = 1,
>> > .length = 0,
>> >
>> > .index = 0,
>> > @@ -868,9 +868,9 @@ get_test_arg_value(struct test_arg* test_arg, const char* value, size_t length)
>> > case enum_type: \
>> > get_func(value, &array, length); \
>> > for(i = 0; i < test_arg->length; i++) { \
>> > - for(c = 0; c < test_arg->cl_size; c++) { \
>> > - ra = i*test_arg->cl_size + c; \
>> > - rb = i*test_arg->cl_mem_size + c; \
>> > + for(c = 0; c < test_arg->vec_elements; c++) { \
>> > + ra = i*test_arg->vec_elements + c; \
>> > + rb = i*test_arg->vec_mem_elements + c; \
>> > ((cl_type*)test_arg->value)[rb] = array[ra%length]; \
>> > } \
>> > } \
>> > @@ -963,23 +963,23 @@ get_test_arg(const char* src, struct test* test, bool arg_in)
>> > test_arg.index = get_int(index_str);
>> > free(index_str);
>> >
>> > - /* Set type, cl_size, cl_mem_size and size (partially for buffers) */
>> > + /* Set type, vec_elements, vec_mem_elements and size (partially for buffers) */
>> > regex_get_match_str(&type, src, pmatch, 2);
>> > if(regex_match(type, "[[:digit:]]+")) {
>> > char* type_size_str;
>> > regex_get_str(&type_size_str, type, "[[:digit:]]+", 0, REG_NEWLINE);
>> > - test_arg.cl_size = get_int(type_size_str);
>> > - test_arg.cl_mem_size = test_arg.cl_size != 3 ? test_arg.cl_size : 4; // test if we have type3
>> > + test_arg.vec_elements = get_int(type_size_str);
>> > + test_arg.vec_mem_elements = test_arg.vec_elements != 3 ? test_arg.vec_elements : 4; // test if we have type3
>> > free(type_size_str);
>> > } else {
>> > - test_arg.cl_size = 1;
>> > - test_arg.cl_mem_size = 1;
>> > + test_arg.vec_elements = 1;
>> > + test_arg.vec_mem_elements = 1;
>> > }
>> >
>> > #define IF(regex_type, enum_type, main_type) \
>> > if(regex_match(type, REGEX_FULL_MATCH(regex_type))) { \
>> > test_arg.cl_type = enum_type; \
>> > - test_arg.size = sizeof(main_type) * test_arg.cl_mem_size; \
>> > + test_arg.size = sizeof(main_type) * test_arg.vec_mem_elements; \
>> > }
>> > #define ELSEIF(regex_type, enum_type, main_type) \
>> > else IF(regex_type, enum_type, main_type)
>> > @@ -1020,7 +1020,7 @@ get_test_arg(const char* src, struct test* test, bool arg_in)
>> > if(regex_match(value, REGEX_FULL_MATCH(REGEX_NULL))) {
>> > test_arg.value = NULL;
>> > } else {
>> > - get_test_arg_value(&test_arg, value, test_arg.cl_size);
>> > + get_test_arg_value(&test_arg, value, test_arg.vec_elements);
>> > }
>> > free(value);
>> > } else if(regex_match(src, REGEX_FULL_MATCH(REGEX_ARG_BUFFER))) { // buffer
>> > @@ -1084,7 +1084,7 @@ get_test_arg(const char* src, struct test* test, bool arg_in)
>> > } else if(regex_match(value, REGEX_ARRAY)) {
>> > get_test_arg_value(&test_arg,
>> > value,
>> > - test_arg.length * test_arg.cl_size);
>> > + test_arg.length * test_arg.vec_elements);
>> > }
>> > }
>> > free(value);
>> > @@ -1623,12 +1623,12 @@ check_test_arg_value(struct test_arg test_arg,
>> > #define CASEI(enum_type, type, cl_type) \
>> > case enum_type: \
>> > for(i = 0; i < test_arg.length; i++) { \
>> > - for(c = 0; c < test_arg.cl_size; c++) { \
>> > - rb = i*test_arg.cl_mem_size + c; \
>> > + for(c = 0; c < test_arg.vec_elements; c++) { \
>> > + rb = i*test_arg.vec_mem_elements + c; \
>> > if(!piglit_cl_probe_integer(((cl_type*)value)[rb], \
>> > ((cl_type*)test_arg.value)[rb], \
>> > test_arg.toli)) { \
>> > - ra = i*test_arg.cl_size + c; \
>> > + ra = i*test_arg.vec_elements + c; \
>> > printf("Error at %s[%zu]\n", type, ra); \
>> > return false; \
>> > } \
>> > @@ -1638,12 +1638,12 @@ check_test_arg_value(struct test_arg test_arg,
>> > #define CASEU(enum_type, type, cl_type) \
>> > case enum_type: \
>> > for(i = 0; i < test_arg.length; i++) { \
>> > - for(c = 0; c < test_arg.cl_size; c++) { \
>> > - rb = i*test_arg.cl_mem_size + c; \
>> > + for(c = 0; c < test_arg.vec_elements; c++) { \
>> > + rb = i*test_arg.vec_mem_elements + c; \
>> > if(!piglit_cl_probe_uinteger(((cl_type*)value)[rb], \
>> > ((cl_type*)test_arg.value)[rb], \
>> > test_arg.tolu)) { \
>> > - ra = i*test_arg.cl_size + c; \
>> > + ra = i*test_arg.vec_elements + c; \
>> > printf("Error at %s[%zu]\n", type, ra); \
>> > return false; \
>> > } \
>> > @@ -1653,12 +1653,12 @@ check_test_arg_value(struct test_arg test_arg,
>> > #define CASEF(enum_type, type, cl_type) \
>> > case enum_type: \
>> > for(i = 0; i < test_arg.length; i++) { \
>> > - for(c = 0; c < test_arg.cl_size; c++) { \
>> > - rb = i*test_arg.cl_mem_size + c; \
>> > + for(c = 0; c < test_arg.vec_elements; c++) { \
>> > + rb = i*test_arg.vec_mem_elements + c; \
>> > if(!piglit_cl_probe_floating(((cl_type*)value)[rb], \
>> > ((cl_type*)test_arg.value)[rb], \
>> > test_arg.tolf)) { \
>> > - ra = i*test_arg.cl_size + c; \
>> > + ra = i*test_arg.vec_elements + c; \
>> > printf("Error at %s[%zu]\n", type, ra); \
>> > return false; \
>> > } \
>> > --
>> > 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