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

Remove -fstrict-aliasing from the MinGW build to allow using MinGW 4.9.2 #1923

Merged
merged 2 commits into from
Mar 27, 2016
Merged

Conversation

bagong
Copy link
Contributor

@bagong bagong commented Mar 24, 2016

To use MinGW 4.9.2 allows to use the same toolchain for sc that is used in the Qt build. The actual underlying issue is beyond my understanding, but Googling it a bit shows that it is not a trivial issue for anybody. In view of that it might simply be more careful to avoid this optimization.
It would be nice to have a quick decision on this, because I'll make a few simplifications to the Readme if this is committed.

@crucialfelix crucialfelix added this to the 3.7.1 milestone Mar 25, 2016
@bagong
Copy link
Contributor Author

bagong commented Mar 27, 2016

We have a problem here. No reactions at all, and a release deadline approaching that I've been working towards. I need to make adjustments to the Readme once this get's committed, and I am likely to lose the opportunity if this becomes a last minute commit before the release.
While unfortunate, de facto I am the only maintainer of the Windows build. So may suggest that I commit this myself and take responsibility, if this turns out to be a problem? I will do so this evening, if I get no reactions.
Thanks for your understanding!

@crucialfelix
Copy link
Member

If it just affects the MinGW build then there should be few complaints. You've commented it well so we know what it is in the future.

If nobody else can speak against it today then go ahead and merge this evening.

@redFrik
Copy link
Contributor

redFrik commented Mar 27, 2016

hi Rainer,
many many thanks for tracking this down. by adding the -fno-strict-aliasing flag i now managed to compile sc master (well, commit 617f980 from feb) using gcc version 4.9.2 (Debian 4.9.2-10) under armv7l on both beaglebone black and raspberry pi 2. the dreaded atIdentityHash startup error is gone! before, i had to install the older gcc-4.8. hurray!

my DIY benchmarks show that these builds are a bit slower. 47 vs 44% and 0.80 vs 0.68 on the bbb before. but i didn’t do extensive testing.
so yes, the flag helps out also on arm linux with the default gcc492 compiler.
_f

27 mar 2016 kl. 11:34 skrev Rainer Schütz notifications@github.com:

We have a problem here. No reactions at all, and a release deadline approaching that I've been working towards. I need to make adjustments to the Readme once this get's committed, and I am likely to lose the opportunity if this becomes a last minute commit before the release.
While unfortunate, de facto I am the only maintainer of the Windows build. So may suggest that I commit this myself and take responsibility, if this turns out to be a problem? I will do so this evening, if I get no reactions.
Thanks for your understanding!


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub

#|
fredrikolofsson.com musicalfieldsforever.com
|#

@bagong
Copy link
Contributor Author

bagong commented Mar 27, 2016

Thanks for confirming, @redFrik! I was actually wondering whether this would help for ARM too. Currently my PR is restricted to Windows only with the MinGW brackets. Extending it to GCC in general would extend it to all Linux platforms, including the many that don't need it. If I am not mistaken, the ARM builds for the time being still require a lot of manual adjustments anyways. Would it be okay to rely on adding that specific flag manually for ARM, at least on the 3.7 branch? I have a feeling that we might be in for a positive surprise on latest master, and if not, we could still add something adjusted more precisely to the scope of this setting on master in a while?

@redFrik
Copy link
Contributor

redFrik commented Mar 27, 2016

agree. don’t add it to master/3.7 linux. manual adjustment for arm is fine now (it’s anyway such a mess of flapping compiler flags).
thanks,
_f

27 mar 2016 kl. 17:51 skrev Rainer Schütz notifications@github.com:

Thanks for confirming, @redFrik! I was actually wondering whether this would help for ARM too. Currently my PR is restricted to Windows only with the MinGW brackets. Extending it to GCC in general would extend it to all Linux platforms, including the many that don't need it. If I am not mistaken, the ARM builds for the time being still require a lot of manual adjustments anyways. Would it be okay to rely on adding that specific flag manually for ARM, at least on the 3.7 branch? I have a feeling that we might be in for a positive surprise on latest master, and if not, we could still add something adjusted more precisely to the scope of this setting on master in a while?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

#|
fredrikolofsson.com musicalfieldsforever.com
|#

@bagong bagong merged commit 74c67b3 into supercollider:3.7 Mar 27, 2016
@bagong bagong deleted the 3.7 branch March 27, 2016 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: build CMake build system os: Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants