app: kconfig: allow FLIX generation#10511
app: kconfig: allow FLIX generation#10511dcpleung wants to merge 2 commits intothesofproject:mainfrom
Conversation
|
@lgirdwood @kv2019i When updating to the latest Zephyr SHA, you will need the kconfig to restore the previous behavior of FLIX instruction generation. |
|
I was misled by the Zephyrs PR title "cmake: toolchain/xt-clang: allow to skip FLIX generation". The title suggested adding an option to optionally disable FLIX generation, not changing the default behavior to disable it. The PR description explicitly states that:
Based on that, I expected an opt-out mechanism only, without altering the default state. If FLIX needs to be disabled for sof application configuration, it should be done explicitly in the Linux overlay, without unnecessarily degrading performance or changing defaults for other platforms. |
|
|
||
| # Allow compiler to determine whether to generate FLIX instructions. | ||
| choice COMPILER_CODEGEN_VLIW | ||
| default COMPILER_CODEGEN_VLIW_AUTO if "$(ZEPHYR_TOOLCHAIN_VARIANT)" = "xt-clang" |
There was a problem hiding this comment.
VLIW is a generic term which can be a project default (AUTO) state regardless of compiler, no need to specify xt-clang here
There was a problem hiding this comment.
I'm pretty sure GCC cant emit VLIW for xtensa so it will need some check for Cadence compilers like above.
There was a problem hiding this comment.
"AUTO" != "enabled", it means compiler will decide (use default config)
There was a problem hiding this comment.
"AUTO" != "enabled", it means compiler will decide (use default config)
ok, so you are confirming no impact for GCC here and Zephyr SDK i.e. I just want to confirm we wont be passing any --enable-flix=auto type arguments to GCC which it wont understand if we remove the compiler check.
There was a problem hiding this comment.
It is only a configuration option, not a value passed to the compiler command line. The "auto" value does not result in setting any compile options.
https://github.com/zephyrproject-rtos/zephyr/blob/b4108b53f6d7ca8c1da596c7130318e64e26eaf0/CMakeLists.txt#L562-L568
There was a problem hiding this comment.
@dcpleung sounds like we have all got alignment now so I think good to update and make non draft ?
There was a problem hiding this comment.
Separated out the west.yml change into its own commit. So should be good to go.
|
|
||
| # Allow compiler to determine whether to generate FLIX instructions. | ||
| choice COMPILER_CODEGEN_VLIW | ||
| default COMPILER_CODEGEN_VLIW_AUTO if "$(ZEPHYR_TOOLCHAIN_VARIANT)" = "xt-clang" |
There was a problem hiding this comment.
I'm pretty sure GCC cant emit VLIW for xtensa so it will need some check for Cadence compilers like above.
|
The original idea was to add an option to disable it on demand. However, it turned out that FLIX was causing multiple CI failures so that needs to be disabled by default. Zephyr main should provide a stable foundation for others to build on. Hence this PR to re-enable it to keep the old behavior. |
@dcpleung just to be aligned - Zephyr main will be FLIX disabled for stability ? For SOF application/module logic we need FLIX enabled for performance. I assume we are good to disable for Zephyr and enable for SOF, i.e. there's no FLIX support needed in Zephyr for managing any ISA/registers (and I dont think there are any?) e.g. around context switches or thread scheduling ? |
Yes. That's correct. Though you will need the kconfig changes in this PR when you move to newer Zephyr.
FLIX does not need any registers. It's simply fusing instructions together for density and parallel execution. |
Total of 1145 commits.
Changes include:
a5a9ff4c28c xtensa: handle instruction TLB multi-hit exception
9b9e9ebde77 xtensa: mmu: data TLB multi-hit only handle
exception address
599df95875c xtensa: return back to interrupted thread for
certain exceptions
27e48026215 xtensa: remove unnecessary setting of is_fatal_error
during exc
8d072e85b1e ipc: intel_adsp: fix inverted sync wait condition
ed4cec2d220 xtensa: ptables: doxygen doc
744a7f4daf1 xtensa: mmu: do doxygen for functions
50e980d9a83 xtensa: mmu: halt system if not enough L2 tables
during boot
82b7d94d452 xtensa: mmu: add debug logs on page table
allocations
0777dbea028 xtensa: mmu: assert when L2 table allocation fails
during dup
71262d0e075 arch: xtensa: avoid the use of "sanity check" term
Signed-off-by: Daniel Leung <daniel.leung@intel.com>
39f5acd to
2a5f714
Compare
There was a problem hiding this comment.
Pull request overview
This PR enables FLIX (Flexible Length Instruction eXtension) instruction generation for the Xtensa toolchain by updating the Zephyr revision and adding Kconfig configuration to allow the compiler to automatically determine whether to generate FLIX instructions.
Changes:
- Updated Zephyr upstream revision to a newer commit
- Added Kconfig choice for COMPILER_CODEGEN_VLIW to enable automatic FLIX instruction generation when using xt-clang toolchain
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| west.yml | Updates Zephyr revision to 05a3f62f4456b25f1830eeb1a09c39e4a2a8bc39 to support FLIX configuration |
| app/Kconfig | Adds COMPILER_CODEGEN_VLIW choice with automatic FLIX generation for xt-clang toolchain |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Allow compiler to determine whether to generate FLIX instructions. | ||
| choice COMPILER_CODEGEN_VLIW | ||
| default COMPILER_CODEGEN_VLIW_AUTO if "$(ZEPHYR_TOOLCHAIN_VARIANT)" = "xt-clang" |
There was a problem hiding this comment.
Inconsistent indentation: The indentation uses spaces here, but the rest of the file uses tabs (see lines 2 and 5). For consistency with the existing code, this line should use a tab instead of spaces.
| default COMPILER_CODEGEN_VLIW_AUTO if "$(ZEPHYR_TOOLCHAIN_VARIANT)" = "xt-clang" | |
| default COMPILER_CODEGEN_VLIW_AUTO if "$(ZEPHYR_TOOLCHAIN_VARIANT)" = "xt-clang" |
|
Need this to be merged first before moving Zephyr to latest: zephyrproject-rtos/zephyr#103465 |
This sets the toolchain kconfig to allow generating FLIX instructions as it is disabled by default on Zephyr upstream. Signed-off-by: Daniel Leung <daniel.leung@intel.com>
2a5f714 to
94bd48b
Compare
This sets the toolchain kconfig to allow generating FLIX instructions as it is disabled by default on Zephyr upstream.