Skip to content

[IRGen] Handle ProtocolInfo for protocols whose members aren't used #18692

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
Merged
merged 1 commit into from
Aug 14, 2018
Merged

[IRGen] Handle ProtocolInfo for protocols whose members aren't used #18692

merged 1 commit into from
Aug 14, 2018

Conversation

jrose-apple
Copy link
Contributor

Certain uses of protocols only formally need the requirement signature, not any of the method requirements. This results in IRGen seeing a protocol where none of the members have been validated except the associated types. Account for this by allowing ProtocolInfo to only contain the layout for the base protocols and associated types, if requested.

Note that this relies on the layout of a witness table always putting the "requirement signature part" at the front, or at least at offsets that aren't affected by function requirements.

rdar://problem/43260117

@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 test compiler performance

slavapestov
slavapestov approved these changes Aug 14, 2018
View reviewed changes
Copy link
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

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

Thanks!

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - d73a09888ccd562b2cf847e27f3dfdceddcb4070

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d73a09888ccd562b2cf847e27f3dfdceddcb4070

@jrose-apple
Copy link
Contributor Author

*sigh* Both silly platform-dependent things that were really rather obvious.

Compiler performance
Source compat

@jrose-apple jrose-apple force-pushed the just-your-signature-please branch from d73a098 to 293082d Compare August 14, 2018 02:58
@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d73a09888ccd562b2cf847e27f3dfdceddcb4070

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - d73a09888ccd562b2cf847e27f3dfdceddcb4070

@swift-ci
Copy link
Contributor

Build comment file:

Summary for master full

Unexpected test results, excluded stats for RxSwift, ChattoAdditions, ProcedureKitCloud, DatabaseKit, ReactiveSwift, ReactiveCocoa, Wordy

Regressions found (see below)

Debug-batch

debug-batch brief

Regressed (2)
name old new delta delta_pct
Frontend.NumInstructions 9,993,050,866,843 10,128,012,738,121 134,961,871,278 1.35% ⛔
time.swift-driver.wall 1123.0s 1139.2s 16.2s 1.44% ⛔
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (1)
name old new delta delta_pct
LLVM.NumLLVMBytesOutput 471,574,776 474,817,150 3,242,374 0.69%

debug-batch detailed

