Re: [Qemu-devel] [PATCH 3/3] tests: Add unit tests for mulu64 and muls64

[Blue Swirl]

On Mon, Jan 28, 2013 at 10:09 PM, Richard Henderson <address@hidden> wrote:
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  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
>