LLEXT: automatic gcc / xt-clang switching #9151
Conversation
lyakh
requested review from
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
marc-hb,
mmaka1 and
plbossart
as code owners
scripts/xtensa-build-zephyr.py
Outdated
| # libraries for Xtensa. Therefore when it's used we switch to | ||
| # building relocatable ELF objects. | ||
| if ("ZEPHYR_TOOLCHAIN_VARIANT" in platf_build_environ.keys() and | ||
| platf_build_environ["ZEPHYR_TOOLCHAIN_VARIANT"] == 'xt-clang'): |
There was a problem hiding this comment.
Python can already do that for you:
| platf_build_environ["ZEPHYR_TOOLCHAIN_VARIANT"] == 'xt-clang'): | |
| if platf_build_environ.get("ZEPHYR_TOOLCHAIN_VARIANT") == 'xt-clang' |
scripts/llext_link_helper.py
Outdated
| last_type = TYPE_NONE | ||
|
|
||
| SHF_ALLOC = 2 | ||
| SHF_EXECINSTR = 4 |
There was a problem hiding this comment.
A simple git grep showed that these are already defined, no need to duplicate
ipython3
In [1]: from elftools.elf.constants import SH_FLAGS
In [2]: SH_FLAGS.SHF_EXECINSTR
Out[2]: 4
There was a problem hiding this comment.
In [1]: from elftools.elf.constants import SH_FLAGS
that's the line I couldn't find, thanks
scripts/llext_link_helper.py
Outdated
| TYPE_NONE = 0 | ||
| TYPE_DATA = 1 | ||
| TYPE_BSS = 2 | ||
| last_type = TYPE_NONE |
There was a problem hiding this comment.
https://docs.python.org/3/library/enum.html
from enum import Enum
SECTION_TYPE = Enum('SECTION_TYPE', ['DATA', 'BSS'])Don't define TYPE_NONE (the "billion dollar mistake"), just use Python's None.
| # A shared object type image, it contains segments | ||
| sections = ['.text', '.rodata', '.data', '.bss'] | ||
| alignment = [0x1000, 0x1000, 0x10, 0x1000] | ||
| # File names differ when building shared or relocatable objects |
There was a problem hiding this comment.
Does this really belong to this script? As opposed to its caller.
There was a problem hiding this comment.
we do deal with both cases internally, and passing a module name additionally to the file name would be redundant
scripts/llext_link_helper.py
Outdated
| data_found = False | ||
| bss_found = False | ||
|
|
||
| # The layout seems to be: .text, followed by other .data, .rodata and |
There was a problem hiding this comment.
@marc-hb I haven't found anywhere a statement, that that should be the case, but so far that's what I've seen
There was a problem hiding this comment.
I haven't tried to understand the logic so I wonder why it matters. Can't you just perform two passes? (1) collect all required information, no side-effect (2) do what must be done.
scripts/llext_link_helper.py
Outdated
|
|
||
| if bss_found or data_found: | ||
| print("ERROR: So far .text is expected first. Fix the script!") | ||
| return |
scripts/llext_link_helper.py
Outdated
| if last_type == TYPE_BSS and data_found: | ||
| # Trouble: intermixed data and bss | ||
| print('ERROR: data again after data and .bss! Aborting') | ||
| return |
There was a problem hiding this comment.
Aborting with a success code?
Maybe:
assert last_type != TYPE_BSS or not data_found, "data again after data and .bss! Aborting'
When using a clang Cadence toolchain to build SOF and LLEXT modules we need to select a different LLEXT type than when using a Zephyr gcc toolchain. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
It was clear that hard-coded section names aren't reliable enough but they broke down way earlier than has been expected. This patch replaces hard-coded sections with a loop, scanning all sections and selecting them based on their flags and types. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
When using xt-clang we cannot build shared objects, so we use '-r' to link relocatable objects. But the decision shouldn't be made based on the name of the compiler, but based on the selected LLEXT_BINARY_TYPE option. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
marc-hb
left a comment
marc-hb
left a comment
There was a problem hiding this comment.
Code style looks good to me.
Someone must review the actual ELF logic.
|
@lyakh Ready to go, pending mandatory CI to pass |
|
@wszypelt is quickbuild hanging here, can we restart, please? |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Even with https://sof-ci.01.org/sofpr/PR9403/build7332/build/firmware_community/mtl.log |
4 participants
We build different LLEXT object types with gcc and with xt-cland and switching between them should be automatic