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

[Richard Henderson]

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~