[m-rev.] for review: reimpelement the hash_pred for ints in Mercury

Zoltan Somogyi zoltan.somogyi at runbox.com
Fri Sep 1 11:10:22 AEST 2017



On Fri, 1 Sep 2017 10:30:21 +1000 (AEST), Julien Fischer <jfischer at opturion.com> wrote:
> -int_hash(N, N).
> -    % For use on non-C backends.
> -    % There are almost certainly better ones out there...
> +int_hash(Key, Hash) :-
> +    UKey = uint.cast_from_int(Key),
> +    uint_hash(UKey, Hash).
> +
> +uint_hash(!.Key, Hash) :-
> +    C2 = 0x_27d4_eb2d_u,
> +    ( if bits_per_uint = 4 then
> +        !:Key = (!.Key `xor` 61_u) `xor` (!.Key >> 16),
> +        !:Key = !.Key + (!.Key << 3),
> +        !:Key = !.Key `xor` (!.Key >> 4),
> +        !:Key = !.Key * C2,
> +        !:Key = !.Key `xor` (!.Key >> 15)
> +    else
> +        !:Key = (\ !.Key) + (!.Key << 21),
> +        !:Key = !.Key `xor` (!.Key >> 24),
> +        !:Key = (!.Key + (!.Key << 3)) + (!.Key << 8),
> +        !:Key = !.Key `xor` (!.Key >> 14),
> +        !:Key = (!.Key + (!.Key << 2)) + (!.Key << 4),
> +        !:Key = !.Key `xor` (!.Key >> 28),
> +        !:Key = !.Key + (!.Key << 31)
> +    ),
> +    Hash = uint.cast_to_int(!.Key).

I see three issues here.

The C code's condition is sizeof(MR_Word) == 4, while the Mercury code's
is BITS_per_uint = 4. That is a bug.

The second is that in the C version, key * c2 has type uint*int, while the
Mercury version is uint*uint (due to the _u suffix on C2). I don't know whether
that is a bug or not.

I think you will want to add a test case to check that the C and Mercury versions
generate the same hash values for a bunch of numbers.

The third is just a style issue; the Mercury version lacks the comments
(such as "key *265") that the C version contains.

Zoltan.


More information about the reviews mailing list