fe310: Support compilation with clang #15176

nmeum · 2020-10-07T15:26:09Z

Contribution description

Not sure if there is any interested in this but I am personally currently working on using the HiFive1 with the clang compiler. The changes below enabled me to do so, currently doing some more manual testing but haven't found any issues so far. 91707a6 might be useful even if you are not interested in supporting the clang/llvm toolchain on the hifive1.

Testing procedure

Compile some example applications with BOARD=hifive1 and run them.

Issues/PRs references

None.

This requires -nostartfiles to be only passed to the linker, not the compiler, as it is a linker flag and passing it to the compiler causes a clang warning to be emitted. Additionally, clang does not seem to support `-mcmodel=medlow` and `-msmall-data-limit=8` but these options do not seem strictly necessary to me anyhow thus they are deactivated conditionally when using clang.

read_csr() returns an unsigned long, not a uint32_t. This causes a -Wformat warning to be emitted when compiling with clang. This commit fixes the warning by changing the format string.

benpicco

Looks good to me, but I don't have the hardware to test.
Output binary does not change with gcc though, so this should be good.

aabadie

Can you detail how you build for hifive1(b) ?
I tried the following but it's failing:

$ BUILD_IN_DOCKER=1 TOOLCHAIN=llvm make BOARD=hifive1b -C examples/default/ flash term
make: Entering directory '/work/riot/RIOT/examples/default'
Falling back to legacy riscv-none-embed toolchain
Launching build container using image "riot/riotbuild:latest".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Europe/Paris:/etc/localtime:ro' -v '/work/riot/RIOT:/data/riotbuild/riotbase:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles'      -e 'BOARD=hifive1b' -e 'TOOLCHAIN=llvm'  -w '/data/riotbuild/riotbase/examples/default/' 'riot/riotbuild:latest' make 'BOARD=hifive1b'    
Falling back to legacy riscv-none-embed toolchain
Building application "default" for "hifive1b" with MCU "fe310".

clang: error: argument unused during compilation: '-march=rv32imac' [-Werror,-Wunused-command-line-argument]
clang: error: argument unused during compilation: '-mabi=ilp32' [-Werror,-Wunused-command-line-argument]
/data/riotbuild/riotbase/Makefile.base:107: recipe for target '/data/riotbuild/riotbase/examples/default/bin/hifive1b/application_default/main.o' failed
make[1]: *** [/data/riotbuild/riotbase/examples/default/bin/hifive1b/application_default/main.o] Error 1
/data/riotbuild/riotbase/Makefile.include:603: recipe for target 'application_default.module' failed

makefiles/arch/riscv.inc.mk

nmeum · 2020-10-12T07:55:58Z

Can you detail how you build for hifive1(b) ?

$ BUILD_IN_DOCKER=1 TOOLCHAIN=llvm make BOARD=hifive1b -C examples/default/ flash term

That is more or less the command I am also using. I don't use the RIOT docker image though, looking at the error it seems to me that the clang version in the docker image doesn't have RV32 support? Can you check if riscv32 is a registered target in the llc --version output?

aabadie · 2020-10-12T08:04:16Z

Thanks @nmeum. Indeed riscv32 is not available in the Docker image. So I tried with my local llvm where it's there. But the build is still failing:

$ TOOLCHAIN=llvm make BOARD=hifive1b -C examples/default/ flash term --no-print-directory 
Falling back to legacy riscv-none-embed toolchain
Building application "default" for "hifive1b" with MCU "fe310".

error: unknown target triple 'riscv-none-embed', please use -triple or -arch

Then I tried to override TARGET_ARCH (thanks @bergzand for the hint!), but it's failing for another reason:

$ TARGET_ARCH=riscv32-unknown-elf TOOLCHAIN=llvm make BOARD=hifive1b -C examples/hello-world flash term --no-print-directory 
Falling back to legacy riscv-none-embed toolchain
Building application "hello-world" for "hifive1b" with MCU "fe310".

"make" -C /work/riot/RIOT/boards/hifive1b
"make" -C /work/riot/RIOT/core
"make" -C /work/riot/RIOT/cpu/fe310
"make" -C /work/riot/RIOT/cpu/fe310/nano
/work/riot/RIOT/cpu/fe310/nano/nanostubs.c:20:10: fatal error: 'reent.h' file not found
#include <reent.h>
         ^~~~~~~~~
1 error generated

nmeum · 2020-10-12T09:51:13Z

The reent.h header is provided by newlib nano, do you have that installed locally for rv32? I use clang in conjunction with riscv-gnu-toolchain (the GNU MCU Eclipse RISC-V thingy should also work) which provides me with newlib for rv32, unfortunately gcc is currently required for linking with the LLVM toolchain anyhow by RIOT:

