From 385d2a143c017c96f6de24bd8a13b34babf9b295 Mon Sep 17 00:00:00 2001 From: Andrea Piseri Date: Wed, 21 Dec 2022 21:09:22 +0100 Subject: [PATCH 1/3] Fix `core:slice.rotate_left` This commit includes two fixes: - a temporary cast to make the function compile - a fix to a logic error that caused the function to hang or return incorrect results --- core/slice/ptr.odin | 18 +++++------------- core/slice/slice.odin | 8 +++++--- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/core/slice/ptr.odin b/core/slice/ptr.odin index 214b745f7..701e55495 100644 --- a/core/slice/ptr.odin +++ b/core/slice/ptr.odin @@ -73,25 +73,17 @@ ptr_rotate :: proc(left: int, mid: ^$T, right: int) { left, mid, right := left, mid, right // TODO(bill): Optimization with a buffer for smaller ranges - if left >= right { - for { - ptr_swap_non_overlapping(ptr_sub(mid, right), mid, right) + for left > 0 && right > 0 { + if left >= right { + ptr_swap_non_overlapping(ptr_sub(mid, right), mid, right * size_of(T)) mid = ptr_sub(mid, right) left -= right - if left < right { - break - } - } - } else { - for { - ptr_swap_non_overlapping(ptr_sub(mid, left), mid, left) + } else { + ptr_swap_non_overlapping(ptr_sub(mid, left), mid, left * size_of(T)) mid = ptr_add(mid, left) right -= left - if right < left { - break - } } } } diff --git a/core/slice/slice.odin b/core/slice/slice.odin index 032a8ca6e..f5305f29e 100644 --- a/core/slice/slice.odin +++ b/core/slice/slice.odin @@ -218,8 +218,10 @@ rotate_left :: proc(array: $T/[]$E, mid: int) { n := len(array) m := mid %% n k := n - m - p := raw_data(array) - ptr_rotate(mid, ptr_add(p, mid), k) + // FIXME: (ap29600) this cast is a temporary fix for the compiler not matching + // [^T] with $P/^$T + p := cast(^int)raw_data(array) + ptr_rotate(m, ptr_add(p, m), k) } rotate_right :: proc(array: $T/[]$E, k: int) { rotate_left(array, -k) @@ -515,4 +517,4 @@ dot_product :: proc(a, b: $S/[]$T) -> (r: T, ok: bool) enumerated_array :: proc(ptr: ^$T) -> []intrinsics.type_elem_type(T) where intrinsics.type_is_enumerated_array(T) { return ([^]intrinsics.type_elem_type(T))(ptr)[:len(T)] -} \ No newline at end of file +} From 191223bb3c6872f952e97d44852823563b47a744 Mon Sep 17 00:00:00 2001 From: Andrea Piseri Date: Wed, 21 Dec 2022 21:58:01 +0100 Subject: [PATCH 2/3] Fix non-generic cast in core:slice.rotate_left --- core/slice/slice.odin | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/slice/slice.odin b/core/slice/slice.odin index f5305f29e..595ec8e37 100644 --- a/core/slice/slice.odin +++ b/core/slice/slice.odin @@ -220,7 +220,7 @@ rotate_left :: proc(array: $T/[]$E, mid: int) { k := n - m // FIXME: (ap29600) this cast is a temporary fix for the compiler not matching // [^T] with $P/^$T - p := cast(^int)raw_data(array) + p := cast(^E)raw_data(array) ptr_rotate(m, ptr_add(p, m), k) } rotate_right :: proc(array: $T/[]$E, k: int) { From 3fa971a510172be922eeaaee9fd3533dc9628832 Mon Sep 17 00:00:00 2001 From: Andrea Piseri Date: Wed, 21 Dec 2022 22:10:02 +0100 Subject: [PATCH 3/3] Add the inner `for` loop back in the logic This could be easier to predict in cases where one of `left` and `right` is significantly greater than the other, and as such the same branch is taken multiple times in a row --- core/slice/ptr.odin | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/core/slice/ptr.odin b/core/slice/ptr.odin index 701e55495..e2f1c3e7b 100644 --- a/core/slice/ptr.odin +++ b/core/slice/ptr.odin @@ -75,15 +75,25 @@ ptr_rotate :: proc(left: int, mid: ^$T, right: int) { // TODO(bill): Optimization with a buffer for smaller ranges for left > 0 && right > 0 { if left >= right { - ptr_swap_non_overlapping(ptr_sub(mid, right), mid, right * size_of(T)) - mid = ptr_sub(mid, right) + for { + ptr_swap_non_overlapping(ptr_sub(mid, right), mid, right * size_of(T)) + mid = ptr_sub(mid, right) - left -= right + left -= right + if left < right { + break + } + } } else { - ptr_swap_non_overlapping(ptr_sub(mid, left), mid, left * size_of(T)) - mid = ptr_add(mid, left) + for { + ptr_swap_non_overlapping(ptr_sub(mid, left), mid, left * size_of(T)) + mid = ptr_add(mid, left) - right -= left + right -= left + if right < left { + break + } + } } } }