Regressed (25)
name old new delta delta_pct
AST.NumASTBytesAllocated 12,929,810,901 13,090,777,191 160,966,290 1.24% ⛔
AST.NumDependencies 81,160 83,510 2,350 2.9% ⛔
AST.NumImportedExternalDefinitions 731,717 744,725 13,008 1.78% ⛔
AST.NumLoadedModules 93,504 95,604 2,100 2.25% ⛔
AST.NumTotalClangImportedEntities 2,403,078 2,444,700 41,622 1.73% ⛔
Driver.ChildrenMaxRSS 33,388,343,296 33,863,929,856 475,586,560 1.42% ⛔
Frontend.NumInstructions 9,993,050,866,843 10,128,012,738,121 134,961,871,278 1.35% ⛔
SILModule.NumSILGenFunctions 808,334 818,976 10,642 1.32% ⛔
SILModule.NumSILGenVtables 3,680 3,742 62 1.68% ⛔
SILModule.NumSILOptVtables 7,070 7,206 136 1.92% ⛔
Sema.ExtendedNominalRequest 1,396,677 1,413,581 16,904 1.21% ⛔
Sema.InheritedDeclsReferencedRequest 52,920,624 53,481,256 560,632 1.06% ⛔
Sema.NamedLazyMemberLoadFailureCount 13,185 13,373 188 1.43% ⛔
Sema.NumConformancesDeserialized 2,348,123 2,372,090 23,967 1.02% ⛔
Sema.NumDeclsDeserialized 15,669,546 15,868,981 199,435 1.27% ⛔
Sema.NumFunctionsTypechecked 429,740 436,220 6,480 1.51% ⛔
Sema.NumGenericSignatureBuilders 750,581 762,009 11,428 1.52% ⛔
Sema.NumLazyGenericEnvironments 2,937,261 2,974,091 36,830 1.25% ⛔
Sema.NumLazyGenericEnvironmentsLoaded 293,248 296,468 3,220 1.1% ⛔
Sema.NumLazyIterableDeclContexts 2,634,288 2,674,876 40,588 1.54% ⛔
Sema.NumTypesDeserialized 7,025,338 7,113,034 87,696 1.25% ⛔
Sema.NumUnloadedLazyIterableDeclContexts 1,792,275 1,821,934 29,659 1.65% ⛔
Sema.OverriddenDeclsRequest 1,411,549 1,425,741 14,192 1.01% ⛔
Sema.SuperclassDeclRequest 40,242,152 40,690,026 447,874 1.11% ⛔
Sema.SuperclassTypeRequest 14,766 14,968 202 1.37% ⛔
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (63)
name old new delta delta_pct
AST.NumDecls 31,829 32,025 196 0.62%
AST.NumInfixOperators 12,815 12,895 80 0.62%
AST.NumLinkLibraries 0 0 0 0.0%
AST.NumLocalTypeDecls 13 13 0 0.0%
AST.NumObjCMethods 12,269 12,347 78 0.64%
AST.NumPostfixOperators 14 14 0 0.0%
AST.NumPrecedenceGroups 6,612 6,678 66 1.0%
AST.NumPrefixOperators 60 60 0 0.0%
AST.NumReferencedDynamicNames 45 45 0 0.0%
AST.NumReferencedMemberNames 1,764,711 1,778,939 14,228 0.81%
AST.NumReferencedTopLevelNames 110,333 111,143 810 0.73%
AST.NumSourceBuffers 143,636 144,951 1,315 0.92%
AST.NumSourceLines 1,164,919 1,168,925 4,006 0.34%
AST.NumSourceLinesPerSecond 659,652 659,950 298 0.05%
AST.NumUsedConformances 96,844 97,520 676 0.7%
Driver.DriverDepCascadingDynamic 0 0 0 0.0%
Driver.DriverDepCascadingExternal 0 0 0 0.0%
Driver.DriverDepCascadingMember 0 0 0 0.0%
Driver.DriverDepCascadingNominal 0 0 0 0.0%
Driver.DriverDepCascadingTopLevel 0 0 0 0.0%
Driver.DriverDepDynamic 0 0 0 0.0%
Driver.DriverDepExternal 0 0 0 0.0%
Driver.DriverDepMember 0 0 0 0.0%
Driver.DriverDepNominal 0 0 0 0.0%
Driver.DriverDepTopLevel 0 0 0 0.0%
Driver.NumDriverJobsRun 6,745 6,801 56 0.83%
Driver.NumDriverJobsSkipped 0 0 0 0.0%
Driver.NumDriverPipePolls 81,775 82,558 783 0.96%
Driver.NumDriverPipeReads 85,202 85,784 582 0.68%
Driver.NumProcessFailures 0 0 0 0.0%
Frontend.NumProcessFailures 0 0 0 0.0%
IRModule.NumIRAliases 24,651 24,789 138 0.56%
IRModule.NumIRBasicBlocks 1,662,793 1,669,021 6,228 0.37%
IRModule.NumIRComdatSymbols 0 0 0 0.0%
IRModule.NumIRFunctions 892,437 899,137 6,700 0.75%
IRModule.NumIRGlobals 1,053,842 1,063,875 10,033 0.95%
IRModule.NumIRIFuncs 0 0 0 0.0%
IRModule.NumIRInsts 19,481,864 19,555,524 73,660 0.38%
IRModule.NumIRNamedMetaData 33,328 33,598 270 0.81%
IRModule.NumIRValueSymbols 1,689,582 1,703,937 14,355 0.85%
LLVM.NumLLVMBytesOutput 471,574,776 474,817,150 3,242,374 0.69%
Parse.NumFunctionsParsed 60,824 61,132 308 0.51%
SILModule.NumSILGenDefaultWitnessTables 0 0 0 0.0%
SILModule.NumSILGenGlobalVariables 14,666 14,702 36 0.25%
SILModule.NumSILGenWitnessTables 17,752 17,874 122 0.69%
SILModule.NumSILOptDefaultWitnessTables 0 0 0 0.0%
SILModule.NumSILOptFunctions 664,304 669,857 5,553 0.84%
SILModule.NumSILOptGlobalVariables 15,082 15,122 40 0.27%
SILModule.NumSILOptWitnessTables 34,906 35,122 216 0.62%
Sema.AccessLevelRequest 887,677 891,057 3,380 0.38%
Sema.DefaultAndMaxAccessLevelRequest 21,147 21,173 26 0.12%
Sema.EnumRawTypeRequest 7,630 7,636 6 0.08%
Sema.InheritedTypeRequest 26,112 26,324 212 0.81%
Sema.IsDynamicRequest 686,104 691,498 5,394 0.79%
Sema.IsObjCRequest 620,473 623,827 3,354 0.54%
Sema.NamedLazyMemberLoadSuccessCount 2,290,559 2,301,463 10,904 0.48%
Sema.NominalTypeLookupDirectCount 16,434,831 16,567,981 133,150 0.81%
Sema.NumConstraintScopes 7,241,690 7,257,326 15,636 0.22%
Sema.NumConstraintsConsideredForEdgeContraction 13,049,227 13,063,369 14,142 0.11%
Sema.NumDeclsValidated 989,048 992,878 3,830 0.39%
Sema.NumTypesValidated 853,297 854,829 1,532 0.18%
Sema.SetterAccessLevelRequest 52,106 52,380 274 0.53%
Sema.UnderlyingTypeDeclsReferencedRequest 2,474,370 2,483,300 8,930 0.36%

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) (3)
name old new delta delta_pct
Frontend.NumInstructions 11,884,235,819,111 11,884,358,490,767 122,671,656 0.0%
LLVM.NumLLVMBytesOutput 458,760,514 458,761,234 720 0.0%
time.swift-driver.wall 2104.8s 2106.3s 1.5s 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 137,101 137,101 0 0.0%
AST.NumLoadedModules 6,178 6,178 0 0.0%
AST.NumTotalClangImportedEntities 440,555 440,555 0 0.0%
AST.NumUsedConformances 99,719 99,719 0 0.0%
IRModule.NumIRBasicBlocks 1,663,650 1,663,650 0 0.0%
IRModule.NumIRFunctions 736,247 736,247 0 0.0%
IRModule.NumIRGlobals 838,343 838,343 0 0.0%
IRModule.NumIRInsts 14,286,041 14,286,079 38 0.0%
IRModule.NumIRValueSymbols 1,426,497 1,426,497 0 0.0%
LLVM.NumLLVMBytesOutput 458,760,514 458,761,234 720 0.0%
SILModule.NumSILGenFunctions 316,536 316,536 0 0.0%
SILModule.NumSILOptFunctions 438,647 438,647 0 0.0%
Sema.NumConformancesDeserialized 1,076,187 1,076,187 0 0.0%
Sema.NumConstraintScopes 7,175,345 7,175,345 0 0.0%
Sema.NumDeclsDeserialized 2,904,581 2,904,581 0 0.0%
Sema.NumDeclsValidated 525,429 525,429 0 0.0%
Sema.NumFunctionsTypechecked 163,144 163,144 0 0.0%
Sema.NumGenericSignatureBuilders 120,174 120,174 0 0.0%
Sema.NumLazyGenericEnvironments 490,085 490,085 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 56,833 56,833 0 0.0%
Sema.NumLazyIterableDeclContexts 319,934 319,934 0 0.0%
Sema.NumTypesDeserialized 1,827,981 1,827,981 0 0.0%
Sema.NumTypesValidated 266,709 266,709 0 0.0%

