Browse Source

Correct `map_reserve_dynamic` caused by an bizarre code generation bug

gingerBill 2 years ago
parent
commit
ad0f11668b
3 changed files with 23 additions and 19 deletions
  1. 3 5
      core/mem/alloc.odin
  2. 3 3
      core/runtime/core_builtin.odin
  3. 17 11
      core/runtime/dynamic_map_internal.odin

+ 3 - 5
core/mem/alloc.odin

@@ -164,8 +164,6 @@ new_clone :: proc(data: $T, allocator := context.allocator, loc := #caller_locat
 	return nil, .Out_Of_Memory
 	return nil, .Out_Of_Memory
 }
 }
 
 
-DEFAULT_RESERVE_CAPACITY :: 16
-
 make_aligned :: proc($T: typeid/[]$E, #any_int len: int, alignment: int, allocator := context.allocator, loc := #caller_location) -> (slice: T, err: Allocator_Error) {
 make_aligned :: proc($T: typeid/[]$E, #any_int len: int, alignment: int, allocator := context.allocator, loc := #caller_location) -> (slice: T, err: Allocator_Error) {
 	runtime.make_slice_error_loc(loc, len)
 	runtime.make_slice_error_loc(loc, len)
 	data := alloc_bytes(size_of(E)*len, alignment, allocator, loc) or_return
 	data := alloc_bytes(size_of(E)*len, alignment, allocator, loc) or_return
@@ -179,7 +177,7 @@ make_slice :: proc($T: typeid/[]$E, #any_int len: int, allocator := context.allo
 	return make_aligned(T, len, align_of(E), allocator, loc)
 	return make_aligned(T, len, align_of(E), allocator, loc)
 }
 }
 make_dynamic_array :: proc($T: typeid/[dynamic]$E, allocator := context.allocator, loc := #caller_location) -> (T, Allocator_Error) {
 make_dynamic_array :: proc($T: typeid/[dynamic]$E, allocator := context.allocator, loc := #caller_location) -> (T, Allocator_Error) {
-	return make_dynamic_array_len_cap(T, 0, DEFAULT_RESERVE_CAPACITY, allocator, loc)
+	return make_dynamic_array_len_cap(T, 0, 16, allocator, loc)
 }
 }
 make_dynamic_array_len :: proc($T: typeid/[dynamic]$E, #any_int len: int, allocator := context.allocator, loc := #caller_location) -> (T, Allocator_Error) {
 make_dynamic_array_len :: proc($T: typeid/[dynamic]$E, #any_int len: int, allocator := context.allocator, loc := #caller_location) -> (T, Allocator_Error) {
 	return make_dynamic_array_len_cap(T, len, len, allocator, loc)
 	return make_dynamic_array_len_cap(T, len, len, allocator, loc)
@@ -194,12 +192,12 @@ make_dynamic_array_len_cap :: proc($T: typeid/[dynamic]$E, #any_int len: int, #a
 	array = transmute(T)s
 	array = transmute(T)s
 	return
 	return
 }
 }
-make_map :: proc($T: typeid/map[$K]$E, #any_int cap: int = DEFAULT_RESERVE_CAPACITY, allocator := context.allocator, loc := #caller_location) -> T {
+make_map :: proc($T: typeid/map[$K]$E, #any_int cap: int = 1<<runtime.MAP_MIN_LOG2_CAPACITY, allocator := context.allocator, loc := #caller_location) -> T {
 	runtime.make_map_expr_error_loc(loc, cap)
 	runtime.make_map_expr_error_loc(loc, cap)
 	context.allocator = allocator
 	context.allocator = allocator
 
 
 	m: T
 	m: T
-	reserve_map(&m, cap)
+	reserve_map(&m, cap, loc)
 	return m
 	return m
 }
 }
 make_multi_pointer :: proc($T: typeid/[^]$E, #any_int len: int, allocator := context.allocator, loc := #caller_location) -> (mp: T, err: Allocator_Error) {
 make_multi_pointer :: proc($T: typeid/[^]$E, #any_int len: int, allocator := context.allocator, loc := #caller_location) -> (mp: T, err: Allocator_Error) {

+ 3 - 3
core/runtime/core_builtin.odin

@@ -231,12 +231,12 @@ make_dynamic_array_len_cap :: proc($T: typeid/[dynamic]$E, #any_int len: int, #a
 	return
 	return
 }
 }
 @(builtin)
 @(builtin)
-make_map :: proc($T: typeid/map[$K]$E, #any_int cap: int = 1<<MAP_MIN_LOG2_CAPACITY, allocator := context.allocator, loc := #caller_location) -> T {
-	make_map_expr_error_loc(loc, cap)
+make_map :: proc($T: typeid/map[$K]$E, #any_int capacity: int = 1<<MAP_MIN_LOG2_CAPACITY, allocator := context.allocator, loc := #caller_location) -> T {
+	make_map_expr_error_loc(loc, capacity)
 	context.allocator = allocator
 	context.allocator = allocator
 
 
 	m: T
 	m: T
-	reserve_map(&m, cap)
+	reserve_map(&m, capacity, loc)
 	return m
 	return m
 }
 }
 @(builtin)
 @(builtin)

+ 17 - 11
core/runtime/dynamic_map_internal.odin

@@ -437,36 +437,42 @@ map_grow_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_Inf
 
 
 
 
 map_reserve_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_Info, new_capacity: uintptr, loc := #caller_location) -> Allocator_Error {
 map_reserve_dynamic :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_Info, new_capacity: uintptr, loc := #caller_location) -> Allocator_Error {
+	ceil_log2 :: #force_inline proc "contextless" (x: uintptr) -> uintptr {
+		z := intrinsics.count_leading_zeros(x)
+		if z > 0 && x & (x-1) != 0 {
+			z -= 1
+		}
+		return size_of(uintptr)*8 - 1 - z
+	}
+
 	if m.allocator.procedure == nil {
 	if m.allocator.procedure == nil {
 		m.allocator = context.allocator
 		m.allocator = context.allocator
 	}
 	}
 
 
 	new_capacity := new_capacity
 	new_capacity := new_capacity
+	old_capacity := uintptr(map_cap(m^))
 
 
-	log2_capacity := map_log2_cap(m^)
-	capacity := uintptr(1) << log2_capacity
-
-	if capacity >= new_capacity {
+	if old_capacity >= new_capacity {
 		return nil
 		return nil
 	}
 	}
 
 
-	new_capacity = max(new_capacity, uintptr(1)<<MAP_MIN_LOG2_CAPACITY)
-
 	// ceiling nearest power of two
 	// ceiling nearest power of two
-	log2_new_capacity := size_of(uintptr) - intrinsics.count_leading_zeros(new_capacity-1)
+	log2_new_capacity := ceil_log2(new_capacity)
+
+	log2_min_cap := max(MAP_MIN_LOG2_CAPACITY, log2_new_capacity)
 
 
 	if m.data == 0 {
 	if m.data == 0 {
-		m^ = map_alloc_dynamic(info, MAP_MIN_LOG2_CAPACITY, m.allocator, loc) or_return
+		m^ = map_alloc_dynamic(info, log2_min_cap, m.allocator, loc) or_return
 		return nil
 		return nil
 	}
 	}
 
 
-	resized := map_alloc_dynamic(info, log2_new_capacity, m.allocator, loc) or_return
+	resized := map_alloc_dynamic(info, log2_min_cap, m.allocator, loc) or_return
 
 
 	ks, vs, hs, _, _ := map_kvh_data_dynamic(m^, info)
 	ks, vs, hs, _, _ := map_kvh_data_dynamic(m^, info)
 
 
 	// Cache these loads to avoid hitting them in the for loop.
 	// Cache these loads to avoid hitting them in the for loop.
 	n := m.len
 	n := m.len
-	for i in 0..<capacity {
+	for i in 0..<old_capacity {
 		hash := hs[i]
 		hash := hs[i]
 		if map_hash_is_empty(hash) {
 		if map_hash_is_empty(hash) {
 			continue
 			continue
@@ -690,7 +696,7 @@ __dynamic_map_get :: proc "contextless" (#no_alias m: ^Raw_Map, #no_alias info:
 
 
 // IMPORTANT: USED WITHIN THE COMPILER
 // IMPORTANT: USED WITHIN THE COMPILER
 __dynamic_map_check_grow :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_Info, loc := #caller_location) -> Allocator_Error {
 __dynamic_map_check_grow :: proc "odin" (#no_alias m: ^Raw_Map, #no_alias info: ^Map_Info, loc := #caller_location) -> Allocator_Error {
-	if m.len + 1 >= map_resize_threshold(m^) {
+	if m.len  >= map_resize_threshold(m^) {
 		return map_grow_dynamic(m, info, loc)
 		return map_grow_dynamic(m, info, loc)
 	}
 	}
 	return nil
 	return nil