Turn off LTO by default on OSX #2284
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
20 tasks
Theodus
previously requested changes
Oct 17, 2017
README.md
Outdated
| @@ -70,7 +70,7 @@ If you're using `docker-machine` instead of native docker, make sure you aren't | |||
|
|
|||
| Pull the latest image as above. | |||
|
|
|||
| ```bash | |||
| ```bas | |||
There was a problem hiding this comment.
The h in bash seems to have been cut off here
Praetonus
approved these changes
Oct 17, 2017
This turns off LTO by default on MacOS. This will have performance implications for default installs, however, turning on LTO by default can lead users to have broken installs via "spookey action at a distance". Here's the scenario. For LTO to work, you need to be using the same bitcode for the linker in both the Pony runtime and the currently installed linker. Ways that having LTO can lead to a broken install: 1) install Pony 2) upgrade XCode such that the linker bitcode version changes In that scenario, you no longer can compile programs. Another scenerio (which can happen via homebrew). 1) build pony and its runtime on a machine that has XCode version X 2) install onto a machine that has an older version of XCode The pony will fail to build programs and fail with a fairly inscrutable error. At one point in the past LTO off was and I advocated for turning it on for MacOS where we felt it was safe to do. "Safe" meant, "won't bork your programs". The issue we were looking at is, LTO is still experimental in GCC but not clang. So, OSX where we use clang's linker, we felt good about making the change. What we didn't think about at the time was that this can lead to broken installs for unknowing users. With this commit, we are turning off LTO by default. Additionally... This commit adds more details about turning on LTO to the README and gives specific information about how it can break MacOS installs if you change your XCode version. This should have it safe for more experienced "build from source" MacOS users to turn on LTO and understand the consequences. No more "spooky breakage at a distance" problem. Further, when reviewing the "performance cheatsheet" on the Pony website, I realized LTO wasn't mentioned. I'll be adding information about building with LTO to the performance cheatsheet with the appropriate caveats.
|
nice catch @Theodus i forced pushed a fix. |
SeanTAllen
force-pushed
the
no-osx-lto
branch
from
08a4bd9
to
bcb280b
Compare
SeanTAllen
added
the
changelog - changed
ponylang-main
added a commit
that referenced
this pull request
Oct 17, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This turns off LTO by default on MacOS. This will have performance
implications for default installs, however, turning on LTO by default
can lead users to have broken installs via "spookey action at a
distance".
Here's the scenario. For LTO to work, you need to be using the same
bitcode for the linker in both the Pony runtime and the currently
installed linker. Ways that having LTO can lead to a broken install:
In that scenario, you no longer can compile programs.
Another scenerio (which can happen via homebrew).
The pony will fail to build programs and fail with a fairly inscrutable
error.
At one point in the past LTO off was and I advocated for turning it on
for MacOS where we felt it was safe to do. "Safe" meant, "won't bork
your programs". The issue we were looking at is, LTO is still
experimental in GCC but not clang. So, OSX where we use clang's linker,
we felt good about making the change.
What we didn't think about at the time was that this can lead to broken
installs for unknowing users. With this commit, we are turning off LTO
by default. Additionally...
This commit adds more details about turning on LTO to the README and
gives specific information about how it can break MacOS installs if you
change your XCode version. This should have it safe for more experienced
"build from source" MacOS users to turn on LTO and understand the
consequences. No more "spooky breakage at a distance" problem. Further,
when reviewing the "performance cheatsheet" on the Pony website, I
realized LTO wasn't mentioned. I'll be adding information about building
with LTO to the performance cheatsheet with the appropriate caveats.