-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Conversation
@swift-ci test |
@swift-ci apple silicon benchmark |
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
1d5911f
to
6714a72
Compare
@swift-ci test |
@swift-ci apple silicon benchmark |
@swift-ci test linux |
@swift-ci smoke test linux |
@swift-ci test linux |
1 similar comment
@swift-ci test linux |
@eeckstein |
|
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. |
@Steelskin can you attach the output of |
@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) |
There was a problem hiding this comment.
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
This pass replaces
alloc_box
withalloc_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.
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