RIOT/makefiles/toolchain/llvm.inc.mk

Lines 15 to 20 in 080d5ca

    
           # LLVM does have a linker, however, it is not entirely 
        
           # compatible with GCC. For instance spec files as used in 
        
           # `makefiles/libc/newlib.mk` are not supported. Therefore 
        
           # we just use GCC for now. 
        
           LINK        = $(PREFIX)gcc 
        
           LINKXX      = $(PREFIX)g++

aabadie · 2020-10-16T05:43:32Z

I use clang in conjunction with riscv-gnu-toolchain (the GNU MCU Eclipse RISC-V thingy should also work) which provides me with newlib for rv32, unfortunately gcc is currently required for linking with the LLVM toolchain anyhow by RIOT

All this should be documented somewhere, otherwise it's pretty useless for other people who don't necessarily know how to set this up (like me :) ).
Maybe start with the RISCV wiki page: https://github.com/RIOT-OS/RIOT/wiki/Family%3A-RISCV.

nmeum · 2020-10-17T16:48:38Z

All this should be documented somewhere […]

Right, I agree. However, this is by no means RISC-V specific. Most RIOT platforms I used so far require newlib to be installed, that's really independent of the utilized C compiler (i.e. not specific to the LLVM toolchain).

benpicco · 2020-10-29T23:02:22Z

Is there something blocking this still?

aabadie · 2020-10-30T08:33:20Z

Is there something blocking this still?

I still have an unaddressed comment. Did you try that PR @benpicco ?

The reent.h header is provided by newlib nano, do you have that installed locally for rv32?

Yes I have a local toolchain for riscv but it's not in a system directory. I would be nice if the build system could handle this location based on the $(TARGET_ARCH_RISCV)-gcc location. I'm also wondering why I have to specify TARGET_ARCH=riscv32-unknown-elf.

nmeum · 2020-10-30T11:06:36Z

I still have an unaddressed comment.

Which one? The one regarding documentation? I agree that clang documentation should be improved but I don't think that this is specific to this PR and maybe could be addressed separately?

Yes I have a local toolchain for riscv but it's not in a system directory. I would be nice if the build system could handle this location based on the $(TARGET_ARCH_RISCV)-gcc location.

Could you elaborate what you mean by that?

I'm also wondering why I have to specify TARGET_ARCH=riscv32-unknown-elf.

Because that's the target triplet of the toolchain you have installed while RIOT defaults to the triplet riscv-none-elf.

RIOT/makefiles/arch/riscv.inc.mk

Line 2 in d9e495f

TARGET_ARCH_RISCV ?= riscv-none-elf

I honestly think that riscv32-unknown-elf would be the better default as that's what is also used by https://github.com/riscv/riscv-gnu-toolchain but seems to me a different conclusion was reached in #14972. There is also a check for riscv64-unknown-elf but not for riscv32-unknown-elf in that Makefile.

CC: @maribu

aabadie · 2020-10-30T11:17:56Z

Which one?

This one

Could you elaborate what you mean by that?

If you need the newlib from the riscv toolchain, I think it would help to add the corresponding includes in the build. Maybe this could be done by the RIOT build system when using the llvm toolchain and will automate things for the user.

For my error above, the reent.h is indeed present in my local riscv toolchain (which is located in a custom location) but is not found when using llvm.

$ find riscv-none-gcc/8.2.0-2.2-20190521-0004/ -name reent.h
riscv-none-gcc/8.2.0-2.2-20190521-0004/riscv-none-embed/include/sys/reent.h
riscv-none-gcc/8.2.0-2.2-20190521-0004/riscv-none-embed/include/reent.h

So I'm just wondering if adding INCLUDES += -I<custom toolchain path>/riscv-none-embed/include would help when building with llvm.
How do you handle that on your side ?

nmeum · 2020-10-30T11:23:23Z

This one

Missed that, sorry! Applied your suggestion now. Should I squash it too?

How do you handle that on your side ?

The LLVM Makefile extracts include directories from the utilized GCC, (i.e. riscv32-unknown-elf-gcc in my case):

RIOT/makefiles/toolchain/llvm.inc.mk

Lines 66 to 71 in 080d5ca

    
           # Extract include directories from GCC 
        
           GCC_C_INCLUDE_DIRS   = $(call gcc_include_dirs,c) 
        
           GCC_CXX_INCLUDE_DIRS = $(call gcc_include_dirs,c++) 
        
           GCC_C_INCLUDES   = $(addprefix -isystem ,$(GCC_C_INCLUDE_DIRS)) 
        
           GCC_CXX_INCLUDES = $(addprefix -isystem ,$(GCC_CXX_INCLUDE_DIRS))

That seems to work for me. Maybe debug this part of the Makefile to figure out why it doesn't work for you?

RIOT/makefiles/toolchain/llvm.inc.mk

Lines 45 to 51 in 080d5ca

    
           gcc_include_dirs = $(realpath \ 
        
               $(shell $(PREFIX)gcc $(CFLAGS_CPU) -v -x $1 -E /dev/null 2>&1 | \ 
        
                   sed \ 
        
                   -e '1,/\#include <...> search starts here:/d' \ 
        
                   -e '/End of search list./,$$d' \ 
        
                   -e 's/^ *//')\ 
        
           )

maribu · 2020-10-30T11:42:09Z

I honestly think that riscv32-unknown-elf would be the better default as that's what is also used by https://github.com/riscv/riscv-gnu-toolchain

File a bug there ;-) From https://clang.llvm.org/docs/CrossCompilation.html#target-triple

When a parameter is not important, it can be omitted, or you can choose unknown and the defaults will be used.

As unknown works, currently the default is none. But I don't like obscuring things and hoping for the default not to change e.g. to linux. However, there is no reason against adding more possible variants of the riscv triple to check for.

aabadie · 2020-10-30T13:55:05Z

I think I'm starting to understand but things are still a bit unclear to me :)

Because that's the target triplet of the toolchain you have installed while RIOT defaults to the triplet riscv-none-elf.

No I think it works because llvm accepts this triplet (as mentioned by @maribu). But my local triplet is riscv-none-embed which comes from https://github.com/xpack-dev-tools/riscv-none-embed-gcc-xpack ("This distribution plans to follow the official SiFive releases.") and the RIOT build system fails to find the includes if I set TARGET_ARCH=riscv32-unknown-elf.
On the other hand, the RIOT build system finds the correct includes with riscv-none-embed but then the problem is that llvm doesn't accept this triplet name.

So with the current default RIOT toolchain setup (the one in Docker which is also riscv-none-embed), it's not possible to support llvm without a custom setup. One would have to build his riscv toolchain directly from https://github.com/riscv/riscv-gnu-toolchain to make it work, like you did. This is why it's important to have this documented somewhere (the wiki at least).

nmeum · 2020-10-30T14:52:31Z

On the other hand, the RIOT build system finds the correct includes with riscv-none-embed but then the problem is that llvm doesn't accept this triplet name.

Ah! That make sense because the triplet is passed to clang too using the -target flag:

RIOT/makefiles/toolchain/llvm.inc.mk

Lines 58 to 60 in 080d5ca

    
           # Tell clang to cross compile 
        
           CFLAGS     += -target $(TARGET_ARCH) 
        
           CXXFLAGS   += -target $(TARGET_ARCH)

Do you get an error message from clang with -target riscv-none-embed? I don't get any error message from clang with the riscv32-unknown-elf triplet.

So with the current default RIOT toolchain setup (the one in Docker which is also riscv-none-embed), it's not possible to support llvm without a custom setup.

Not sure if I missing something here, but if this is just about the triplet name I would rather vouch for adjusting the default RIOT RISC-V triplet to make it work by with clang if clang does not support the riscv-none-embed triplet?

aabadie · 2020-10-30T15:17:42Z

Do you get an error message from clang with -target riscv-none-embed?

Yes: error: unknown target triple 'riscv-none-embed', please use -triple or -arch. So that's why I think it's a clang issue, as mentioned by @maribu in #15176 (comment)

I would rather vouch for adjusting the default RIOT RISC-V triplet to make it work by with clang if clang does not support the riscv-none-embed triplet?

That could be an option but that would need an update of riotdocker.

nmeum · 2020-11-05T12:15:16Z

File a bug there ;-) From https://clang.llvm.org/docs/CrossCompilation.html#target-triple

@maribu Could you briefly explain what you mean by that? I started using the Alpine packed gcc-riscv-none-elf now (which was also created by you I believe 😉), instead of building my own cross-compilers and with gcc-riscv-none-elf I can indeed also reproduce the issue described by @aabadie.

If I make the following changes to the build system, compilation with clang works fine again:

diff --git a/makefiles/toolchain/llvm.inc.mk b/makefiles/toolchain/llvm.inc.mk
index 9898f10f8..182d71ee6 100644
--- a/makefiles/toolchain/llvm.inc.mk
+++ b/makefiles/toolchain/llvm.inc.mk
@@ -56,8 +56,8 @@ ifneq (,$(TARGET_ARCH))
   endif
 
   # Tell clang to cross compile
-  CFLAGS     += -target $(TARGET_ARCH)
-  CXXFLAGS   += -target $(TARGET_ARCH)
+  CFLAGS     += -target riscv32-unknown-elf
+  CXXFLAGS   += -target riscv32-unknown-elf
   # We currently don't use LLVM for linking (see comment above).
   # LINKFLAGS  += -target $(TARGET_ARCH)

So why does the RIOT docker image and the Alpine package use a triplet which is not supported by clang? If RIOT (and Alpine) just default to riscv32-unknown-elf everything would work fine from my perspective.

maribu · 2020-11-05T18:55:33Z

@nmeum: I can also reproduce the issue. I don't think there is a need to have the GCC target triple and the clang target triple identical, e.g. the legacy toolchain with riscv-none-embed might still be used and that triple is not going to get support from clang.

However: I suggest to use riscv32-none-elf over riscv32-unknown-elf, as IMO unknown is just obfuscating what is actually used.

The reason that for GCC I chose riscv-none-elf over riscv32-none-elf is that the triplet is there not used as parameter, but part of the file name. And that toolchain is compiled with multilib support and can emit code for both 32 bit and 64 bit flavors of RISC-V, so I figured riscv is less misleading than riscv32 or riscv64 there.

nmeum · 2020-11-05T19:42:01Z

I don't think there is a need to have the GCC target triple and the clang target triple identical

I would personally want to avoid introducing yet another Makefile variable for configuring the compilation target, just makes the current setup even more complicated IMHO.

However: I suggest to use riscv32-none-elf over riscv32-unknown-elf […]

Fine with me, if the former is supported by clang too.

[…] as IMO unknown is just obfuscating what is actually used.

I doubt that anyone sees through the semantic meaning of these triplets. I would personally just stick to what is used elsewhere, for instance in https://github.com/riscv/riscv-gnu-toolchain.

nmeum · 2021-02-10T08:59:57Z

In the meantime, the affected files have moved to cpu/riscv_common

Thanks for the heads up, rebased against master.

bergzand · 2021-02-10T09:19:45Z

As for ef5aba3, I think this should be fixed in the read_csr() function and friends. Those should IMHO return an uint32_t (or to be more portable an uword_t). Something we can fix in a follow up. What do you think?

nmeum · 2021-02-10T09:27:16Z

Maybe better to fix this separately, changing the type might require changes elsewhere in the code base too.

bergzand · 2021-02-10T09:33:22Z

Maybe better to fix this separately, changing the type might require changes elsewhere in the code base too.

Fully agree, don't want to make this PR bigger than it has to be, just wanted to have your opinion on this.

maribu

ACK

maribu · 2021-02-10T09:48:24Z

@aabadie: Can you have a look here?

aabadie · 2021-02-10T17:06:05Z

Can you have a look here?

Now I end up with another build failure:

clang: error: argument unused during compilation: '-nostartfiles' [-Werror,-Wunused-command-line-argument]
make[1]: *** [/work/riot/RIOT/Makefile.base:107: /work/riot/RIOT/examples/hello-world/bin/hifive1b/application_hello-world/main.o] Error 1

What is supposed to be done to get a successful build with clang ? Anyone else tried ?

miri64 · 2021-02-10T18:20:13Z

Sorry for joining in that late into the discussion, but should we maybe add an fe310 board to

RIOT/.murdock

Line 7 in d7f3e92

    
           #: ${TEST_BOARDS_LLVM_COMPILE:="iotlab-m3 native nrf52dk mulle nucleo-f401re samr21-xpro slstk3402a"}

so once gets merged we also compile test the LLVM config?

maribu · 2021-02-10T19:40:49Z

Sorry for joining in that late into the discussion, but should we maybe add an fe310 board to

RIOT/.murdock

Line 7 in d7f3e92

#: ${TEST_BOARDS_LLVM_COMPILE:="iotlab-m3 native nrf52dk mulle nucleo-f401re samr21-xpro slstk3402a"}

so once gets merged we also compile test the LLVM config?

I thought the same :-D

miri64 · 2021-02-10T19:47:14Z

I thought the same :-D

Oh sorry, just looked at the title and the code, but not the discussion ^^"

maribu · 2021-02-10T20:00:24Z

Can you have a look here?

Now I end up with another build failure:
clang: error: argument unused during compilation: '-nostartfiles' [-Werror,-Wunused-command-line-argument]
make[1]: *** [/work/riot/RIOT/Makefile.base:107: /work/riot/RIOT/examples/hello-world/bin/hifive1b/application_hello-world/main.o] Error 1
What is supposed to be done to get a successful build with clang ? Anyone else tried ?

This is a regression, -nostartfiles was previously not part of CFLAGS_LINK and was added upstream outside of this PR (I think with during the factoring out of the common riscv code). I can reproduce the issue with clang10.

My local rebuild of all toolchains is still ongoing, so I cannot test (due to missing c lib). But I think this should fix the issue:

diff --git a/makefiles/arch/riscv.inc.mk b/makefiles/arch/riscv.inc.mk
index 354a920b38..02c79613e9 100644
--- a/makefiles/arch/riscv.inc.mk
+++ b/makefiles/arch/riscv.inc.mk
@@ -35,10 +35,12 @@ ifeq ($(TOOLCHAIN),llvm)
   # Always use riscv32-none-elf as target triple for clang, as some
   # autodetected gcc target triples are incompatible with clang
   TARGET_ARCH := riscv32-none-elf
+  CFLAGS_LINK :=
 else
   CFLAGS_CPU += -mcmodel=medlow -msmall-data-limit=8
+  CFLAGS_LINK := -nostartfiles
 endif
-CFLAGS_LINK  = -nostartfiles -ffunction-sections -fdata-sections
+CFLAGS_LINK += -ffunction-sections -fdata-sections
 CFLAGS_DBG  ?= -g3
 CFLAGS_OPT  ?= -Os

nmeum · 2021-02-10T20:01:17Z

Can you have a look here?

Now I end up with another build failure:

clang: error: argument unused during compilation: '-nostartfiles' [-Werror,-Wunused-command-line-argument]
make[1]: *** [/work/riot/RIOT/Makefile.base:107: /work/riot/RIOT/examples/hello-world/bin/hifive1b/application_hello-world/main.o] Error 1

Oops, I think I made a minor mistake during rebasing today. Might have not properly rebuild my test application. Will check tomorrow, sorry for the inconvenience.

maribu · 2021-02-10T20:03:39Z

Maybe you could also enable Murdock compilation of the hifive1b right away? That way we can be sure that clang version in our CI is compatible with all flags provided.

nmeum · 2021-02-10T20:08:52Z

Not too familiar with murdock but I will look into it.

aabadie · 2021-02-10T20:18:41Z

Maybe you could also enable Murdock compilation of the hifive1b right away?

I doubt that will work right away on Murdock. If I build in Docker, I get this:

$ BUILD_IN_DOCKER=1 TOOLCHAIN=llvm make BOARD=hifive1b -C examples/hello-world
make: Entering directory '/work/riot/RIOT/examples/hello-world'
Launching build container using image "riot/riotbuild:latest".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Europe/Paris:/etc/localtime:ro' -v '/work/riot/RIOT:/data/riotbuild/riotbase:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles'      -e 'BOARD=hifive1b' -e 'TOOLCHAIN=llvm'  -w '/data/riotbuild/riotbase/examples/hello-world/' 'riot/riotbuild:latest' make 'BOARD=hifive1b'    
Building application "hello-world" for "hifive1b" with MCU "fe310".

/data/riotbuild/riotbase/examples/hello-world/main.c:22:10: fatal error: 'stdio.h' file not found
#include <stdio.h>
         ^~~~~~~~~
1 error generated.
/data/riotbuild/riotbase/Makefile.base:107: recipe for target '/data/riotbuild/riotbase/examples/hello-world/bin/hifive1b/application_hello-world/main.o' failed
make[1]: *** [/data/riotbuild/riotbase/examples/hello-world/bin/hifive1b/application_hello-world/main.o] Error 1

And I have the same issue when using my local toolchain (and after applying your patch above).

benpicco · 2021-02-10T20:24:44Z

Not too familiar with murdock but I will look into it.

see #15176 (comment)

maribu · 2021-02-10T20:29:32Z

Maybe you could also enable Murdock compilation of the hifive1b right away? That way we can be sure that clang version in our CI is compatible with all flags provided.

I just remembered that clang is disabled in the CI due to compatibility issues since upgrading clang in the container anyway :-/

nmeum · 2021-02-11T14:58:02Z

Fixed the problem with -nostartupfiles. The other issue raised by @aabadie in #15176 (comment) is related to the change proposed by @maribu in #15176 (comment) this changes TARGET_ARCH and also seems to cause PREFIX to change, if the PREFIX of your local gcc installation is not riscv32-none-elf then the RIOT clang setup will not be able to extract toolchain flags from the GCC installation, i.e. the following code no longer works:

RIOT/makefiles/toolchain/llvm.inc.mk

Lines 45 to 51 in 080d5ca

    
           gcc_include_dirs = $(realpath \ 
        
               $(shell $(PREFIX)gcc $(CFLAGS_CPU) -v -x $1 -E /dev/null 2>&1 | \ 
        
                   sed \ 
        
                   -e '1,/\#include <...> search starts here:/d' \ 
        
                   -e '/End of search list./,$$d' \ 
        
                   -e 's/^ *//')\ 
        
           )

For the record, PREFIX is set here:

RIOT/Makefile.include

