Patchwork [3/3] tests: Add unit tests for mulu64 and muls64

login
register
mail settings
Submitter Richard Henderson
Date Jan. 28, 2013, 10:09 p.m.
Message ID <1359410943-20057-4-git-send-email-rth@twiddle.net>
Download mbox | patch
Permalink /patch/216384/
State New
Headers show

Comments

Richard Henderson - Jan. 28, 2013, 10:09 p.m.
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
Blue Swirl - Jan. 29, 2013, 8:06 p.m.
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
>
Richard Henderson - Jan. 29, 2013, 8:52 p.m.
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~
Blue Swirl - Feb. 2, 2013, 12:17 p.m.
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~

Patch

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();
+}