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

Support GCC7 on Linux #2134

Merged
merged 1 commit into from
Aug 5, 2017
Merged

Support GCC7 on Linux #2134

merged 1 commit into from
Aug 5, 2017

Conversation

SeanTAllen
Copy link
Member

Our only GCC7 issue appears to that it needs to be linked against
libatomic explicitly. This commit links to libamotic explicitly on any
Linux platform regardless of compiler version.

Closes #1756

@SeanTAllen SeanTAllen requested a review from Praetonus August 5, 2017 14:13
@SeanTAllen SeanTAllen mentioned this pull request Aug 5, 2017
@SeanTAllen
Copy link
Member Author

This passes CI so, no problem for using with GCC6 on Trusty.

@lisael @russel could you verify this fixes #1756 for you?

@SeanTAllen SeanTAllen added the triggers release Major issue that when fixed, results in an "emergency" release label Aug 5, 2017
@SeanTAllen SeanTAllen mentioned this pull request Aug 5, 2017
@SeanTAllen
Copy link
Member Author

SeanTAllen commented Aug 5, 2017

I set up a Sid VM and verified both the issue and the fix. I'll have a different PR for instructions for building on Sid as I had to figure that out.

However there is then a problem with building pony programs.

Writing ./helloworld.o
Linking ./helloworld
Warning: environment variable $CC undefined, using cc as the linker
/home/vagrant/ponyc/build/release//libponyrt-pic.a(pool.o):pool.c:function pool_get.constprop.5: error: undefined reference to '__atomic_compare_exchange_16'
/home/vagrant/ponyc/build/release//libponyrt-pic.a(pool.o):pool.c:function ponyint_pool_alloc: error: undefined reference to '__atomic_compare_exchange_16'
/home/vagrant/ponyc/build/release//libponyrt-pic.a(pool.o):pool.c:function ponyint_pool_free: error: undefined reference to '__atomic_compare_exchange_16'
/home/vagrant/ponyc/build/release//libponyrt-pic.a(pool.o):pool.c:function ponyint_pool_thread_cleanup: error: undefined reference to '__atomic_compare_exchange_16'

@SeanTAllen
Copy link
Member Author

-latomic needs to be added during the linking phase as well

@SeanTAllen SeanTAllen force-pushed the gcc7-doesnt-link-atomics branch from c695f4a to 670ed74 Compare August 5, 2017 17:03
@SeanTAllen SeanTAllen added the do not merge This PR should not be merged at this time label Aug 5, 2017
@SeanTAllen SeanTAllen removed the request for review from Praetonus August 5, 2017 17:03
@SeanTAllen
Copy link
Member Author

Ok, I have fix for linux. verifying it passes our CI.

Going to check to see if fixes freebsd issue as well.

Our only GCC7 issue appears to that it needs to be linked against
libatomic explicitly. This commit links to libamotic explicitly on any
Linux platform regardless of compiler version.

Closes #1756
@SeanTAllen SeanTAllen force-pushed the gcc7-doesnt-link-atomics branch from 670ed74 to a1f32f4 Compare August 5, 2017 17:38
@SeanTAllen
Copy link
Member Author

This doesnt work on FreeBSD. I've updated the PR to be linux only. If this passes CI, its good to merge.

@SeanTAllen SeanTAllen added the changelog - added Automatically add "Added" CHANGELOG entry on merge label Aug 5, 2017
@Theodus Theodus removed the do not merge This PR should not be merged at this time label Aug 5, 2017
@Theodus Theodus merged commit 30a8648 into master Aug 5, 2017
ponylang-main added a commit that referenced this pull request Aug 5, 2017
@Theodus Theodus deleted the gcc7-doesnt-link-atomics branch August 5, 2017 18:16
@russel
Copy link
Contributor

russel commented Aug 5, 2017

Reported in on #1756 that 499a1df seems to work fine on Debian Sid with GCC 7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - added Automatically add "Added" CHANGELOG entry on merge triggers release Major issue that when fixed, results in an "emergency" release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants