search

raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.68k stars 917 forks source link

Clang support #196

Closed foopub closed 2 months ago

foopub commented 3 years ago

I've been experimenting with clang and thought it might be useful to open a discussion here before submitting pull requests. There might be some things I'm doing wrong, as I'm by no means an expert.

To get clang working, I use set(CMAKE_SYSROOT /usr/arm-none-eabi) in my CMakeLists, as well as:

-pico_find_compiler(PICO_COMPILER_CXX clang)
+pico_find_compiler(PICO_COMPILER_CXX clang++)
...
-set(CMAKE_CXX_FLAGS_INIT "${ARM_CLANG_COMMON_FLAGS}")
+set(CMAKE_CXX_FLAGS_INIT "${ARM_CLANG_COMMON_FLAGS} -isystem /usr/arm-none-eabi/include/c++/9.3.0/arm-none-eabi -isystem /usr/arm-none-eabi/include/c++/9.3.0/")

in the pico_arm_clang file.

Both -isystem flags seem necessary even after setting sysroot. Since one might choose to download newlib to any location, maybe the toolchain preloader should just check the CMAKE_SYSROOT flag first, then fall back to searching the usual locations (i.e. /usr/) automatically, and if nothing is found, have cmake raise an error?

From here I can get a blink project to compile and assemble to an elf with just a few modifications to the sdk source. The only things clang complains about are in the assembly files. The first two issues make no difference to the binaries with gcc (I've checked the disassembly) and should probably be changed anyway.

With these changes, clang reaches the linking stage. Of course there's a whole bunch of issues with lld, but I haven't touched any of those yet.

kilograham commented 3 years ago

I've been experimenting with clang and thought it might be useful to open a discussion here before submitting pull requests. There might be some things I'm doing wrong, as I'm by no means an expert.

Yeah definitely uncharted territory, I'll probably create a feature branch anyway. I spent an hour or two playing with it way back when (hence the build files) but it wasn't high priority, just wanted to see if it it was going to be trivial (it wasn't!)

To get clang working, I use set(CMAKE_SYSROOT /usr/arm-none-eabi) in my CMakeLists, as well as:

-pico_find_compiler(PICO_COMPILER_CXX clang)
+pico_find_compiler(PICO_COMPILER_CXX clang++)
...
-set(CMAKE_CXX_FLAGS_INIT "${ARM_CLANG_COMMON_FLAGS}")
+set(CMAKE_CXX_FLAGS_INIT "${ARM_CLANG_COMMON_FLAGS} -isystem /usr/arm-none-eabi/include/c++/9.3.0/arm-none-eabi -isystem /usr/arm-none-eabi/include/c++/9.3.0/")

in the pico_arm_clang file.

Both -isystem flags seem necessary even after setting sysroot. Since one might choose to download newlib to any location, maybe the toolchain preloader should just check the CMAKE_SYSROOT flag first, then fall back to searching the usual locations (i.e. /usr/) automatically, and if nothing is found, have cmake raise an error?

Yeah, we obviously need to avoid hard coded paths (and eventually also consider non Unix OS).. are all these flags specifically to pick up newlib?

From here I can get a blink project to compile and assemble to an elf with just a few modifications to the sdk source. The only things clang complains about are in the assembly files. The first two issues make no difference to the binaries with gcc (I've checked the disassembly) and should probably be changed anyway.

* use of the unsupported `rsbs` with 0 immediate which gcc assembles to `negs` anyway, i.e.
-    rsbs r0, #0
+    negs r0, r0

Probably also fixable by .syntax unified which is probably what we should be doing

* extraneous `#`s in ldr commands, i.e.
- ldr r7,=#SIO_BASE
+ ldr r7,=SIO_BASE

oops

* use of unsigned suffix in defines... I don't like this fix because it's not in line with the coding style but I'm also not sure if any clang flags will help.
-#define PICO_STACK_SIZE 0x800u
+#define PICO_STACK_SIZE 0x800

develop branch now uses _u(0x800) instead with a different macro for assembler (actually it may not for stack size i will look)

* use of any arithmetic expressions in labels, the only examples I've encountered are in ` src/rp2_common/pico_double/double_v1_rom_shim.S`:
- adr r4,drsqrtapp-8            @ first eight table entries are never accessed because of the mantissa's leading 1
+ adr r4,drsqrtapp              @ first eight table entries are never accessed because of the mantissa's leading 1
+ subs r4,r4,#8

This seems a little stupid on Clang's part, but I can't find any other solutions.

Ugh - we should at least #ifdef this - is this a known feature or can you coerce it with parenthesis or something

With these changes, clang reaches the linking stage. Of course there's a whole bunch of issues with lld, but I haven't touched any of those yet.

yup.

foopub commented 3 years ago

are all these flags specifically to pick up newlib?

Yep - by default clang checks the gcc installation and uses its libraries, which can be seen with clang -v test.c:

#include "..." search starts here:
#include <...> search starts here:
 /bin/../lib64/gcc/x86_64-linux-musl/10.2/../../../../include/c++/10.2
 /bin/../lib64/gcc/x86_64-linux-musl/10.2/../../../../include/c++/10.2/x86_64-linux-musl
 /bin/../lib64/gcc/x86_64-linux-musl/10.2/../../../../include/c++/10.2/backward
 /usr/local/include
 /usr/include
 /usr/lib/clang/11.0.0/include

Unfortunately, it seems these flags need to be set manually when cross compiling.

Probably also fixable by .syntax unified which is probably what we should be doing

Actually most of the assembly files already use .syntax unified, and the rest don't have any syntax specified at all, so clang would assume unified. 

find . -name \*\.S -exec grep -L syntax \{\} \;
./lib/tinyusb/hw/bsp/fomu/crt0-vexriscv.S
./src/rp2_common/boot_stage2/asminclude/boot2_helpers/exit_from_boot2.S
./src/rp2_common/boot_stage2/asminclude/boot2_helpers/read_flash_sreg.S
./src/rp2_common/boot_stage2/asminclude/boot2_helpers/wait_ssi_ready.S
./src/rp2_common/boot_stage2/boot2_generic_03h.S
./src/rp2_common/boot_stage2/boot2_is25lp080.S
./src/rp2_common/boot_stage2/boot2_usb_blinky.S
./src/rp2_common/boot_stage2/boot2_w25x10cl.S
./src/rp2_common/pico_platform/include/pico/asm_helper.S

I'll see if adding .syntax unified to these causes any problems for gcc.

Ugh - we should at least #ifdef this - is this a known feature or can you coerce it with parenthesis or something

Definitely. I've tried a bunch of things but clang still complains and I can't find any information about this behaviour online. I'll try asking on the irc or mailing list.

As for the linker, doing something like ln -s /bin/lld /usr/arm-none-eabi/bin/ld.lld, adding -fuse-ld=lld -Wl,-v in CMakeFiles/[project-name].dir/link.txt and running make (with the default CMakeLists.txt) prints the lld invocation. However, even with that there's an error:

ld.lld: error: CMakeFiles/my_project.dir/home/.../pico-sdk/src/rp2_common/pico_divider/divider.S.obj:(.text.__wrap___aeabi_uidiv+0x6): unrecognized relocation R_ARM_THM_JUMP8
collect2: error: ld returned 1 exit status

which seems to be an issue with lld itself. https://lld.llvm.org/ says that lld should support armv6 "however it was built", so I can only assume it's a bug.

kilograham commented 3 years ago

adding newlib label, as we may want to build our own newlib anyway in some cases

braxtons12 commented 3 years ago

Unfortunately, I don't have anything directly useful to add to this discussion (most of my experience with cross-compiling or embedded is using Rust), but I'd like to say that this would be an amazing benefit.

I would LOVE if this can get clang support. I use clang/llvm-based tooling for all my C and C++ projects and not being able to use that when working on pico-related projects is unfortunate.

kilograham commented 3 years ago

note #511 which points out the CMSIS headers have some useful cross platform defines akin to our platform.h

SquallATF commented 2 years ago

Arm has released a pure llvm toolchain https://github.com/ARM-software/LLVM-embedded-toolchain-for-Arm

rgrr commented 1 year ago

I've put some effort into compiling the pico-examples with clang. After some fiddling most of it compiles at least. Required changes are as follows:

pico_arm_clang.cmake

option(PICO_DEOPTIMIZED_DEBUG "Build debug builds with -O0" 0)

# working setup for https://github.com/ARM-software/LLVM-embedded-toolchain-for-Arm/releases/tag/release-15.0.2
FILE(REAL_PATH $ENV{PICO_TOOLCHAIN_PATH}/../lib/clang-runtimes/armv6m_soft_nofp MY_SYSROOT)
string(CONCAT ARM_TOOLCHAIN_COMMON_FLAGS
      " --target=armv6m-none-eabi -mcpu=cortex-m0plus"
      " -Wno-duplicate-decl-specifier"
      " -Wno-unused-command-line-argument"
      " -Wno-macro-redefined"
      " -Wno-unused-function"
      " -Wno-gnu-designator"
      " -fno-exceptions -fno-rtti"
      " --sysroot=${MY_SYSROOT}"
)
message(STATUS "Toolchain path: $ENV{PICO_TOOLCHAIN_PATH}")
message(STATUS "Sysroot:        ${MY_SYSROOT}")

include(${CMAKE_CURRENT_LIST_DIR}/set_flags.cmake)

memmap_default.ld:

    .heap (COPY):
    {
        __end__ = .;
        end = __end__;
    __heap_start = .;
        *(.heap*)
        __HeapLimit = .;
    __heap_end = .;
    } > RAM

Fetch https://github.com/ARM-software/LLVM-embedded-toolchain-for-Arm/releases/tag/release-15.0.2 and unpack it somewhere.

Testing is done in pico-examples directory:

mkdir build  &&  cd build
export PICO_TOOLCHAIN_PATH=<llvm-unpack-dir>/bin
cmake -DCMAKE_BUILD_TYPE=Debug     \
      -DPICO_BOARD=pico_w -DWIFI_SSID="XXX" -DWIFI_PASSWORD="YYY"   \
      -DFREERTOS_KERNEL_PATH=$PICO_SDK_PATH/lib/tinyusb/lib/FreeRTOS-Kernel  \
      -DTEST_TCP_SERVER_IP=192.168.0.99 -DPICO_COMPILER=pico_arm_clang ..

There are some issues with inline assembler e.g. in divider.h and the RTT files, but as said most compiles.

Compile times on my machine for the whole examples are 13m25 for arm-none-eabi-gcc 12.2.1 and 5m15 for the arm-none-eabi clang.

Note that the directory structure of the other toolchain blobs may vary, so this is currently tested only with 15.0.2.

If wanted, I will create a pull request.

kilograham commented 1 year ago

cool, thanks.. yeah, coincidentally i was playing with this week, just to see the state of it, and have things compiling (and running correctly) locally ... note i'm still using release 14.0.0 as the move to picolib breaks a bunch of stuff, but yeah no need for a PR at the moment; i will push my changes to a branch at some point.

I'm certainly interested if you put any effort into fixing up the SYS calls / printf etc with picolib., it will require a (probably overdue) cleanup/up-abstraction of the way we deal with such things

rgrr commented 1 year ago

It seems, that the LLVM-embedded-toolchain guys are fiddling with the directory structure (or is it done within picolib?). Current pico_arm_clang.cmake has to be extended accordingly:

find_path(PICO_COMPILER_SYSROOT NAMES include/stdio.h 
                HINTS 
                    ${PICO_COMPILER_DIR}/../lib/clang-runtimes/arm-none-eabi/armv6m_soft_nofp
                    ${PICO_COMPILER_DIR}/../lib/clang-runtimes/armv6m_soft_nofp
         )

set(ARM_TOOLCHAIN_COMMON_FLAGS "--target=armv6m-none-eabi -mfloat-abi=soft -march=armv6m --sysroot ${PICO_COMPILER_SYSROOT}")

Then its working for both clang 15.0.2 and the current develop branch.

kilograham commented 1 year ago

yeah, the code in develop is only really tested with 14.0 release as 15.0.2 uses picolib, and that breaks some of the other integration (we will fix that later).