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

Timer examples fails build with linker error when LTO is enabled #3

Closed
YuhanLiin opened this issue Sep 3, 2019 · 9 comments
Closed

Comments

@YuhanLiin
Copy link
Owner

When building the timer example on release mode with LTO, the build fails at link time with the following error:
note: "msp430-elf-gcc" "-mcpu=msp430" "-c" "-o" "/home/yuhan/projects/rust/embedded/msp430-quickstart/target/msp430-none-elf/release/examples/timer-4c17cb377a7ad494.timer.6hi1yd99-cgu.0.rcgu.o" "/home/yuhan/projects/rust/embedded/msp430-quickstart/target/msp430-none-elf/release/examples/timer-4c17cb377a7ad494.timer.6hi1yd99-cgu.0.rcgu.s"

note: /home/yuhan/projects/rust/embedded/msp430-quickstart/target/msp430-none-elf/release/examples/timer-4c17cb377a7ad494.timer.6hi1yd99-cgu.0.rcgu.s: Assembler messages:
/home/yuhan/projects/rust/embedded/msp430-quickstart/target/msp430-none-elf/release/examples/timer-4c17cb377a7ad494.timer.6hi1yd99-cgu.0.rcgu.s:60: Error: redefined symbol cannot be used on reloc.

The error is due to the following LLVM code (https://github.com/access-softek/msp430-clang/blob/672cd1feb4044a11ed4974f848af3cb676aafde4/src/llvm/lib/Target/MSP430/MSP430AsmPrinter.cpp#L169) generating output incompatible with our current linker scripts.

@YuhanLiin
Copy link
Owner Author

@pftbest Do you know which part of the code generated by LLVM is causing issues with our linker scripts? The error message looks to me like it came from the assembler, which runs before linking.

@pftbest
Copy link
Contributor

pftbest commented Sep 4, 2019

@YuhanLiin The issue is not specifically in linker scripts, as you noticed correctly they are only used at link time. The issue is in architecture. The way we generate interrupt vectors is different from what is currently implemented in LLVM by the code from link above. Latest LLVM code tries to automatically create a 2 byte interrupt vector for every interrupt handler it generates, and places them each in a different sections named __interrupt_vector_X. It does this because default linker scripts provided by TI expect to find interrupt vectors in this sections.

Our linker scripts expect a different layout, we have a single section where all the interrupt vectors are located. And all of them are assigned to a weak symbol pointing to default handler. You can see them here.

The assembler error is caused by the names conflict between our vectors and automatically generated ones. Even if we somehow solve the names conflict by for example renaming it somehow, the underlying issue will still be present.

I want to solve this by sending a patch to LLVM so it doesn't generate this vectors automatically for our use case, like it did in older versions of LLVM. Because we already have svd2rust generator which does it better.

cc @cr1901

@cr1901
Copy link

cr1901 commented Sep 4, 2019

@pftbest I say send the patch, but try to get in contact with the Access Softek maintainers as well and see how much they object :P.

@pftbest
Copy link
Contributor

pftbest commented Sep 7, 2019

Pushed the commit, now we wait for review

https://reviews.llvm.org/D67313

@YuhanLiin
Copy link
Owner Author

Looks like it's been merged.

@cr1901
Copy link

cr1901 commented Nov 27, 2019

Interesting... I intended to submit a patch, but didn't get around to it because of health issues.

That being said, I just updated my own copy of the Rust source to current HEAD, and I can't find any indication that @pftbest's commit is being used by Rust's copy of LLVM:

william@xubuntu-dtrain:~/Projects/rust$ git rev-parse HEAD
809e180a76ce97340bf4354ff357bc59e3ca40b2
william@xubuntu-dtrain:~/Projects/rust$ cd src/llvm-project/
william@xubuntu-dtrain:~/Projects/rust/src/llvm-project$ git rev-parse HEAD
de1a7dbf6c6b34f56e65732d45970ff27a8e84bf
william@xubuntu-dtrain:~/Projects/rust/src/llvm-project$ git rev-parse origin/rustc/9.0-2019-09-19
de1a7dbf6c6b34f56e65732d45970ff27a8e84bf
william@xubuntu-dtrain:~/Projects/rust/src/llvm-project$ git log --grep msp430_intrcc
william@xubuntu-dtrain:~/Projects/rust/src/llvm-project$

On the other hand, I keep a reference to upstream for visualization purposes (git log --all --decorate --oneline --graph):

william@xubuntu-dtrain:~/Projects/rust/src/llvm-project$ git remote get-url upstream
https://github.com/llvm/llvm-project.git
william@xubuntu-dtrain:~/Projects/rust/src/llvm-project$ git rev-parse upstream/master
98189755cd98f6e1e22e03e55b951d3ed53a5ae5
william@xubuntu-dtrain:~/Projects/rust/src/llvm-project$ git log --grep msp430_intrcc upstream/master
commit efcad77431e10b615469595eff301804a5027f18
Author: Vadzim Dambrouski <pftbest@gmail.com>
Date:   Wed Sep 25 18:58:07 2019 +0000

    [MSP430] Allow msp430_intrcc functions to not have interrupt attribute.

    Summary:
    Useful in case you want to have control over interrupt vector generation.
    For example in Rust language we have an arrangement where all unhandled
    ISR vectors gets mapped to a single default handler function. Which is
    hard to implement when LLVM tries to generate vectors on its own.

    Reviewers: asl, krisb

    Subscribers: hiraditya, JDevlieghere, awygle, llvm-commits

    Tags: #llvm

    Differential Revision: https://reviews.llvm.org/D67313

    llvm-svn: 372910
william@xubuntu-dtrain:~/Projects/rust/src/llvm-project$

@pftbest's msp430 patch made it into LLVM upstream, but didn't make it's way into Rust's copy yet. Adding his patch isn't difficult; I've done a cherry-pick into Rust's copy of LLVM before and am happy to do it again.

But before I do that (the patch needs to be merged anyway to prevent assertion failures when they are enabled), I'm curious why @YuhanLiin's issue disappeared without the patch being merged?

@YuhanLiin
Copy link
Owner Author

Did the link error go away? The last time I tried it still couldn't link. If you managed to make it go away, which nightly did you use?

@cr1901
Copy link

cr1901 commented Nov 28, 2019

Did the link error go away? The last time I tried it still couldn't link.

No, and this is currently the behavior I would expect until I get the patch into Rust. I'll do that soon. Thanks for checking!

JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 28, 2020
…chton

Bump LLVM submodule to fix LLVM assertion failure in MSP430 interrupt generation.

This PR brings in changes introduced by [this cherry-pick](rust-lang/llvm-project#37) to the Rust repository.

Nightlies downloaded from `rustup` do not appear to have llvm assertions enabled; the assertion failure [sometimes](YuhanLiin/msp430fr2355-quickstart#3) causes link errors that shouldn't occur. I couldn't find any indication of other bugs; however, it should still be fixed.
@YuhanLiin
Copy link
Owner Author

Just got the chance to try the newest nightly (1.42) and all the interrupt examples build and run fine with LTO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants