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

fe310: Support compilation with clang #15176

Merged
merged 2 commits into from
Feb 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cpu/riscv_common/Makefile.include
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
CFLAGS += -Wno-pedantic
INCLUDES += -I$(RIOTCPU)/riscv_common/include

TOOLCHAINS_SUPPORTED = gnu llvm

# All variables must be defined in the CPU configuration when using the common
# `ldscripts/riscv.ld`
ifneq (,$(ROM_START_ADDR)$(RAM_START_ADDR)$(ROM_LEN)$(RAM_LEN))
Expand Down
4 changes: 2 additions & 2 deletions cpu/riscv_common/irq_arch.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ void handle_trap(uint32_t mcause)
#ifdef DEVELHELP
printf("Unhandled trap:\n");
printf(" mcause: 0x%" PRIx32 "\n", mcause);
printf(" mepc: 0x%" PRIx32 "\n", read_csr(mepc));
printf(" mtval: 0x%" PRIx32 "\n", read_csr(mtval));
printf(" mepc: 0x%lx\n", read_csr(mepc));
printf(" mtval: 0x%lx\n", read_csr(mtval));
#endif
/* Unknown trap */
core_panic(PANIC_GENERAL_ERROR, "Unhandled trap");
Expand Down
13 changes: 10 additions & 3 deletions makefiles/arch/riscv.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,15 @@ TARGET_ARCH_RISCV ?= \
TARGET_ARCH ?= $(TARGET_ARCH_RISCV)

# define build specific options
CFLAGS_CPU = -march=rv32imac -mabi=ilp32 -mcmodel=medlow -msmall-data-limit=8
CFLAGS_LINK = -nostartfiles -ffunction-sections -fdata-sections
CFLAGS_CPU = -march=rv32imac -mabi=ilp32
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
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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 ?

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

else
CFLAGS_CPU += -mcmodel=medlow -msmall-data-limit=8
endif
CFLAGS_LINK = -ffunction-sections -fdata-sections
CFLAGS_DBG ?= -g3
CFLAGS_OPT ?= -Os

Expand All @@ -42,4 +49,4 @@ LINKFLAGS += -T$(LINKER_SCRIPT)
CFLAGS += $(CFLAGS_CPU) $(CFLAGS_DBG) $(CFLAGS_OPT) $(CFLAGS_LINK)
ASFLAGS += $(CFLAGS_CPU) $(CFLAGS_DBG)
# export linker flags
LINKFLAGS += $(CFLAGS_CPU) $(CFLAGS_LINK) $(CFLAGS_DBG) $(CFLAGS_OPT) -Wl,--gc-sections -static -lgcc
LINKFLAGS += $(CFLAGS_CPU) $(CFLAGS_LINK) $(CFLAGS_DBG) $(CFLAGS_OPT) -nostartfiles -Wl,--gc-sections -static -lgcc