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

WIP, POC: Add LLVM support for AVR #16924

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

Conversation

maribu
Copy link
Member

@maribu maribu commented Sep 30, 2021

Contribution description

This PR is working towards adding LLVM support for the AVR platform.

WARNING: This is work in progress and not in a shape in which it makes sense to start a code review.

I needed to add some workarounds for features of the avrlibc that do not compile with clang. One big pain point is that clang verifies constraints of inline assembly before eliminating dead branches, while GCC does it the other way round. E.g. something like

static inline __attribute__((always_inline)) uint8_t get_foo(void)
{
    uint8_t dest;
    if (FOO < 64) {
         __asm__(
            "in r0,%[foo] \r\n"
            : [dest] "=r"(dest)
            : [foo] "I"(foo)
        );
    }
    else {
        dest = *_SFR_MEM_ADDR(FOO);
    }
    return dest;
}

will fail to compile for FOO >= 64 with LLVM, despite the inline assembly never being executed in that case. The header in <avr/wdt.h> uses this a lot, so I provided versions of wdt_enable() etc. inline - I'm not sure if those will work on al AT(X)mega MCUs supported by RIOT, though.

Another issue with LLVM is that commonly used helpers like __tmp_reg__, __SREG__, __SP_L__, __SP_H__ are not available. For now, I replaced them with magic numbers / register names.

Testing procedure

$ make BOARD=atmega328p TOOLCHAIN=llvm -C examples/hello-world compiles and links, but cannot be flashed.

Issues/PRs references

Depends on:

@github-actions github-actions bot added Area: build system Area: Build system Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: cpu Area: CPU/MCU ports Platform: AVR Platform: This PR/issue effects AVR-based platforms labels Sep 30, 2021
@maribu maribu added Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet and removed Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: build system Area: Build system Area: cpu Area: CPU/MCU ports labels Sep 30, 2021
@maribu
Copy link
Member Author

maribu commented Sep 30, 2021

@chrysn: Since you seemed to be interested in using LLVM with AVR: I dumped my current state of work here. It does at least compile and link finally, but it cannot be flashed for now...

@chrysn
Copy link
Member

chrysn commented Sep 30, 2021

Thanks, and I appreciate any WIP PRs as they make sure that it's findable -- but to clarify: I have no interest in AVR itself (last project had on it was >10a ago, not looking back), it was just that #16919 means that I can avoid the mess the C2Rust transpiler makes of C11 atomics by telling it to go through the atomic utils instead.

(Staying subscribed as in general I like having LLVM usable more widely -- it opens up cross-language-LTO opportunities.)

@benpicco
Copy link
Contributor

benpicco commented Feb 4, 2022

This one needs a rebase

@maribu
Copy link
Member Author

maribu commented Feb 4, 2022

Indeed :) But more importantly, it also depends on some infrastructure work. First, llvm/llvm-project#51557 needs to be resolved. Afterwards, the avrlibc headers need to be fixed to be compatible with clang.

Since the avrlibc was recently resurrected, this seems to be quite feasible actually :)

@maribu
Copy link
Member Author

maribu commented Mar 30, 2022

LLVM14 will contain support for the register aliases and the AVR backend has received quite some attention. Also avr-libc became active maintenance again, so I guess I can just aim for fixing things upstream first.

@stale
Copy link

stale bot commented Nov 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Nov 2, 2022
@Teufelchen1
Copy link
Contributor

@maribu Still working on this?

@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Jan 30, 2024
@maribu
Copy link
Member Author

maribu commented Jan 30, 2024

I think some work on LLVMs AVR support has been merged since the last time I looked. Maybe now is a good time to pick this up again.

There are some bugs in avrlibc that GCC doesn't trigger, that can cause a headache. Upstream is pretty much dead for avrlibc.

@Teufelchen1
Copy link
Contributor

Reminder: Soft-freeze is on the 25th.

@maribu maribu requested a review from nandojve as a code owner March 18, 2024 19:44
@github-actions github-actions bot added Area: build system Area: Build system Area: cpu Area: CPU/MCU ports and removed Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … labels Mar 18, 2024
@github-actions github-actions bot added the Area: sys Area: System label Mar 18, 2024
@maribu
Copy link
Member Author

maribu commented Mar 19, 2024

This does actually compile now on both LLVM and GCC. There are still some rough corners, but this may be on track to get in prior to the soft feature freeze.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 19, 2024
@Teufelchen1
Copy link
Contributor

WARNING: This is work in progress and not in a shape in which it makes sense to start a code review.

Can we do reviewing now?

@riot-ci
Copy link

riot-ci commented Mar 19, 2024

Murdock results

FAILED

470564e cpu/avr8_common: Add support for LLVM

Success Failures Total Runtime
1 0 9019 01m:22s

Artifacts

maribu added a commit to maribu/RIOT that referenced this pull request Mar 19, 2024
Mixing address spaces is something LLVM doesn't like (for good
reason). This re-organized the code a bit so that this does not
happen anymore, even on AVR.

Split out of RIOT-OS#16924
@maribu
Copy link
Member Author

maribu commented Mar 19, 2024

Looks like there are still compilation issues with GCC (at least the old version) that need fixing and static tests. I'm also not happy with the quality yet.

But I split out the commit that I think is ready, though: #20483

maribu added a commit to maribu/RIOT that referenced this pull request Mar 19, 2024
Mixing address spaces is something LLVM doesn't like (for good
reason). This re-organized the code a bit so that this does not
happen anymore, even on AVR.

Split out of RIOT-OS#16924
maribu added a commit to maribu/RIOT that referenced this pull request Mar 19, 2024
Mixing address spaces is something LLVM doesn't like (for good
reason). This re-organized the code a bit so that this does not
happen anymore, even on AVR.

Split out of RIOT-OS#16924
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: AVR Platform: This PR/issue effects AVR-based platforms State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants