feat: Add NETopKV function. #1251
Conversation
morgolock
force-pushed
the
pr/cpu_topkv
branch
from
cb2af0c to
1f0778c
Compare
* The Neon(TM) implementation of TopKV reduces execution time from 447.8 ms (scalar CPP) to 11.65 ms for the same workload (F32, C=1000, N=32000, k=3, 6 threads), achieving an approximate 38× speedup. This gain comes from SIMD vectorization, removal of per-element branches, and a more efficient inner loop. * Resolves ARMCL-1227 Change-Id: Ifdf161ce4254dc5ecd57aff9ae22410facd31705 Signed-off-by: Pablo Marquez Tello <pablo.tello@arm.com>
| * @publicapi | ||
| */ | ||
|
|
||
| #include "arm_compute/core/Types.h" |
There was a problem hiding this comment.
Which types are needed from this file?
There was a problem hiding this comment.
Status
If I remove it I get
In file included from src/runtime/NEON/functions/NETopKV.cpp:24:0:
./arm_compute/runtime/NEON/functions/NETopKV.h:79:12: error: ‘Status’ does not name a type; did you mean ‘static’?
static Status
^~~~~~
There was a problem hiding this comment.
That class is defined under arm_compute/core/Error.h
There was a problem hiding this comment.
Replaced Types.h with Error.h in next patchset
| */ | ||
| void configure(const ITensor *predictions, const ITensor *targets, ITensor *output, const unsigned int k); | ||
|
|
||
| /** Static function to check if given info will lead to a valid configuration of @ref CPPTopKVKernel |
There was a problem hiding this comment.
Done in next patchset
| #include "src/core/helpers/WindowHelpers.h" | ||
| #include "src/cpu/kernels/topkv/list.h" | ||
|
|
||
| #include <array> |
There was a problem hiding this comment.
vector? also we need to add cstdint
There was a problem hiding this comment.
Done in next patchset
| void | ||
| configure(const ITensorInfo *predictions, const ITensorInfo *targets, ITensorInfo *output, const unsigned int k); | ||
|
|
||
| /** Static function to check if given info will lead to a valid configuration of @ref CPPTopKVKernel |
There was a problem hiding this comment.
can we just say "similar to @ref CpuTopKV::configure()" to reduce duplication? Have a look at arm_compute/runtime/experimental/operators/CpuActivation.h
Same comment is valid for every header
There was a problem hiding this comment.
Done in next patchset
| ARM_COMPUTE_TRACE_EVENT(ARM_COMPUTE_PROF_CAT_CPU, ARM_COMPUTE_PROF_LVL_CPU, "CpuTopKV::run"); | ||
| ARM_COMPUTE_ERROR_ON_MSG(tensors.empty(), "No inputs provided"); | ||
|
|
||
| NEScheduler::get().schedule_op(_kernel.get(), 0, _kernel->window(), tensors); |
There was a problem hiding this comment.
instead of 0, we use Window::DimX
| *reinterpret_cast<const T *>(predictions(Coordinates{static_cast<int>(c), static_cast<int>(i)})); | ||
| const float v = static_cast<float>(vt); | ||
|
|
||
| // Mirror CPPTopKVKernel: (a - b > epsilon) |
There was a problem hiding this comment.
The same way it's implemented in CPPTopKVKernel
| { | ||
|
|
||
| template <typename T> | ||
| SimpleTensor<uint8_t> topkv(SimpleTensor<T> &predictions, SimpleTensor<uint32_t> &targets, uint32_t k) |
There was a problem hiding this comment.
I think we need to include
- SimpleTensor.h
- TensorShape.h
- cstdint (because of int types)
- CoreTypes.h for DataType
- Coordinates.h
Anything else? We don't need the big Types.h file
There was a problem hiding this comment.
Done in next patchset
| #include "tests/IAccessor.h" | ||
| #include "tests/validation/Helpers.h" | ||
| #include "tests/validation/reference/TopKV.h" | ||
|
|
There was a problem hiding this comment.
I think we need to include cstdint, type_traits, random and algorithm STL headers for the functions used here.
There was a problem hiding this comment.
Done in next patchset
| #include "tests/AssetsLibrary.h" | ||
| #include "tests/framework/Asserts.h" | ||
| #include "tests/framework/Fixture.h" | ||
| #include "tests/framework/ParametersLibrary.h" |
There was a problem hiding this comment.
Do we need
- ParametersLibrary.h
- IAccessor.h
- tests/validation/Helpers.h
There was a problem hiding this comment.
Removed in next patch
| #define ACL_TESTS_VALIDATION_FIXTURES_TOPKVLAYERFIXTURE_H | ||
|
|
||
| #include "arm_compute/core/TensorShape.h" | ||
| #include "arm_compute/core/Types.h" |
There was a problem hiding this comment.
Do we need anything particular from this file?
There was a problem hiding this comment.
Removed them in next patch
morgolock
force-pushed
the
pr/cpu_topkv
branch
from
1f0778c to
cb1481c
Compare
The Neon(TM) implementation of TopKV reduces execution time from 447.8 ms (scalar CPP) to 11.65 ms for the same workload (F32, C=1000, N=32000, k=3, 6 threads), achieving an approximate 38× speedup. This gain comes from SIMD vectorization, removal of per-element branches, and a more efficient inner loop.
Resolves ARMCL-1227
Change-Id: Ifdf161ce4254dc5ecd57aff9ae22410facd31705