Skip to content

[SIL][PackageCMO] Allow optimizing [serialized_for_pkg] functions #73902

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 2 commits into from
Jun 4, 2024

Conversation

elsh
Copy link
Contributor

@elsh elsh commented May 24, 2024

Allow optimizing [serialized_for_pkg] functions during SIL inlining, generic/closure specialization, and devirtualization optimization passes.

SILFunction::canBeInlinedIntoCaller now exlicitly requires a caller's SerializedKind_t arg. isAnySerialized() is added as a convenience function that checks if [serialized] or [serialized_for_pkg].

Resolves rdar://128704752

@elsh
Copy link
Contributor Author

elsh commented May 24, 2024

@swift-ci test

@aschwaighofer
Copy link
Contributor

We will also want to update the SwiftCompilerSources, they have instances of both usage of isSerialized and hasValidLinkageForFragileRef.

SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyLoad.swift|284| if !function.isSerialized {
SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift|552| if parentFunction.isSerialized,                                                                                                            
SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift|554| !calleeFunction.isSerialized {
SwiftCompilerSources/Sources/SIL/Function.swift|123| public var isSerialized: Bool { bridged.isSerialized(

This code for example needs updating to take into account our canInlineInto matrix:

extension FullApplySite {                                                       
  var canInline: Bool {                                                         
    // Some checks which are implemented in C++                                 
    if !FullApplySite_canInline(bridged) {                                      
      return false                                                              
    }                                                                           
    // Cannot inline a non-inlinable function it an inlinable function.         
    if parentFunction.isSerialized,                                             
       let calleeFunction = referencedFunction,                                 
       !calleeFunction.isSerialized {                                           
      return false                                                              
    }        

@elsh elsh force-pushed the elsh/pkg-cmo-inline branch from b3ef257 to d16dce6 Compare May 24, 2024 23:17
@elsh elsh requested a review from eeckstein as a code owner May 25, 2024 00:47
@elsh
Copy link
Contributor Author

elsh commented May 25, 2024

@swift-ci test

@elsh
Copy link
Contributor Author

elsh commented May 28, 2024

@swift-ci test macOS

…ing SIL

inlining, generic/closure specialization, and devirtualization optimization passes.

SILFunction::canBeInlinedIntoCaller now exlicitly requires a caller's SerializedKind_t arg.
isAnySerialized() is added as a convenience function that checks if [serialized] or [serialized_for_pkg].

Resolves rdar://128704752
@nkcsgexi
Copy link
Contributor

The functional part makes sense to me. Can we add tests to ensure that these optimization passes are actually active for serialized_for_pkg functions?

@aschwaighofer
Copy link
Contributor

The added changes since my last review look good to me. We also need a change in SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift to take serializedForPackage into account:

     // Cannot inline a non-inlinable function it an inlinable function.         
    if parentFunction.isSerialized,                                             
       let calleeFunction = referencedFunction,                                 
       !calleeFunction.isSerialized {                                           
      return false                                                              
    } 

Add isAnySerialized() and canBeInlinedIntoCaller(SerializedKind).
@elsh
Copy link
Contributor Author

elsh commented May 30, 2024

@swift-ci test

@elsh
Copy link
Contributor Author

elsh commented Jun 4, 2024

The functional part makes sense to me. Can we add tests to ensure that these optimization passes are actually active for serialized_for_pkg functions?

More tests including this are in the upcoming PR.

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