@jrose-apple
Copy link
Contributor Author

I'm pretty sure those Debug numbers are showing that some projects got fixed, which is what I expected from this change.

@jrose-apple
[IRGen] Handle ProtocolInfo for protocols whose members aren't used
b21d283 
Certain uses of protocols only formally need the requirement
signature, not any of the method requirements. This results in IRGen
seeing a protocol where none of the members have been validated except
the associated types. Account for this by allowing ProtocolInfo to
only contain the layout for the base protocols and associated types,
if requested.

Note that this relies on the layout of a witness table always putting
the "requirement signature part" at the front, or at least at offsets
that aren't affected by function requirements.

rdar://problem/43260117
@jrose-apple jrose-apple force-pushed the just-your-signature-please branch from 293082d to b21d283 Compare August 14, 2018 16:42
@jrose-apple
Copy link
Contributor Author

@swift-ci Please test Linux

@jrose-apple
Copy link
Contributor Author

@swift-ci Please smoke test macOS

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 293082d2ecbada64f5c0d663cd312fa68818a291

@jrose-apple jrose-apple merged commit 84f471b into swiftlang:master Aug 14, 2018
@jrose-apple jrose-apple deleted the just-your-signature-please branch August 14, 2018 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slavapestov slavapestov slavapestov approved these changes

@rjmccall rjmccall rjmccall approved these changes

@aschwaighofer aschwaighofer Awaiting requested review from aschwaighofer

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 @rjmccall