Skip to content

Infer imports for linking #16317

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Infer imports for linking #16317

wants to merge 3 commits into from

Conversation

jrose-apple
Copy link
Contributor

@jrose-apple jrose-apple commented May 2, 2018 •
edited
Loading

Alternate version of #16299. Checks what modules are being used in inlinable code, and adds them to the "usable from inline" list for autolinking purposes.

This isn't quite as clean as the version Slava had, but that one started affecting name lookup, and so I went for a more heavy-handed version.

rdar://problem/39338239

slavapestov and others added 3 commits May 2, 2018 13:34
@slavapestov @jrose-apple
Sema: Infer '@_usableFromInline' imports by looking at references fro…
a89e3c7 
…m inlinable functions

Completes the fix for <rdar://problem/39338239>.
@jrose-apple
AST: Improve doc comment for ModuleDecl::getImportedModulesForLinking()
65b5d54
@jrose-apple
Fix '@_usableFromInline import' inference interaction with '@_exporte…
2f112fb 
…d import'

A module might be visible indirectly, because it's @_exported by one
of the modules we import (either in Swift, or with the C module map
equivalent). In this case, we have to add it to our direct list of
imports so that it can be autolinked.

This isn't as clean as the previous version because now there's a
/second/ list of imports in a SourceFile, but trying to use the
regular list of imports to also track modules used from inlinable
functions resulted in name lookup /actually looking into these modules/.

The other possibility I considered (suggested by Slava) was to make
UsableFromInline imports not participate in lookup at all, but that
seemed trickier to get right.

More rdar://problem/39338239.
@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test source compatibility

@jrose-apple
Copy link
Contributor Author

@swift-ci Please smoke test compiler performance

@jrose-apple jrose-apple requested a review from graydon May 2, 2018 22:40
@jrose-apple
Copy link
Contributor Author

#16211 added the notion of "usable from inline imports", by the way, if you're looking for the context.

graydon
graydon approved these changes May 2, 2018
View reviewed changes
Copy link
Contributor

@graydon graydon left a comment

Choose a reason for hiding this comment

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

Discussed on chat, this looks sensible to me (as far as I can understand it and its backstory).

@swift-ci
Copy link
Contributor

swift-ci commented May 3, 2018

Build comment file:

Summary for master smoketest

Unexpected test results, excluded stats for ReactiveCocoa

No regressions above thresholds

Debug

debug brief

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
name old new delta delta_pct
LLVM.NumLLVMBytesOutput 41,051,794 41,093,962 42,168 0.1%
time.swift-driver.wall 67.8s 67.2s -605.4ms -0.89%

debug detailed

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
name old new delta delta_pct
AST.NumImportedExternalDefinitions 84,556 84,556 0 0.0%
AST.NumLoadedModules 12,956 12,956 0 0.0%
AST.NumTotalClangImportedEntities 256,453 256,453 0 0.0%
AST.NumUsedConformances 8,773 8,773 0 0.0%
IRModule.NumIRBasicBlocks 126,134 126,134 0 0.0%
IRModule.NumIRFunctions 74,571 74,752 181 0.24%
IRModule.NumIRGlobals 68,046 68,227 181 0.27%
IRModule.NumIRInsts 1,405,191 1,405,191 0 0.0%
IRModule.NumIRValueSymbols 127,531 127,893 362 0.28%
LLVM.NumLLVMBytesOutput 41,051,794 41,093,962 42,168 0.1%
SILModule.NumSILGenFunctions 42,734 42,734 0 0.0%
SILModule.NumSILOptFunctions 47,122 47,122 0 0.0%
Sema.NumConformancesDeserialized 329,506 329,506 0 0.0%
Sema.NumConstraintScopes 900,175 900,175 0 0.0%
Sema.NumDeclsDeserialized 2,024,300 2,024,300 0 0.0%
Sema.NumDeclsValidated 190,411 190,411 0 0.0%
Sema.NumFunctionsTypechecked 48,042 48,042 0 0.0%
Sema.NumGenericSignatureBuilders 59,038 59,038 0 0.0%
Sema.NumLazyGenericEnvironments 402,299 402,299 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 39,182 39,182 0 0.0%
Sema.NumLazyIterableDeclContexts 330,454 330,454 0 0.0%
Sema.NumTypesDeserialized 2,120,500 2,120,500 0 0.0%
Sema.NumTypesValidated 211,800 211,800 0 0.0%

Release

release brief

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
name old new delta delta_pct
LLVM.NumLLVMBytesOutput 30,584,840 30,680,580 95,740 0.31%
time.swift-driver.wall 163.1s 163.2s 109.7ms 0.07%

release detailed

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
name old new delta delta_pct
AST.NumImportedExternalDefinitions 8,764 8,764 0 0.0%
AST.NumLoadedModules 406 406 0 0.0%
AST.NumTotalClangImportedEntities 25,382 25,382 0 0.0%
AST.NumUsedConformances 8,779 8,779 0 0.0%
IRModule.NumIRBasicBlocks 176,366 176,366 0 0.0%
IRModule.NumIRFunctions 57,276 57,660 384 0.67%
IRModule.NumIRGlobals 55,330 55,714 384 0.69%
IRModule.NumIRInsts 1,393,512 1,393,512 0 0.0%
IRModule.NumIRValueSymbols 101,286 102,054 768 0.76%
LLVM.NumLLVMBytesOutput 30,584,840 30,680,580 95,740 0.31%
SILModule.NumSILGenFunctions 22,139 22,139 0 0.0%
SILModule.NumSILOptFunctions 38,891 38,891 0 0.0%
Sema.NumConformancesDeserialized 153,119 153,119 0 0.0%
Sema.NumConstraintScopes 798,881 798,881 0 0.0%
Sema.NumDeclsDeserialized 221,997 221,997 0 0.0%
Sema.NumDeclsValidated 59,864 59,864 0 0.0%
Sema.NumFunctionsTypechecked 10,695 10,695 0 0.0%
Sema.NumGenericSignatureBuilders 6,466 6,466 0 0.0%
Sema.NumLazyGenericEnvironments 33,720 33,720 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 4,538 4,538 0 0.0%
Sema.NumLazyIterableDeclContexts 21,410 21,410 0 0.0%
Sema.NumTypesDeserialized 279,451 279,451 0 0.0%
Sema.NumTypesValidated 34,802 34,802 0 0.0%

@slavapestov
Copy link
Contributor

LGTM

@swift-ci
Copy link
Contributor

swift-ci commented May 3, 2018

Build failed
Swift Test OS X Platform
Git Sha - 2f112fb

@jrose-apple jrose-apple mentioned this pull request May 3, 2018
Merged
@jrose-apple
Copy link
Contributor Author

I have a fix for this particular issue but now I'm foreseeing problems with using this within Apple due to the way we lay out certain SDK content. I'll talk with Graydon and Doug about it tomorrow.

This was referenced May 3, 2018
Closed
Merged
@jrose-apple
Copy link
Contributor Author

Closing in favor of #16349.

@jrose-apple jrose-apple closed this May 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@graydon graydon graydon approved these changes

@slavapestov slavapestov Awaiting requested review from slavapestov

@DougGregor DougGregor Awaiting requested review from DougGregor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jrose-apple @swift-ci @slavapestov @graydon