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

arch/xtensa: fix definition for _int32_t and _uint32_t #16022

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fdcavalcanti
Copy link
Contributor

Summary

Modify types.h and inttypes.h to use the correct _int32_t and _uint32_t types, on Xtensa arch.

This PR solves an issue first mentioned in #15755.

Type is now defined according to recent compiler versions.

On xtensa-esp32-gcc version 8.4.0 we have:

#define __INT32_TYPE__ int
#define __UINT32_TYPE__ unsigned int

While version 12.2.0 has:

#define __INT32_TYPE__ long int
#define __UINT32_TYPE__ long unsigned int

Due to this type modification, many logging calls had to be fixed with the proper format specifiers: %u, %lu, %ld, etc across arch and board files.

Impact

  • Impact on user: No.
  • Impact on build: Yes, but all fixes are present in this PR.
  • Impact on hardware: Yes, Xtensa devices only.
  • Impact on documentation: No.
  • Impact on security: No.
  • Impact on compatibility: Yes, improves compatibility since it uses the proper int types as specified by the compiler.

Testing

Most tests done on Espressif's internal CI. Each build that failed had the error fixed, resulting in the commits of this PR.
Since errors are all logging related, the build system should catch everything.

Tested build on all defconfigs for:

  • esp32-devkitc
  • esp32-ethernet-kit
  • esp32-lyrat
  • esp32-pico-kit
  • esp32-wrover-kit
  • esp32s2-saola-1
  • esp32s3-devkit
  • esp32s3-korvo-2
  • esp32s3-lcd-ev

Modify types.h and inttypes.h to use the correct _int32_t and _uint32_t types.
Type is now defined according to recent compiler versions.

Signed-off-by: Filipe Cavalcanti <[email protected]>
@github-actions github-actions bot added Arch: xtensa Issues related to the Xtensa architecture Board: xtensa Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. labels Mar 19, 2025
@nuttxpr
Copy link

nuttxpr commented Mar 19, 2025

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the NuttX requirements. The summary clearly explains the problem, the solution, and the affected code. The impact section addresses all relevant areas, noting the specific hardware affected (Xtensa) and the improved compatibility. While ideally more detailed local testing logs would be included, the explanation regarding Espressif's internal CI and the comprehensive list of tested defconfigs provide reasonable assurance of testing. The connection to issue #15755 adds valuable context.

@xiaoxiang781216
Copy link
Contributor

Due to this type modification, many logging calls had to be fixed with the proper format specifiers: %u, %lu, %ld, etc across arch and board files.
No, you should use PRIxyy from include/inttypes.h to be portable.

@fdcavalcanti
Copy link
Contributor Author

fdcavalcanti commented Mar 19, 2025

@yamt seems we have an issue on toywasm. Could you please take a look at the CI build log?

Fix format specifier macros after compiler type changes.

Signed-off-by: Filipe Cavalcanti <[email protected]>
Fix format specifier macros after compiler type changes.

Signed-off-by: Filipe Cavalcanti <[email protected]>

Fix boards with PRI
@fdcavalcanti fdcavalcanti force-pushed the feature/xtensa-newlib-fix branch from 494b020 to 20adec7 Compare March 21, 2025 17:24
@fdcavalcanti
Copy link
Contributor Author

Applied the changes recommended. Let's please hold this PR while we fix the toywasm build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: xtensa Issues related to the Xtensa architecture Board: xtensa Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants