| Message ID | 1359410943-20057-4-git-send-email-rth@twiddle.net |
|---|---|
| State | New |
| Headers | show |
On Mon, Jan 28, 2013 at 10:09 PM, Richard Henderson <rth@twiddle.net> wrote: > Signed-off-by: Richard Henderson <rth@twiddle.net> > --- > tests/Makefile | 7 +++++- > tests/test-mul64.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 75 insertions(+), 1 deletion(-) > create mode 100644 tests/test-mul64.c > > diff --git a/tests/Makefile b/tests/Makefile > index 442b286..e695ef6 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -45,6 +45,8 @@ gcov-files-test-aio-$(CONFIG_WIN32) = aio-win32.c > gcov-files-test-aio-$(CONFIG_POSIX) = aio-posix.c > check-unit-y += tests/test-thread-pool$(EXESUF) > gcov-files-test-thread-pool-y = thread-pool.c > +check-unit-y += tests/test-mul64$(EXESUF) > +gcov-files-test-mul64-y = util/host-utils.c > > check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh > > @@ -72,7 +74,8 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \ > tests/test-coroutine.o tests/test-string-output-visitor.o \ > tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \ > tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \ > - tests/test-qmp-commands.o tests/test-visitor-serialization.o > + tests/test-qmp-commands.o tests/test-visitor-serialization.o \ > + tests/test-mul64.o > > test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o > > @@ -108,6 +111,8 @@ tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi- > tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) qapi-types.o qapi-visit.o libqemuutil.a libqemustub.a > tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a > > +tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a > + > tests/rtc-test$(EXESUF): tests/rtc-test.o > tests/m48t59-test$(EXESUF): tests/m48t59-test.o > tests/fdc-test$(EXESUF): tests/fdc-test.o > diff --git a/tests/test-mul64.c b/tests/test-mul64.c > new file mode 100644 > index 0000000..6abf57c > --- /dev/null > +++ b/tests/test-mul64.c > @@ -0,0 +1,69 @@ > +/* > + * Test 64x64 -> 128 multiply subroutines > + * > + * This work is licensed under the terms of the GNU LGPL, version 2 or later. > + * See the COPYING.LIB file in the top-level directory. > + * > + */ > + > +#include <glib.h> > +#include <stdint.h> > +#include "qemu/host-utils.h" > +#include "qemu/osdep.h" > + > + > +typedef struct { > + uint64_t a, b; > + uint64_t rh, rl; > +} Test; > + > +static const Test test_u_data[] = { > + { 1, 1, 0, 1 }, > + { 10000, 10000, 0, 100000000 }, > + { -1ull, 2, 1, -2ull }, Shouldn't '1' be '-1'? How can this test pass? It could be clearer to use 'ull' or 'ULL' (stands out better) always. > + { -1ull, -1ull, -2ull, 1 }, This looks buggy too. > + { 0x1122334455667788ull, 0x8877665544332211ull, > + 0x092228FB777AE38Full, 0x0A3E963337C60008ull }, > +}; > + > +static const Test test_s_data[] = { > + { 1, 1, 0, 1 }, > + { 1, -1, -1, -1 }, Negative numbers should probably have ll/LL or ull/ULL. > + { -10, -10, 0, 100 }, > + { 10000, 10000, 0, 100000000 }, > + { -1, 2, -1, -2 }, > + { 0x1122334455667788ll, 0x1122334455667788ull, Spurious 'll', also below. > + 0x01258F60BBC2975Cll, 0x1EACE4A3C82FB840ull }, > +}; > + > +static void test_u(void) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) { > + uint64_t rl, rh; > + mulu64(&rl, &rh, test_u_data[i].a, test_u_data[i].b); > + g_assert_cmpuint(rl, ==, test_u_data[i].rl); This could explain why the test passes: g_assert_cmpuint() uses unsigned ints so there is truncation from uint64_t. > + g_assert_cmpuint(rh, ==, test_u_data[i].rh); > + } > +} > + > +static void test_s(void) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) { test_s_data > + uint64_t rl, rh; > + muls64(&rl, &rh, test_s_data[i].a, test_s_data[i].b); > + g_assert_cmpuint(rl, ==, test_s_data[i].rl); > + g_assert_cmpint(rh, ==, test_s_data[i].rh); > + } > +} > + > +int main(int argc, char **argv) > +{ > + g_test_init(&argc, &argv, NULL); > + g_test_add_func("/host-utils/mulu64", test_u); > + g_test_add_func("/host-utils/muls64", test_s); > + return g_test_run(); > +} > -- > 1.7.11.7 >
On 01/29/2013 12:06 PM, Blue Swirl wrote: >> +static const Test test_u_data[] = { >> + { 1, 1, 0, 1 }, >> + { 10000, 10000, 0, 100000000 }, >> + { -1ull, 2, 1, -2ull }, > > Shouldn't '1' be '-1'? How can this test pass? This is 0xffff_ffff_ffff_ffff * 2 = 0x1_ffff_ffff_ffff_fffe with the knowledge that -1 = 0xf...f and -2 = 0xf...e. >> + { -1ull, -1ull, -2ull, 1 }, > > This looks buggy too. See above. >> + { -10, -10, 0, 100 }, >> + { 10000, 10000, 0, 100000000 }, >> + { -1, 2, -1, -2 }, >> + { 0x1122334455667788ll, 0x1122334455667788ull, > > Spurious 'll', also below. Why spurious? >> +static void test_u(void) >> +{ >> + int i; >> + >> + for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) { >> + uint64_t rl, rh; >> + mulu64(&rl, &rh, test_u_data[i].a, test_u_data[i].b); >> + g_assert_cmpuint(rl, ==, test_u_data[i].rl); > > This could explain why the test passes: g_assert_cmpuint() uses > unsigned ints so there is truncation from uint64_t. Does it? It sure doesn't look like it: > #define g_assert_cmpuint(n1, cmp, n2) do { guint64 __n1 = (n1), __n2 = (n2); \ > if (__n1 cmp __n2) ; else \ > g_assertion_message_cmpnum (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ > #n1 " " #cmp " " #n2, __n1, #cmp, __n2, 'i'); } while (0) I see guint64 in there, thus no truncation. >> +static void test_s(void) >> +{ >> + int i; >> + >> + for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) { > > test_s_data Good catch. I wasn't running all of the test_s data points. r~
On Tue, Jan 29, 2013 at 8:52 PM, Richard Henderson <rth@twiddle.net> wrote: > On 01/29/2013 12:06 PM, Blue Swirl wrote: >>> >>> +static const Test test_u_data[] = { >>> + { 1, 1, 0, 1 }, >>> + { 10000, 10000, 0, 100000000 }, >>> + { -1ull, 2, 1, -2ull }, >> >> >> Shouldn't '1' be '-1'? How can this test pass? > > > This is 0xffff_ffff_ffff_ffff * 2 = 0x1_ffff_ffff_ffff_fffe > > with the knowledge that -1 = 0xf...f and -2 = 0xf...e. OK, I somehow used signed multiplication rules. > > >>> + { -1ull, -1ull, -2ull, 1 }, >> >> >> This looks buggy too. > > > See above. > > >>> + { -10, -10, 0, 100 }, >>> + { 10000, 10000, 0, 100000000 }, >>> + { -1, 2, -1, -2 }, >>> + { 0x1122334455667788ll, 0x1122334455667788ull, >> >> >> Spurious 'll', also below. > > > Why spurious? I'd use ULL/ull everywhere for hex constants. > > >>> +static void test_u(void) >>> +{ >>> + int i; >>> + >>> + for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) { >>> + uint64_t rl, rh; >>> + mulu64(&rl, &rh, test_u_data[i].a, test_u_data[i].b); >>> + g_assert_cmpuint(rl, ==, test_u_data[i].rl); >> >> >> This could explain why the test passes: g_assert_cmpuint() uses >> unsigned ints so there is truncation from uint64_t. > > > Does it? It sure doesn't look like it: > >> #define g_assert_cmpuint(n1, cmp, n2) do { guint64 __n1 = (n1), __n2 = >> (n2); \ >> if (__n1 cmp __n2) ; else \ >> g_assertion_message_cmpnum >> (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ >> #n1 " " #cmp " " #n2, >> __n1, #cmp, __n2, 'i'); } while (0) > > > I see guint64 in there, thus no truncation. The manual only talks about unsigned integers, maybe they really mean guint64: http://developer.gnome.org/glib/2.34/glib-Testing.html#g-assert-cmpuint > > >>> +static void test_s(void) >>> +{ >>> + int i; >>> + >>> + for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) { >> >> >> test_s_data > > > Good catch. I wasn't running all of the test_s data points. > > > r~
diff --git a/tests/Makefile b/tests/Makefile index 442b286..e695ef6 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -45,6 +45,8 @@ gcov-files-test-aio-$(CONFIG_WIN32) = aio-win32.c gcov-files-test-aio-$(CONFIG_POSIX) = aio-posix.c check-unit-y += tests/test-thread-pool$(EXESUF) gcov-files-test-thread-pool-y = thread-pool.c +check-unit-y += tests/test-mul64$(EXESUF) +gcov-files-test-mul64-y = util/host-utils.c check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh @@ -72,7 +74,8 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \ tests/test-coroutine.o tests/test-string-output-visitor.o \ tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \ tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \ - tests/test-qmp-commands.o tests/test-visitor-serialization.o + tests/test-qmp-commands.o tests/test-visitor-serialization.o \ + tests/test-mul64.o test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o @@ -108,6 +111,8 @@ tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi- tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) qapi-types.o qapi-visit.o libqemuutil.a libqemustub.a tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a +tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a + tests/rtc-test$(EXESUF): tests/rtc-test.o tests/m48t59-test$(EXESUF): tests/m48t59-test.o tests/fdc-test$(EXESUF): tests/fdc-test.o diff --git a/tests/test-mul64.c b/tests/test-mul64.c new file mode 100644 index 0000000..6abf57c --- /dev/null +++ b/tests/test-mul64.c @@ -0,0 +1,69 @@ +/* + * Test 64x64 -> 128 multiply subroutines + * + * This work is licensed under the terms of the GNU LGPL, version 2 or later. + * See the COPYING.LIB file in the top-level directory. + * + */ + +#include <glib.h> +#include <stdint.h> +#include "qemu/host-utils.h" +#include "qemu/osdep.h" + + +typedef struct { + uint64_t a, b; + uint64_t rh, rl; +} Test; + +static const Test test_u_data[] = { + { 1, 1, 0, 1 }, + { 10000, 10000, 0, 100000000 }, + { -1ull, 2, 1, -2ull }, + { -1ull, -1ull, -2ull, 1 }, + { 0x1122334455667788ull, 0x8877665544332211ull, + 0x092228FB777AE38Full, 0x0A3E963337C60008ull }, +}; + +static const Test test_s_data[] = { + { 1, 1, 0, 1 }, + { 1, -1, -1, -1 }, + { -10, -10, 0, 100 }, + { 10000, 10000, 0, 100000000 }, + { -1, 2, -1, -2 }, + { 0x1122334455667788ll, 0x1122334455667788ull, + 0x01258F60BBC2975Cll, 0x1EACE4A3C82FB840ull }, +}; + +static void test_u(void) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) { + uint64_t rl, rh; + mulu64(&rl, &rh, test_u_data[i].a, test_u_data[i].b); + g_assert_cmpuint(rl, ==, test_u_data[i].rl); + g_assert_cmpuint(rh, ==, test_u_data[i].rh); + } +} + +static void test_s(void) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) { + uint64_t rl, rh; + muls64(&rl, &rh, test_s_data[i].a, test_s_data[i].b); + g_assert_cmpuint(rl, ==, test_s_data[i].rl); + g_assert_cmpint(rh, ==, test_s_data[i].rh); + } +} + +int main(int argc, char **argv) +{ + g_test_init(&argc, &argv, NULL); + g_test_add_func("/host-utils/mulu64", test_u); + g_test_add_func("/host-utils/muls64", test_s); + return g_test_run(); +}
Signed-off-by: Richard Henderson <rth@twiddle.net> --- tests/Makefile | 7 +++++- tests/test-mul64.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 tests/test-mul64.c