Skip to content

Introduce a CMake flag to disable fast-path context descriptor lookup based on short manglings #39729

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

Merged
merged 1 commit into from
Oct 14, 2021

Conversation

kubamracek
Copy link
Contributor

@kubamracek kubamracek commented Oct 13, 2021

This is useful for codesize reductions. The fast-path is holding all the well-known short-mangling types referenced, so they cannot be optimized via GlobalDCE/dead-stripping.

@kubamracek kubamracek requested a review from mikeash October 13, 2021 23:54
@kubamracek
Copy link
Contributor Author

preset=stdlib_S_standalone_minimal_macho_x86_64,build,test
@swift-ci please test with toolchain and preset

2 similar comments
@kubamracek
Copy link
Contributor Author

preset=stdlib_S_standalone_minimal_macho_x86_64,build,test
@swift-ci please test with toolchain and preset

@kubamracek
Copy link
Contributor Author

preset=stdlib_S_standalone_minimal_macho_x86_64,build,test
@swift-ci please test with toolchain and preset

@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek kubamracek merged commit 99a24d4 into swiftlang:main Oct 14, 2021
@kubamracek kubamracek deleted the no-fastpath-lookups branch October 14, 2021 15:08
@davezarzycki
Copy link
Contributor

Why is it important to have the CMake variable default to TRUE but have build-script default to FALSE? This complicates building/testing for people not using build-script.

@kubamracek
Copy link
Contributor Author

build-script default to FALSE

It's only set to FALSE in one particular special configuration ("freestanding"). TRUE is the correct default for (almost) everyone.

complicates building/testing for people not using build-script

I don't follow, would you mind explaining? It's a CMake config option, so you can either keep it set to the default (TRUE), or configure it yourself if you care about this setting.

@davezarzycki
Copy link
Contributor

Ah, sorry. I didn't look at the build-script change in context and that's my fault.

I've just been bitten a few times when people tweak defaults at the build-script level but not the CMake level, which makes for confusing build failures and conversations.

Nothing to see here.

@kubamracek
Copy link
Contributor Author

👍 Cool. Different defaults in CMake and build-script are certainly bad, and we need to avoid them, agree on that.

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.

3 participants