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

[Frontend] Generate an interface hash for emit-module-separately jobs #38939

Merged
merged 1 commit into from
Aug 19, 2021

Conversation

xymus
Copy link
Contributor

@xymus xymus commented Aug 18, 2021

We need emit-module jobs created for an emit-module-separately incremental build to generate the interface has for this build mode to support fine-grained incremental builds.

I used typeOpts.SkipFunctionBodies == FunctionBodySkipping::NonInlinableWithoutTypes as an heuristic to differentiate this emit-module-separately job from other kind of emit-module jobs. This flag should be passed to the compiler only in this build mode. In the future, we may want to have a flag dedicated to identifying jobs used in an incremental build from those from WMO as we move away from merge-module.

@xymus xymus requested a review from CodaFi August 18, 2021 21:09
@xymus
Copy link
Contributor Author

xymus commented Aug 18, 2021

@swift-ci Please smoke test

@xymus xymus changed the title [Frontend] Generate an interface has for emit-module-separately jobs [Frontend] Generate an interface hash for emit-module-separately jobs Aug 18, 2021
// Enable interface hash computation for primaries or emit-module-separately,
// but not in WMO, as it's only currently needed for incremental mode.
if (forPrimary ||
typeOpts.SkipFunctionBodies == FunctionBodySkipping::NonInlinableWithoutTypes) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use FrontOptions::ActionType::EmitModuleOnly here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can if we don't mind turning on interface hashing in some non-incremental builds. EmitModuleOnly is used when we skip compilation, it's already pretty common before the emit-module-separately work afaict. It looks like the stdlib is built this way via CMake and a lot of tests just do an -emit-module without an -emit-library, so they are considered EmitModuleOnly.

What do you think? Can we turn on interface hashing in more cases or should we keep limiting it only to emit-module-separately builds with the check on -experimental-skip-non-inlinable-function-bodies-without-types or some new flag?

@xymus
Copy link
Contributor Author

xymus commented Aug 19, 2021

Merging to move on with testing on the driver side. I'll be happy to revisit if we have a better way to differentiate incremental emit-module jobs.

@xymus xymus merged commit f90f06d into swiftlang:main Aug 19, 2021
brentleyjones added a commit to bazelbuild/rules_swift that referenced this pull request Oct 14, 2021
Uses the newer `-experimental-skip-non-inlinable-function-bodies-without-types` which was introduced here: swiftlang/swift#34612. This should improve LLDB usage in some cases.

When using WMO, it has two downsides in Swift 5.5, both introduced by swiftlang/swift#38939:
- The swiftmodules will have swiftdeps info embedded in them, which is only needed for incremental compilation
- Interface hashing is enabled, which again is only needed for incremental compilation

This is because the swift compiler only expects `-experimental-skip-non-inlinable-function-bodies-without-types` to be used with the new `emit-module-separately` incremental build.
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.

2 participants