Skip to content

Optimizer: re-implement and improve the AllocBoxToStack pass #82349

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 24 commits into from
Jun 21, 2025

Conversation

eeckstein
Copy link
Contributor

@eeckstein eeckstein commented Jun 18, 2025

This pass replaces alloc_box with alloc_stack if the box is not escaping.
The original implementation had some limitations. It could not handle cases of local functions which are called multiple times or even recursively, e.g.

public func foo() -> Int {
  var i = 1

  func localFunction() { i += 1 }

  localFunction()
  localFunction()
  return i
}

The new implementation (done in Swift) fixes this problem with a new algorithm.
It's not only more powerful, but also simpler: the new pass has less than half lines of code than the old pass.

The pass is invoked in the mandatory pipeline and later in the optimizer pipeline.
The new implementation provides a module-pass for the mandatory pipeline (whereas the "regular" pass is a function pass).
This is required because the mandatory pass needs to remove originals of specialized closures, which cannot be done from a function-pass.
In the old implementation this was done with a hack by adding a semantic attribute and deleting the function later in the pipeline.

I still kept the sources of the old pass for being able to bootstrap the compiler without a host compiler.

rdar://142756547

@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci apple silicon benchmark

eeckstein added 16 commits June 20, 2025 08:14
SIL may only be modified through a MutatingContext. Otherwise analysis notifications may get lost.
* move `isBox` from `SIL.Type` to `TypeProperties` to make it also available for AST types
* add `BoxFieldsArray.isMutable(fieldIndex:)`
…ith unreachable blocks

`InstructionRange.contains` did yield a wrong result if the range ended in the begin-block and another instruction was inserted in an unreachable block.
Originally, `FunctionWorklist` was a private utility in MandatoryPerformanceOptimizations.
Moved this to `Worklist.swift` to make it generally available.
Also, simplify the `pop()` function which changes the popping order - therefore some test changes were necessary.
…nction)`

This is a safer API than using
```
  let argIdx = applySite.calleeArgumentIndex(of: op)
  let arg = callee.arguments[argIdx]
```
because there is no potential misuse of the index.
Originally this was a "private" utility for the ClosureSpecialization pass.
Now, make it a general utility which can be used for all kind of function specializations.
* rename `var autoGenerated` -> `var asAutoGenerated`
* add `var asCleanup`
* add `func withScope`
…tting Builder locations from instructions

This does not work in some situations.
If mangled the wrong argument indices.
* `insertFunctionArgument`
* `BeginAccessInst.set(accessKind:)`
* `erase(function:)`
* `isReferencedInModule`
* `shouldOptimize`
* `MultipleValueInstruction.replace(with:)`
* `eraseIfDead(functions:)`
* add `cloneFunctionBody` without an `entryBlockArguments` argument
* remove the `swift::ClosureSpecializationCloner` from the bridging code and replace it with a more general `SpecializationCloner`
There are some restrictions for which kind of locations can be on which kind of instructions.
This change implements them correctly.
… with `serializedKind`

No need for bridging functions
* some APIs for `MarkUnresolvedNonCopyableValueInst`
* `AllocBoxInst.hasDynamicLifetime`
A new function must not be added to the pass-manager's worklist if done from a module pass.
…-remoteast-test

This is needed once we implement mandatory passes in Swift, because those testing tools might parse swift interface files from the SDK.
This pass replaces `alloc_box` with `alloc_stack` if the box is not escaping.
The original implementation had some limitations. It could not handle cases of local functions which are called multiple times or even recursively, e.g.

```
public func foo() -> Int {
  var i = 1

  func localFunction() { i += 1 }

  localFunction()
  localFunction()
  return i
}

```

The new implementation (done in Swift) fixes this problem with a new algorithm.
It's not only more powerful, but also simpler: the new pass has less than half lines of code than the old pass.

The pass is invoked in the mandatory pipeline and later in the optimizer pipeline.
The new implementation provides a module-pass for the mandatory pipeline (whereas the "regular" pass is a function pass).
This is required because the mandatory pass needs to remove originals of specialized closures, which cannot be done from a function-pass.
In the old implementation this was done with a hack by adding a semantic attribute and deleting the function later in the pipeline.

I still kept the sources of the old pass for being able to bootstrap the compiler without a host compiler.

rdar://142756547
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci apple silicon benchmark

@eeckstein
Copy link
Contributor Author

@swift-ci test linux

@eeckstein
Copy link
Contributor Author

@swift-ci smoke test linux

@eeckstein
Copy link
Contributor Author

@swift-ci test linux

1 similar comment
@eeckstein
Copy link
Contributor Author

@swift-ci test linux

@eeckstein eeckstein merged commit 1d38956 into swiftlang:main Jun 21, 2025
6 checks passed
@eeckstein eeckstein deleted the alloc-box-to-stack branch June 21, 2025 05:28
@atrick
Copy link
Contributor

atrick commented Jun 21, 2025

@eeckstein getFinalDestroys has a hard-coded set of opcodes that is assumes is complete in terms of ownership forwarding, borrowing, and copying. Use OwnershipUseVisitor for a covered classification of ownership uses.

@eeckstein
Copy link
Contributor Author

getFinalDestroys can only see instructions which are also handled in canPromote

@Steelskin
Copy link
Contributor

FYI, this has broken a SIL pass on Windows. I am looking into making a small reproducer and I will file a bug. In the meantime, the error log is attached.

sil error.log

@eeckstein
Copy link
Contributor Author

@Steelskin can you attach the output of -Xllvm -sil-print-function=s11SwiftSyntax017StringLiteralExprB0V0aB7BuilderE13openDelimiter12openingQuote7content07closingJ005closeH0AcA05TokenB0VSg_AKSSAkLtcfC?

@Steelskin
Copy link
Contributor

@eeckstein I filed #82466 for this. You'll find the requested output attached there. I am still looking into making a smaller reproducer.

}
}

private typealias Flags = (isLexical: Bool, isFromVarDecl: Bool)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a small observation for future SIL design... AllocBoxInst already knows whether it has a VarDecl. So, if AllocBoxToStack sets the isFromVarDecl flag here based on AllocBoxInst.getDecl, then I'm not sure why we need to emit all those redundant begin_borrow [var_decl] instruction.s

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