-
Notifications
You must be signed in to change notification settings - Fork 350
Math: FFT: Move twiddle factors data to DRAM #10457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,6 +111,16 @@ config MATH_FFT_MULTI | |
| this should not be selected directly, please select it from other | ||
| audio components where need it. | ||
|
|
||
| config MATH_FFT_COLD_TWIDDLE_FACTORS | ||
| bool "FFT cold twiddle factors" | ||
| depends on MATH_FFT | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The MATH_FFT_COLD_TWIDDLE_FACTORS kconfig should also depend on the main Kconfig that controls system-wide DRAM usage which we can easily toggle for the whole FW and on the other hand just prevents linker errors when |
||
| default y | ||
| help | ||
| Link twiddle factors for the maximum FFT size to | ||
| DRAM and copy the needed set to SRAM when the FFT | ||
| plan is initialized. The twiddle factors data | ||
| size is currently 29 kB. | ||
|
|
||
| menu "Supported FFT word lengths" | ||
| visible if MATH_FFT | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use
__builtin_assume_aligned(ptr, SIMD_VECTOR_SIZE)when we assign these ?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lgirdwood you mean in fft_common.c at line 139 and in fft_multi.c at line 145 below? Hm, I'm reading, that
__builtin_assume_aligned()is a "hint to the compiler" that the address is aligned. What is it needed here for? To avoid the compiler doing additional alignment for SIMD / HiFi?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, 2 main reasons:
@singalsu if you also iterate our loops by vector size it will also mean CC should not generate any trailing bytes handling code too. Especially important for non intrinsic C code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lgirdwood @singalsu so we want, e.g. in line 139 in fft_common.c
? I might be wrong but I think that these hints have local effect? E.g. where you have code like
you could help the compiler by doing
Then the compiler will know that
datais aligned, so it won't generate unaligned pointer handling for the intrinsic op? So I think it should be used close to where those intrinsic operations are used? See https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html and https://gcc.gnu.org/projects/tree-ssa/vectorization.html Also I don't see warnings being issued anywhere. Seems like that would be difficult when used as in those examples on linked sites.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lyakh thats fine, as long as we can give the compiler the hint (and yes, this may be needed in more than one place) - the vectorizer will use this hint too and has better warnings about when it cant vectorize.