-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
cmake: xcc: fix compilation issues #79731
Conversation
This is great if it is enough to fix XCC compilation that was just broken but note this does not seem to answer the confusing cc: @tejlmand |
Twister failures are on |
This comment was marked as outdated.
This comment was marked as outdated.
XCC does not recognize -fno-printf-return-value as compiler flag. So skip it. Signed-off-by: Daniel Leung <[email protected]>
Turns out it is not too difficult to remove |
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.
@@ -6,6 +6,7 @@ set(COMPILER xcc) | |||
set(OPTIMIZE_FOR_DEBUG_FLAG "-O0") | |||
set(CC xcc) | |||
set(C++ xc++) | |||
set(LINKER xt-ld) |
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.
Do we really need to use an obsolete linker in addition to using an obsolete compiler? The existing solution worked fine until the big CMake shuffle. Moreover, you seemed to have cracked the TOOLCHAIN_LIBS nut in v2 so is this switch really still necessary?
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.
I think it is better to stay within the confine of the Xtensa toolchain instead of mixing toolchain components that may not be compatible with each other.
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.
I think this is debatable. Compilation and linkage are two steps fairly independent from each other with good compatibility and zephyr/cmake seems to assume they are indeed. xt-xcc
has been a pre-C11 pain but it features proprietary customizations found no one else, so it's a necessary pain. What value does xt-ld
bring to the table besides old linker bugs that no-one wants to debug and a lack of recent features? Isn't the same than ld
, just older? Also: it has apparently NOT been used in production. Probably by accident but still.
But back to the initial issue: can't we have ONLY a fix for the CMake regression FIRST? And then only later a linker change once SOF CI is back on its feet?
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.
Also:
xt-ld
has apparently NOT been used in production. Probably by accident but still.
Correction.
So I just tested multiple Zephyr versions with -DEXTRA_LDFLAGS=-lfail_and_tell
to see what linker is actually in use. As usual: the best way to test something is to break it. For a build system it's probably the ONLY testing method available... I digress
It turns out that linkage is and was performed by NOT invoking the linker directly but by invoking the the compiler front-end XtensaTools/bin/xt-xcc
. As it should be. Also, the error message is always XtensaTools/bin/xt-ld: cannot find -lfail_and_tell
.
So, xt-ld
has always been used?
The error message comes from XtensaTools/bin/xt-ld
before and after the big #78320 shuffle (which I clearly don't have a clue about, just in case that wasn't obvious yet). This is also true before and after this PR. Funny enough this is true before AND after commit cmake: xcc: use xt-ld as linker
. So at the very least this commit title must be fixed because this does not seem to actually switch the linker.
-lhal
is missing without that commit so that commit does "something" but it apparently does not switch the linker.
Note: I tested SOF compilation with the SOF configuration script sof/scripts/xtensa-build-zephyr.py tgl
. I really, really hope that does not make any linker difference; I'd be amazed!
Shuffling of ld/lld on C library linking cmake code causes issue with XCC as the HAL library is not being included in linking. So make XCC to use xt-ld linker cmake code such that the HAL library is included. Signed-off-by: Daniel Leung <[email protected]>
Following the footstep of GCC/Clang cmake code to remove TOOLCHAIN_LIBS, xcc also has it removed and utilizes something similar to c_library to link the HAL library. Signed-off-by: Daniel Leung <[email protected]>
The commit message actually refers to the "xt-ld linker cmake code" so the message itself is fine. I just changed the title to reflect that. |
Thanks, the old one really threw me off. |
placed a comment here: #79731 (review) EDIT: here #78320 (comment) |
Fixes some issues when compiling with good old XCC.