From 8991ca14ad18d4f4d3868f742867ce77689bed1d Mon Sep 17 00:00:00 2001 From: Noah Verke Date: Tue, 25 Oct 2022 10:36:34 -0700 Subject: [PATCH 1/5] [Hexagon] Add fix for VTCM allocation search and new test case to cover the issue. --- src/runtime/hexagon/hexagon_vtcm_pool.cc | 2 +- .../hexagon/hexagon_vtcm_pool_tests.cc | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/runtime/hexagon/hexagon_vtcm_pool.cc b/src/runtime/hexagon/hexagon_vtcm_pool.cc index 6024550ba732..df4ff494ef41 100644 --- a/src/runtime/hexagon/hexagon_vtcm_pool.cc +++ b/src/runtime/hexagon/hexagon_vtcm_pool.cc @@ -85,7 +85,7 @@ void* HexagonVtcmPool::Allocate(size_t nbytes) { auto entry_to_allocate = free_.begin(); for (auto it = free_.begin(); it != free_.end(); it++) { - if ((it->second < entry_to_allocate->second) && (it->second >= nbytes)) { + if (((entry_to_allocate->second < nbytes) || (it->second < entry_to_allocate->second)) && (it->second >= nbytes)) { entry_to_allocate = it; if (entry_to_allocate->second == nbytes) { break; diff --git a/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc b/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc index 13c459be0c34..fa380f5c55a0 100644 --- a/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc @@ -34,6 +34,8 @@ class HexagonVtcmPoolTest : public ::testing::Test { public: HexagonVtcmPool* vtcm_pool; size_t max_bytes; + size_t eight_k_block = 8192; + size_t four_k_block = 4096; size_t two_k_block = 2048; size_t one_k_block = 1024; size_t min_bytes = 128; @@ -164,6 +166,45 @@ TEST_F(HexagonVtcmPoolTest, free_alloc_combinations) { vtcm_pool->Free(ptr4, max_bytes); } +TEST_F(HexagonVtcmPoolTest, find_allocation) { + void* ptr1; + void* ptr2; + void* ptr3; + void* ptr4; + void* new_ptr; + + ptr1 = vtcm_pool->Allocate(min_bytes); + ptr2 = vtcm_pool->Allocate(one_k_block); + ptr3 = vtcm_pool->Allocate(two_k_block); + // Free 2, realloc it, make sure it is the same as before + vtcm_pool->Free(ptr2, two_k_block); + + ptr4 = vtcm_pool->Allocate(four_k_block); + new_ptr = vtcm_pool->Allocate(two_k_block); + CHECK(new_ptr == ptr3); + + vtcm_pool->Free(ptr1, min_bytes); + vtcm_pool->Free(ptr2, one_k_block); + vtcm_pool->Free(ptr3, two_k_block); + vtcm_pool->Free(ptr4, four_k_block); + + new_ptr = vtcm_pool->Allocate(min_bytes); + CHECK(new_ptr == ptr1); + + new_ptr = vtcm_pool->Allocate(one_k_block); + CHECK(new_ptr == ptr2); + + new_ptr = vtcm_pool->Allocate(two_k_block); + CHECK(new_ptr == ptr3); + + new_ptr = vtcm_pool->Allocate(four_k_block); + CHECK(new_ptr == ptr4); + + // Make sure at the end we have the full amount available again + ptr4 = vtcm_pool->Allocate(max_bytes); + vtcm_pool->Free(ptr4, max_bytes); +} + // Test alignment edge cases allocating through HexagonBuffer TEST_F(HexagonVtcmPoolTest, vtcm_alignment) { std::unique_ptr test_hexbuffs = std::make_unique(); From 2523087a3f1a74c46f3aee7645e3aa436dd6e4b0 Mon Sep 17 00:00:00 2001 From: Noah Verke Date: Tue, 25 Oct 2022 11:11:57 -0700 Subject: [PATCH 2/5] Add another test for a specific case. --- .../hexagon/hexagon_vtcm_pool_tests.cc | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc b/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc index fa380f5c55a0..a0557dd94625 100644 --- a/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc @@ -171,8 +171,26 @@ TEST_F(HexagonVtcmPoolTest, find_allocation) { void* ptr2; void* ptr3; void* ptr4; - void* new_ptr; + ptr1 = vtcm_pool->Allocate(two_k_block); + ptr2 = vtcm_pool->Allocate(two_k_block); + // Free 2, realloc it, make sure it is the same as before + vtcm_pool->Free(ptr2, two_k_block); + + ptr3 = vtcm_pool->Allocate(four_k_block); + + // Make sure at the end we have the full amount available again + ptr4 = vtcm_pool->Allocate(max_bytes); + vtcm_pool->Free(ptr4, max_bytes); +} + +TEST_F(HexagonVtcmPoolTest, find_smallest_allocation_combinations) { + void* ptr1; + void* ptr2; + void* ptr3; + void* ptr4; + void* new_ptr; + ptr1 = vtcm_pool->Allocate(min_bytes); ptr2 = vtcm_pool->Allocate(one_k_block); ptr3 = vtcm_pool->Allocate(two_k_block); From eaf42a9f731baa21463fff8ca3fcc5af2c68b117 Mon Sep 17 00:00:00 2001 From: Noah Verke Date: Tue, 25 Oct 2022 14:58:00 -0700 Subject: [PATCH 3/5] [Hexagon] Fix tests. --- src/runtime/hexagon/hexagon_vtcm_pool.cc | 3 +- .../hexagon/hexagon_vtcm_pool_tests.cc | 60 +++++++++++++------ 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/src/runtime/hexagon/hexagon_vtcm_pool.cc b/src/runtime/hexagon/hexagon_vtcm_pool.cc index df4ff494ef41..17089852a954 100644 --- a/src/runtime/hexagon/hexagon_vtcm_pool.cc +++ b/src/runtime/hexagon/hexagon_vtcm_pool.cc @@ -85,7 +85,8 @@ void* HexagonVtcmPool::Allocate(size_t nbytes) { auto entry_to_allocate = free_.begin(); for (auto it = free_.begin(); it != free_.end(); it++) { - if (((entry_to_allocate->second < nbytes) || (it->second < entry_to_allocate->second)) && (it->second >= nbytes)) { + if ((entry_to_allocate->second < nbytes || it->second < entry_to_allocate->second) && + it->second >= nbytes) { entry_to_allocate = it; if (entry_to_allocate->second == nbytes) { break; diff --git a/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc b/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc index a0557dd94625..5d83c0ccd5f5 100644 --- a/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc @@ -170,18 +170,24 @@ TEST_F(HexagonVtcmPoolTest, find_allocation) { void* ptr1; void* ptr2; void* ptr3; - void* ptr4; - + ptr1 = vtcm_pool->Allocate(two_k_block); ptr2 = vtcm_pool->Allocate(two_k_block); - // Free 2, realloc it, make sure it is the same as before - vtcm_pool->Free(ptr2, two_k_block); + // Free the first allocation + vtcm_pool->Free(ptr1, two_k_block); + + // Allocate a new larger block to initiate search and ensure + // it succeeds despite there not being a matching block. ptr3 = vtcm_pool->Allocate(four_k_block); + // Clean up the ptrs + vtcm_pool->Free(ptr2, two_k_block); + vtcm_pool->Free(ptr3, four_k_block); + // Make sure at the end we have the full amount available again - ptr4 = vtcm_pool->Allocate(max_bytes); - vtcm_pool->Free(ptr4, max_bytes); + ptr1 = vtcm_pool->Allocate(max_bytes); + vtcm_pool->Free(ptr1, max_bytes); } TEST_F(HexagonVtcmPoolTest, find_smallest_allocation_combinations) { @@ -191,32 +197,48 @@ TEST_F(HexagonVtcmPoolTest, find_smallest_allocation_combinations) { void* ptr4; void* new_ptr; - ptr1 = vtcm_pool->Allocate(min_bytes); - ptr2 = vtcm_pool->Allocate(one_k_block); - ptr3 = vtcm_pool->Allocate(two_k_block); - // Free 2, realloc it, make sure it is the same as before + ptr1 = vtcm_pool->Allocate(two_k_block); + ptr2 = vtcm_pool->Allocate(two_k_block); + ptr3 = vtcm_pool->Allocate(four_k_block); + ptr4 = vtcm_pool->Allocate(four_k_block); + + // Fragment memory allocations. vtcm_pool->Free(ptr2, two_k_block); + vtcm_pool->Free(ptr3, four_k_block); + + // Reallocate memory allocations and ensure that the smallest free allocations are used. + new_ptr = vtcm_pool->Allocate(two_k_block); + CHECK(new_ptr == ptr2); - ptr4 = vtcm_pool->Allocate(four_k_block); new_ptr = vtcm_pool->Allocate(two_k_block); CHECK(new_ptr == ptr3); - vtcm_pool->Free(ptr1, min_bytes); - vtcm_pool->Free(ptr2, one_k_block); + vtcm_pool->Free(ptr1, two_k_block); + vtcm_pool->Free(ptr2, two_k_block); vtcm_pool->Free(ptr3, two_k_block); vtcm_pool->Free(ptr4, four_k_block); + // Rerun the same test for non 2k aligned allocations. + ptr1 = vtcm_pool->Allocate(min_bytes); + ptr2 = vtcm_pool->Allocate(min_bytes); + ptr3 = vtcm_pool->Allocate(one_k_block); + ptr4 = vtcm_pool->Allocate(one_k_block); + + // Fragment memory allocations. + vtcm_pool->Free(ptr2, min_bytes); + vtcm_pool->Free(ptr3, one_k_block); + + // Reallocate memory allocations and ensure that the smallest free allocations are used. new_ptr = vtcm_pool->Allocate(min_bytes); - CHECK(new_ptr == ptr1); + CHECK(new_ptr == ptr2); new_ptr = vtcm_pool->Allocate(one_k_block); - CHECK(new_ptr == ptr2); - - new_ptr = vtcm_pool->Allocate(two_k_block); CHECK(new_ptr == ptr3); - new_ptr = vtcm_pool->Allocate(four_k_block); - CHECK(new_ptr == ptr4); + vtcm_pool->Free(ptr1, min_bytes); + vtcm_pool->Free(ptr2, min_bytes); + vtcm_pool->Free(ptr3, one_k_block); + vtcm_pool->Free(ptr4, one_k_block); // Make sure at the end we have the full amount available again ptr4 = vtcm_pool->Allocate(max_bytes); From 544e9bd70cc995624bf608023d3322c74ccb194f Mon Sep 17 00:00:00 2001 From: Noah Verke Date: Tue, 25 Oct 2022 15:00:15 -0700 Subject: [PATCH 4/5] Remove unused var --- tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc b/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc index 5d83c0ccd5f5..ee460e18ba8b 100644 --- a/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc @@ -34,7 +34,6 @@ class HexagonVtcmPoolTest : public ::testing::Test { public: HexagonVtcmPool* vtcm_pool; size_t max_bytes; - size_t eight_k_block = 8192; size_t four_k_block = 4096; size_t two_k_block = 2048; size_t one_k_block = 1024; From b4fb521777e1374455838c8ff513044b3e3044d2 Mon Sep 17 00:00:00 2001 From: Noah Verke Date: Tue, 25 Oct 2022 15:31:45 -0700 Subject: [PATCH 5/5] Change to comments to make more clear. --- tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc b/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc index ee460e18ba8b..81bd31cc84d5 100644 --- a/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc +++ b/tests/cpp-runtime/hexagon/hexagon_vtcm_pool_tests.cc @@ -177,7 +177,7 @@ TEST_F(HexagonVtcmPoolTest, find_allocation) { vtcm_pool->Free(ptr1, two_k_block); // Allocate a new larger block to initiate search and ensure - // it succeeds despite there not being a matching block. + // it succeeds despite there not being a match in the first free block. ptr3 = vtcm_pool->Allocate(four_k_block); // Clean up the ptrs