Lines 344 to 346 in d1deb6f

    
           # default toolchain prefix, defaults to target triple followed by a dash, you 
        
           # will most likely not need to touch this. 
        
           export PREFIX ?= $(if $(TARGET_ARCH),$(TARGET_ARCH)-)

I already laid out my preferred way to fix the triplet mismatch problems in #15176 (comment). Unfortunately, another problem appread with the merge of #15011: the picolib version my RISC-V cross toolchain is using does not work with clang presently.

tl;dr This requires a recent picolib version and the triplet mismatch issue needs a different fix I guess.

makefiles/arch/riscv.inc.mk

nmeum · 2021-02-12T09:44:33Z

Rebased. The following command:

$ FEATURES_REQUIRED=newlib TOOLCHAIN=llvm BOARD=hifive1 make -C examples/hello-world/

works for me again now with clang10. Thanks @maribu!

maribu

ACK. (And this time I double checked that no regressions have popped up due to upstream changes)

aabadie · 2021-02-12T16:34:43Z

The following command:

Now you don't even need to add FEATURES_REQUIRED=newlib to the command line. I don't know by what magic but now it builds fine for me as well (clang 11):

in Docker (my image uses https://github.com/RIOT-OS/riotdocker/pull/131):

$ BUILD_IN_DOCKER=1 TOOLCHAIN=llvm BOARD=hifive1b make -C examples/hello-world --no-print-directory 
Launching build container using image "riot/riotbuild:latest".
docker run --rm --tty --user $(id -u) -v '/usr/share/zoneinfo/Europe/Paris:/etc/localtime:ro' -v '/work/riot/RIOT:/data/riotbuild/riotbase:delegated' -e 'RIOTBASE=/data/riotbuild/riotbase' -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' -e 'BUILD_DIR=/data/riotbuild/riotbase/build' -e 'RIOTPROJECT=/data/riotbuild/riotbase' -e 'RIOTCPU=/data/riotbuild/riotbase/cpu' -e 'RIOTBOARD=/data/riotbuild/riotbase/boards' -e 'RIOTMAKE=/data/riotbuild/riotbase/makefiles'      -e 'BOARD=hifive1b' -e 'TOOLCHAIN=llvm'  -w '/data/riotbuild/riotbase/examples/hello-world/' 'riot/riotbuild:latest' make     
Building application "hello-world" for "hifive1b" with MCU "fe310".

"make" -C /data/riotbuild/riotbase/boards/hifive1b
"make" -C /data/riotbuild/riotbase/core
"make" -C /data/riotbuild/riotbase/cpu/fe310
"make" -C /data/riotbuild/riotbase/cpu/fe310/periph
"make" -C /data/riotbuild/riotbase/cpu/fe310/vendor
"make" -C /data/riotbuild/riotbase/cpu/riscv_common
"make" -C /data/riotbuild/riotbase/cpu/riscv_common/periph
"make" -C /data/riotbuild/riotbase/drivers
"make" -C /data/riotbuild/riotbase/drivers/periph_common
"make" -C /data/riotbuild/riotbase/sys
"make" -C /data/riotbuild/riotbase/sys/auto_init
"make" -C /data/riotbuild/riotbase/sys/malloc_thread_safe
"make" -C /data/riotbuild/riotbase/sys/newlib_syscalls_default
"make" -C /data/riotbuild/riotbase/sys/stdio_uart
   text	   data	    bss	    dec	    hex	filename
   7800	    108	   2248	  10156	   27ac	/data/riotbuild/riotbase/examples/hello-world/bin/hifive1b/hello-world.elf

on the host (Ubuntu):

$ TOOLCHAIN=llvm BOARD=hifive1b make -C examples/hello-world --no-print-directory  flash term
Building application "hello-world" for "hifive1b" with MCU "fe310".

"make" -C /work/riot/RIOT/boards/hifive1b
"make" -C /work/riot/RIOT/core
"make" -C /work/riot/RIOT/cpu/fe310
"make" -C /work/riot/RIOT/cpu/fe310/periph
"make" -C /work/riot/RIOT/cpu/fe310/vendor
"make" -C /work/riot/RIOT/cpu/riscv_common
"make" -C /work/riot/RIOT/cpu/riscv_common/periph
"make" -C /work/riot/RIOT/drivers
"make" -C /work/riot/RIOT/drivers/periph_common
"make" -C /work/riot/RIOT/sys
"make" -C /work/riot/RIOT/sys/auto_init
"make" -C /work/riot/RIOT/sys/malloc_thread_safe
"make" -C /work/riot/RIOT/sys/newlib_syscalls_default
"make" -C /work/riot/RIOT/sys/stdio_uart
   text	   data	    bss	    dec	    hex	filename
   7792	    108	   2248	  10148	   27a4	/work/riot/RIOT/examples/hello-world/bin/hifive1b/hello-world.elf
/work/riot/RIOT/dist/tools/jlink/jlink.sh flash /work/riot/RIOT/examples/hello-world/bin/hifive1b/hello-world.bin
### Flashing Target ###
### Flashing at base address 0x20010000 with offset 0 ###
SEGGER J-Link Commander V6.94b (Compiled Jan 26 2021 18:05:49)
DLL version V6.94b, compiled Jan 26 2021 18:05:34

J-Link Commander will now exit on Error

J-Link Command File read successfully.
Processing script file...

J-Link connection not established yet but required for command.
Connecting to J-Link via USB...O.K.
Firmware: J-Link OB-K22-SiFive compiled Jan 18 2021 09:05:42
Hardware version: V1.00
S/N: 979001370
VTref=3.300V
Target connection not established yet but required for command.
Device "FE310" selected.


Connecting to target via JTAG
ConfigTargetSettings() start
ConfigTargetSettings() end
TotalIRLen = 5, IRPrint = 0x01
JTAG chain detection found 1 devices:
 #0 Id: 0x20000913, IRLen: 05, Unknown device
Debug architecture:
  RISC-V debug: 0.13
  AddrBits: 7
  DataBits: 32
  IdleClks: 5
Memory access:
  Via system bus: No
  Via ProgBuf: Yes (16 ProgBuf entries)
DataBuf: 1 entries
  autoexec[0] implemented: Yes
Detected: RV32 core
CSR access via abs. commands: No
Temp. halted CPU for NumHWBP detection
HW instruction/data BPs: 8
Support set/clr BPs while running: No
HW data BPs trigger before execution of inst
RISC-V identified.
Halting CPU for downloading file.
Downloading file [/work/riot/RIOT/examples/hello-world/bin/hifive1b/hello-world.bin]...
Comparing flash   [100%] Done.
J-Link: Flash download: Bank 0 @ 0x20000000: Skipped. Contents already match
O.K.

Reset delay: 0 ms
Reset type Normal: Resets core & peripherals using <ndmreset> bit in <dmcontrol> debug register.
RISC-V: Performing reset via <ndmreset>



Script processing completed.

/work/riot/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200"  
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2021-02-12 17:34:21,689 # Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Bench Clock Reset Complete
2021-02-12 17:34:22,693 # 
2021-02-12 17:34:22,715 # ATE0-->ATE0
2021-02-12 17:34:22,732 # OK
2021-02-12 17:34:22,889 # AT+BLEINIT=0-->OK
2021-02-12 17:34:23,045 # AT+CWMODE=0-->OK
2021-02-12 17:34:23,045 # 
2021-02-12 17:34:23,054 # main(): This is RIOT! (Version: 2021.04-devel-539-g5b15dc-review_riscv_llvm)
2021-02-12 17:34:23,056 # Hello World!
2021-02-12 17:34:23,060 # You are running RIOT on a(n) hifive1b board.
2021-02-12 17:34:23,063 # This board features a(n) fe310 MCU.
2021-02-12 17:34:24,084 # Exiting Pyterm

aabadie · 2021-02-12T16:38:04Z

makefiles/arch/riscv.inc.mk

+ifeq ($(TOOLCHAIN),llvm)
+  # Always use riscv32-none-elf as target triple for clang, as some
+  # autodetected gcc target triples are incompatible with clang
+  TARGET_ARCH_LLVM := riscv32-none-elf


Suggested change

TARGET_ARCH_LLVM := riscv32-none-elf

TARGET_ARCH_LLVM ?= riscv32-none-elf

With this, you can override this value with a different triple.

This flexibility is luckily not needed for LLVM. Technically, riscv32-none-elf is the correct target triplet for 32 bit embedded (bare metal) RISC-V targets. Differently to GCC, the target triplet is just an argument rather than a prefix to the binaries and managed upstream, so unless one manually patches clang to only work with incorrect target triples, nobody will need to change this.

I was thinking of the riscv64-none-elf triplet available in the Debian/Ubuntu package manager. On Debian/Ubuntu, that would need picolibc but IIUC, it might be possible to use llvm also with picolibc ?

I was thinking of the riscv64-none-elf triplet available in the Debian/Ubuntu package manager.

No, the libraries are resolved using the triple in TARGET_ARCH, which can be distinct from TARGET_ARCH_LLVM. The issue is that riscv64-none-elf is the correct triple for the 64 bit RISC-V, but we only support 32 bit so far. A riscv64-none-elf multilib GCC toolchain would support both 32 and 64 bit flavors, but clang wants the correct triple.

E.g. when the c lib is installed as riscv-none-embed, this is a completely invalid target triple. But splitting the target triple used to resolve the GCC toolchain and c library from the target triple passed to LLVM, we overcome the issues.

IIUC, it might be possible to use llvm also with picolibc ?

Yes, starting with clang11 it is possible. clang10 doesn't understand the -specs= parameter.

Thanks for the clarifications @maribu. Makes sense to me now.

aabadie

My latest tests were successful and the code changes look good to me.

ACK

nmeum requested review from aabadie, bergzand and kaspar030 as code owners October 7, 2020 15:26

fe310: Fix debug format strings

5b15dc8

read_csr() returns an unsigned long, not a uint32_t. This causes a -Wformat warning to be emitted when compiling with clang. This commit fixes the warning by changing the format string.

benpicco approved these changes Oct 11, 2020

View reviewed changes

aabadie requested changes Oct 12, 2020

View reviewed changes

makefiles/arch/riscv.inc.mk Outdated Show resolved Hide resolved

maribu approved these changes Feb 10, 2021

View reviewed changes

maribu mentioned this pull request Feb 11, 2021

build system: support wrong target triples with LLVM #15986

Merged

maribu reviewed Feb 11, 2021

View reviewed changes

makefiles/arch/riscv.inc.mk Outdated Show resolved Hide resolved

nmeum force-pushed the pr/hifive1-llvm branch from 087ac0d to 5b15dc8 Compare February 12, 2021 09:43

maribu approved these changes Feb 12, 2021

View reviewed changes

aabadie reviewed Feb 12, 2021

View reviewed changes

aabadie approved these changes Feb 13, 2021

View reviewed changes

aabadie merged commit 54dbc55 into RIOT-OS:master Feb 13, 2021

kaspar030 added this to the Release 2021.04 milestone Apr 23, 2021

	TARGET_ARCH_LLVM := riscv32-none-elf
	TARGET_ARCH_LLVM ?= riscv32-none-elf

fe310: Support compilation with clang #15176

fe310: Support compilation with clang #15176

Conversation

nmeum commented Oct 7, 2020

Contribution description

Testing procedure

Issues/PRs references

benpicco left a comment

Choose a reason for hiding this comment

aabadie left a comment

Choose a reason for hiding this comment

nmeum commented Oct 12, 2020 • edited Loading

aabadie commented Oct 12, 2020

nmeum commented Oct 12, 2020 • edited Loading

aabadie commented Oct 16, 2020

nmeum commented Oct 17, 2020

benpicco commented Oct 29, 2020

aabadie commented Oct 30, 2020

nmeum commented Oct 30, 2020 • edited Loading

aabadie commented Oct 30, 2020

nmeum commented Oct 30, 2020 • edited Loading

maribu commented Oct 30, 2020

aabadie commented Oct 30, 2020

nmeum commented Oct 30, 2020

aabadie commented Oct 30, 2020

nmeum commented Nov 5, 2020 • edited Loading

maribu commented Nov 5, 2020

nmeum commented Nov 5, 2020

nmeum commented Feb 10, 2021

bergzand commented Feb 10, 2021

nmeum commented Feb 10, 2021

bergzand commented Feb 10, 2021

maribu left a comment

Choose a reason for hiding this comment

maribu commented Feb 10, 2021

aabadie commented Feb 10, 2021

miri64 commented Feb 10, 2021

maribu commented Feb 10, 2021

miri64 commented Feb 10, 2021

maribu commented Feb 10, 2021

nmeum commented Feb 10, 2021

maribu commented Feb 10, 2021

nmeum commented Feb 10, 2021

aabadie commented Feb 10, 2021

benpicco commented Feb 10, 2021

maribu commented Feb 10, 2021

nmeum commented Feb 11, 2021 • edited Loading

nmeum commented Feb 12, 2021 • edited Loading

maribu left a comment

Choose a reason for hiding this comment

aabadie commented Feb 12, 2021

aabadie Feb 12, 2021

Choose a reason for hiding this comment

maribu Feb 12, 2021

Choose a reason for hiding this comment

aabadie Feb 13, 2021

Choose a reason for hiding this comment

maribu Feb 13, 2021

Choose a reason for hiding this comment

aabadie Feb 13, 2021

Choose a reason for hiding this comment

aabadie left a comment

Choose a reason for hiding this comment

nmeum commented Oct 12, 2020 •

edited

Loading

nmeum commented Oct 12, 2020 •

edited

Loading

nmeum commented Oct 30, 2020 •

edited

Loading

nmeum commented Oct 30, 2020 •

edited

Loading

nmeum commented Nov 5, 2020 •

edited

Loading

nmeum commented Feb 11, 2021 •

edited

Loading

nmeum commented Feb 12, 2021 •

edited

Loading