[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