RFC: Streamline implementation overrides #392
Draft
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background and motivation
While preparing initial patches for RISC-V support (as mentioned in #368), I noticed that overriding function implementations requires a lot of manual copy-pasting, leading to code duplication and increased maintenance effort.
This PR proposes a streamlined approach to declaring function overrides. I applied it to two functions in Silk (
silk_NSQ
andsilk_VQ_WMat_EC
) for x86 to demonstrate its benefits and gather feedback on whether such a refactor would be accepted upstream.If approved, I can extend this approach to all overridable functions across all architectures, including Celt (which seems to use a similar override mechanism – please correct me if I'm wrong).
Proposal
main_overrides.h
), which can also be used for function type definition for use in the implementation array (e.g., forsilk_NSQ
function, there is nowSILK_NSQ_DECL(impl)
macro, which can be used to declare the function for all backends).arch
argument in the main macro definition is moved to the front, allowing for the use of macro variadic arguments, reducing the copy-paste of argument lists for each implementation.OVERRIDE_IMPL_SINGLE
, which references the selected backend implementation.OVERRIDE_IMPL_ARRAY
, which addresses the override array created withOVERRIDE_IMPL_ARRAY_DECL
.This approach significantly reduces manual (and error-prone) copy-paste work, minimizes the risk of errors when developing new backends (e.g., RISC-V in our case), and improves maintainability. Now, function definition changes require modification in only one place, reducing the likelihood of regressions across platforms.
Example
To enable the override of a function
foo
in Silk:main_overrides.h
in the form ofFOO_DECL(impl, ...)
macro and function type definition withtypedef FOO_DECL(t, *const);
.main.h
:FOO_DECL(c) { ... }
.arch
as the first argument.Now, you can add arch-specific implementation (an example for x86 SSE):
main_sse.h
:x86_silk_map.c
:foo.c
with