Skip to content

Conversation

@aykevl
Copy link
Member

@aykevl aykevl commented Apr 1, 2020

This commit also adds a bit of version independence, in particular for external commands. It also adds the LLVM version to the tinygo version command, which might help while debugging.

TODO: make TinyGo work with LLVM version 9 too, for Fedora (and perhaps other distributions). I'm making a PR now so that I can see whether the Windows build will work. done

@aykevl aykevl force-pushed the llvm10 branch 8 times, most recently from 0ec2d34 to b526cac Compare April 2, 2020 14:39
@aykevl
Copy link
Member Author

aykevl commented Apr 2, 2020

Looks like I'm hitting this problem: https://reviews.llvm.org/D66324#1680231

@aykevl
Copy link
Member Author

aykevl commented Apr 2, 2020

I'm afraid fixing this will require some redesigning of how certain tools (cc1 mainly) are invoked.

@aykevl aykevl force-pushed the llvm10 branch 4 times, most recently from 9545d9a to a2320ed Compare April 2, 2020 21:06
This should make it much easier to update existing tests.
@aykevl aykevl force-pushed the llvm10 branch 3 times, most recently from 390bc19 to 7505707 Compare April 3, 2020 11:28
This commit also adds a bit of version independence, in particular for
external commands. It also adds the LLVM version to the `tinygo version`
command, which might help while debugging.
See comment in the commit for details. It works around a bug that's been
reported here: https://bugs.llvm.org/show_bug.cgi?id=45336

This is a separate commit so it can easily be reverted if/when this
patch is backported to the LLVM 10 stable branch.
@aykevl aykevl marked this pull request as ready for review April 3, 2020 19:43
@aykevl
Copy link
Member Author

aykevl commented Apr 3, 2020

This PR is now ready for review. I've intentionally kept the last commit separate so that it can be reverted if/when the lld bug is fixed upstream.

This PR also adds testing for LLVM 9, to make sure it won't break this older release for Linux distributions.

@QuLogic

@deadprogram
Copy link
Member

I was able to install LLVM 10 using the apt repo, and compile several code examples successfully using this branch.

When I first tried under this branch, I did get

error: failed to link /tmp/tinygo339089044/main: none of these commands were found in your $PATH: ld.lld-10 ld.lld

I probably did not have the proper symlink setup for LLVM 9? Now that I have LLVM 10 installed I cannot backtest LLVM 9 very easily.

That said, from my POV I think we should merge this now.

@aykevl
Copy link
Member Author

aykevl commented Apr 6, 2020

error: failed to link /tmp/tinygo339089044/main: none of these commands were found in your $PATH: ld.lld-10 ld.lld

Did you install ld.lld-10?

This is what I normally use:

sudo apt-get install llvm-10-dev libclang-10-dev lld-10 clang-10

Also, I currently have both LLVM 9 and LLVM 10 installed, from apt.llvm.org. No symlinks necessary, it should always use the correct version (depending on which branch you're on).

@deadprogram
Copy link
Member

The error occurred before I had installed llvm10, but already had llvm9 installed.

@aykevl
Copy link
Member Author

aykevl commented Apr 6, 2020

Ah I understand now.

@QuLogic
Copy link
Contributor

QuLogic commented Apr 7, 2020

Ah cool, did you run into any issues with ARM sections? IIRC, that's what I got before stopping to focus on Go 1.14.

I will try this out myself, but probably won't be able to get to it for a few days.

@aykevl
Copy link
Member Author

aykevl commented Apr 7, 2020

I haven't seen anything about ARM sections, no. I did find a different linker bug, though (that I've worked around now).

@aykevl aykevl mentioned this pull request Apr 7, 2020
@deadprogram
Copy link
Member

Been testing this branch, and seems ready to merge. Last call for feedback!

@deadprogram
Copy link
Member

OK, let's merge!

@deadprogram deadprogram merged commit 9e453a5 into dev Apr 9, 2020
@deadprogram deadprogram deleted the llvm10 branch April 9, 2020 18:40
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

Successfully merging this pull request may close these issues.

4 participants