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

Julien Fischer jfischer at opturion.com
Fri Sep 1 11:41:46 AEST 2017


On Fri, 1 Sep 2017, Zoltan Somogyi wrote:

>
>
> 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).

...

> 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.

It is.  Fixed.

> 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.

It's fine, the C version implicitly promotes c2 to an unsigned value
anyway.  And according to the original webpage:

     If the source were to be translated to C, then the Java 'int' data type
     should be replaced with C 'uint32_t' data type.

> 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.

Ok; what I might do is shift Ralph's original implementation into that
test case then.  (And indeed add the original Java versions).

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

Fixed.

Thanks for that.

Julien.


More information about the reviews mailing list