-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Various small fixes towards building with other compilers #1285
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
Conversation
kilograham
commented
Feb 27, 2023
- Fix various non-GCC warnings (no effect on GCC)
- Reduce use of typeof since non GCC compilers may not support it
- Introduce PICO_C_COMPILER_IS_GNU, PICO_C_COMPILER_IS_CLANG, PICO_C_COMPILER_IS_IAR to CMake as if (CMAKE_C_COMPILER_ID STREQUAL "xxx") is a bit verbose
- Use "unified_asm" macro for all inline asm (it is "volatile __asm" on GNU with a .syntex unified)
- Use NOLOAD instead of COPY in linker scripts (arguably more correct anyway)
- Use the same style for setting _etext in all 4 linker scripts (to the beginning of .data). Clang aligns .data on a 16 byte boundary. Note ideally we'd add a new symbol __data_source, however that would break backwards compatibility with existing user linker scripts
- Use "a" for .stack, .heap sections because clang complains otherwise, and they are explicitly NOLOAD anyway
- Avoid duplicating __sev, __wfe, __wfi which Clang sometimes seems to provide as built-ins
- Add missing kitchen_sink_blocked_ram binary
- Allow build with LLVM Embedded Toolchain Form ARM v 14.0.0 (unsupported atm)
* Fix various non-GCC warnings (no effect on GCC) * Reduce use of typeof since non GCC compilers may not support it * Introduce PICO_C_COMPILER_IS_GNU, PICO_C_COMPILER_IS_CLANG, PICO_C_COMPILER_IS_IAR to CMake as if (CMAKE_C_COMPILER_ID STREQUAL "xxx") is a bit verbose * Use "unified_asm" macro for all inline asm (it is "volatile __asm" on GNU with a .syntex unified) * Use NOLOAD instead of COPY in linker scripts (arguably more correct anyway) * Use the same style for setting _etext in all 4 linker scripts (to the beginning of .data). Clang aligns .data on a 16 byte boundary. Note ideally we'd add a new symbol __data_source, however that would break backwards compatibility with existing user linker scripts * Use "a" for .stack, .heap sections because clang complains otherwise, and they are explicitly NOLOAD anyway * Avoid duplicating __sev, __wfe, __wfi which Clang sometimes seems to provide as built-ins * Add missing kitchen_sink_blocked_ram binary * Allow build with LLVM Embedded Toolchain Form ARM v 14.0.0 (unsupported atm)
| static inline int diszero(double x) { return dgetexp (x)==0; } | ||
| static inline int dispzero(double x) { return dgetsignexp(x)==0; } | ||
| static inline int dismzero(double x) { return dgetsignexp(x)==0x800; } | ||
| //static inline int dispzero(double x) { return dgetsignexp(x)==0; } |
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.
Why are these commented out?
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.
because they are unused - and that causes a warning - i guess i could have deleted them
| if (PICO_C_COMPILER_IS_GNU) | ||
| target_link_options(pico_runtime INTERFACE "--specs=nosys.specs") | ||
| elseif (PICO_C_COMPILER_IS_CLANG) | ||
| # target_link_options(pico_runtime INTERFACE "-nostdlib") |
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.
Why is this commented out?
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 not necessary; again i could have deleted i guess
Wren6991
left a comment
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.
Looks ok to me. I did notice some non-volatile asm that became volatile due to the new unified_asm macro but none of it looked worrying.
peterharperuk
left a comment
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 can't spot any issues.
yes, i figured this was harmless |