Re: [PATCH 7/7] d3dx9/tests: Use static test blob in test_effect_preshader_ops().
2017-05-19 13:56 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: Thanks, I do think this is an improvement compared to the old tests. A few comments below.
static const struct { - unsigned int pos; + const char *mnem; + unsigned int expected_result[4]; unsigned int result_index; - unsigned int ins_count; + float *result; + D3DXVECTOR4 opvect1, opvect2, opvect3; + unsigned int ulps; + BOOL todo[4]; }
I assume ulps and todo are there for future tests?
+ for (j = 0; j < 4; ++j) + { + const float *v = op_tests[i].result;
Can you put the initialization of 'v' outside of the inner loop? I'm sure the compiler sees the optimization and the generated code is ultimately the same (not that it would really matter for a test anyway...) but it seems a bit nicer to do that in the "correct" block. You could also call it "result" or similar.
+ todo_wine_if(op_tests[i].todo[j]) + ok(compare_float(v[j], ((float *)op_tests[i].expected_result)[j], op_tests[i].ulps), + "Operation %s, component %u, expected %#x (%g), got %#x (%g).\n", op_tests[i].mnem, + j, op_tests[i].expected_result[j], ((float *)op_tests[i].expected_result)[j], + ((unsigned int *)v)[j], v[j]);
Just an idea, maybe use %.8e to print the float values? Mostly for consistency with the usual d3d stuff, although in this case it doesn't matter much since you're also printing the bit encoding in hex. Also you can clean the ok() a bit by defining a "const float *expected_float;" variable (similarly to test_effect_preshader_relative_addressing()). BTW, notice that you're currently casting away const from v.
On 05/22/2017 02:28 PM, Matteo Bruni wrote:
2017-05-19 13:56 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:
Thanks, I do think this is an improvement compared to the old tests. A few comments below.
static const struct { - unsigned int pos; + const char *mnem; + unsigned int expected_result[4]; unsigned int result_index; - unsigned int ins_count; + float *result; + D3DXVECTOR4 opvect1, opvect2, opvect3; + unsigned int ulps; + BOOL todo[4]; }
I assume ulps and todo are there for future tests? Yes, they are not used right now. I left them in place because they already were in the structure and I thought it would be a bit strange to remove and add them now and then.
+ for (j = 0; j < 4; ++j) + { + const float *v = op_tests[i].result; Can you put the initialization of 'v' outside of the inner loop? I'm sure the compiler sees the optimization and the generated code is ultimately the same (not that it would really matter for a test anyway...) but it seems a bit nicer to do that in the "correct" block. You could also call it "result" or similar. Yes, sure.
+ todo_wine_if(op_tests[i].todo[j]) + ok(compare_float(v[j], ((float *)op_tests[i].expected_result)[j], op_tests[i].ulps), + "Operation %s, component %u, expected %#x (%g), got %#x (%g).\n", op_tests[i].mnem, + j, op_tests[i].expected_result[j], ((float *)op_tests[i].expected_result)[j], + ((unsigned int *)v)[j], v[j]); Just an idea, maybe use %.8e to print the float values? Mostly for consistency with the usual d3d stuff, although in this case it doesn't matter much since you're also printing the bit encoding in hex. I thought "%g" shows the values in a tiny bit more convenient way for viewing them (and as you pointed we have hex output for, e. g., copying the exact value from this output into the table). I actually found %g form of output handy when viewing these tests output. I have no problems changing it to ".8e" if you consider this better, but currently we are using "%g" throughout all the preshader tests, in this case I should probably do it throughout all the tests?
Also you can clean the ok() a bit by defining a "const float *expected_float;" variable (similarly to test_effect_preshader_relative_addressing()). BTW, notice that you're currently casting away const from v.
I will do that.
2017-05-22 13:48 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:
On 05/22/2017 02:28 PM, Matteo Bruni wrote:
Just an idea, maybe use %.8e to print the float values? Mostly for consistency with the usual d3d stuff, although in this case it doesn't matter much since you're also printing the bit encoding in hex.
I thought "%g" shows the values in a tiny bit more convenient way for viewing them (and as you pointed we have hex output for, e. g., copying the exact value from this output into the table). I actually found %g form of output handy when viewing these tests output. I have no problems changing it to ".8e" if you consider this better, but currently we are using "%g" throughout all the preshader tests, in this case I should probably do it throughout all the tests?
Yes, ideally all the strings would need to be changed, although on the other hand a patch just updating the ok() strings isn't particularly appealing and mine is a very mild suggestion to begin with. It's up to you, really.
participants (2)
-
Matteo Bruni -
Paul Gofman