Switch log2 #2
Switch log2 #2
Conversation
(cherry picked from commit 9e44c67)
|
@mlemerre this should be ready to merge following your fix of
|
README.md
Outdated
| - Supports generic maps and sets: a `'m map` that maps `'k key` to `('k, 'm) value`. | ||
| This is especially useful when using [GADTs](https://v2.ocaml.org/manual/gadts-tutorial.html) for the type of keys. This is also sometimes called a dependent map. | ||
| - Allows easy and fast operations across different types of maps and set (e.g. | ||
| an intersection between a map and a set), since all sets and maps, no matter their key type, are really positive integer sets or maps. | ||
| an intersection between a map and a set), since all sets and maps, no matter their key type, are really integer sets or maps. |
There was a problem hiding this comment.
I am not sure what this means (even before the change)
There was a problem hiding this comment.
Yes that wasn't very clear. I rephrased it.
| external highest_bit: int -> (int[@untagged]) = | ||
| "caml_int_builtin_highest_bit_byte" "caml_int_builtin_highest_bit" [@@noalloc] | ||
|
|
||
| let unsigned_lt x y = x - min_int < y - min_int |
There was a problem hiding this comment.
Did you test this one? It is not obvious to me (and it is not the fix of Midtgaard's paper)
There was a problem hiding this comment.
I'll add tests.
I got the idea from NativeInt.unsigned compare in OCaml's standard library.
The fix from Midtgaard's paper only works if we know the arguments are powers of two. This will work for all integer arguments.
Testing in compiler explorer shows OCaml fails to optimize this to an unsigned compare, however using an external C function probably won't improve performance since it prevents inlining.
|
Except for the few commands, sounds good! I don't think the fix would introduces a noticeable slow down (but I am not sure the OCaml compiler can detect that this is just an unsigned comparison. Ideally we would have a primitive for this...). The names are not short but in the spirit of the library! |
Todo: check how this works with negative numbers