Skip to content
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

xcc: fix building errors related to logging #43174

Merged
merged 7 commits into from
Feb 28, 2022

Conversation

dcpleung
Copy link
Member

XCC is based on very old GCC (sigh...) that there is no auto type, and _Pragma cannot be used within functions. The latest logging changes related to argument evaluation resulted in build errors regarding these. See individual commits for more details.

@dcpleung dcpleung linked an issue Feb 24, 2022 that may be closed by this pull request
@github-actions github-actions bot added area: API Changes to public APIs area: Logging labels Feb 24, 2022
@cgturner1
Copy link

i did test these with XCC for all adsp targets with the issues mentioned in #43087 and now everything builds as expected without the _pragma related errors

@dcpleung dcpleung marked this pull request as ready for review February 25, 2022 00:53
@dcpleung dcpleung force-pushed the xcc_logging_build_errors branch from 6359abe to 50e3dfb Compare February 25, 2022 00:54
andyross
andyross previously approved these changes Feb 25, 2022
Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpicks but nothing worth blocking merge over

@@ -34,6 +35,12 @@ typedef uint64_t log_timestamp_t;
typedef uint32_t log_timestamp_t;
#endif

#if !defined(CONFIG_LOG) && \
!GCC_VERSION_AT_LEAST(4, 9, 0) && \
!CLANG_VERSION_AT_LEAST(3, 8, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: abstraction-wise, might be better to have toolchain.h define "TOOLCHAIN_HAS_AUTO_TYPE" or somesuch instead of putting the logic here in logging.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also guards against using _Pragma() within functions, so not exactly about auto type only.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm also for moving flags to toolchain.h.
TOOLCHAIN_HAS_AUTO_TYPE, TOOLCHAIN_HAS_C11_GENERIC, maybe TOOLCHAIN_HAS_DIAGNOSTIC_PRAGMAS (seems that this was introduced in 4.6).

It's already third place i'm aware of where we do that (one in PR

#if defined(__cplusplus) || (((__STDC_VERSION__ >= 201112L) || \
).

include/logging/log_msg2.h Outdated Show resolved Hide resolved
@hongshui3000
Copy link
Contributor

I have previously reported that XCC cannot compile the current LOG code without optimization (with the -O0 parameter). The PR I submitted seems to be uninteresting. I don't know if this PR can solve this issue.@dcpleung

Adds a GCC_VERSION macro to make it easier to write
preprocessor conditionals.

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
Adds a CLANG_VERSION macro to make it easier to write
preprocessor conditionals.

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
@dcpleung dcpleung force-pushed the xcc_logging_build_errors branch from 3e90f6c to 277b568 Compare February 25, 2022 22:52
This introduces the macro TOOLCHAIN_HAS_PRAGMA_DIAG to indicate
that the toolchain supports #pragma diagnostics. This is
supported since GCC 4.6.0 and, AFAIK, all Clang.

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
This introduces the macro TOOLCHAIN_HAS_C_GENERIC to indicate
that the toolchain supports C Generic (i.e. _Generic keyword).
This is introduced in C11, and is also supported since GCC 4.9.0
and Clang 3.8.0.

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
This introduces the macro TOOLCHAIN_HAS_C_AUTO_TYPE to
indicate that the toolchain supports __auto_type. This is
supported since GCC 4.9.0 and Clang 3.8.0.

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
For GCC < 4.9.0 and Clang < 3.8.0, auto type is not supported.
But the previous behavior to depend on CONFIG_LOG2_ALWAYS_RUNTIME
to be enabled doesn't work because this kconfig is not available
when CONFIG_LOG is not enabled, as LOG_*() are still being
expanded when CONFIG_LOG=n, resulting in toolchain complaining
about unknown keyword. So when CONFIG_LOG=n and old toolchains,
force it to use runtime packaging to avoid the issue.

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
XCC is based on GCC 4.2.0 which doesn't support auto type.
So force CONFIG_LOG2_ALWAYS_RUNTIME to be enabled if XCC is
being used.

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
@dcpleung dcpleung force-pushed the xcc_logging_build_errors branch from 277b568 to 143d4f3 Compare February 25, 2022 23:01
@carlescufi carlescufi merged commit e54b4fa into zephyrproject-rtos:main Feb 28, 2022
@dcpleung dcpleung deleted the xcc_logging_build_errors branch February 28, 2022 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Logging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XCC build failures for all intel_adsp tests/platforms
6 participants