[OSLogOptimization] Improve the OSLogOptimization pass so that it can fold constant arrays and closure literals #28168
Conversation
|
@swift-ci Please test |
|
Build failed |
|
@swift-ci Please test Linux Platform |
| SILInstruction *constructorInst = definingInst; | ||
| if (isa<DestructureTupleInst>(definingInst) || | ||
| isa<TupleExtractInst>(definingInst)) { | ||
| constructorInst = definingInst->getOperand(0)->getDefiningInstruction(); |
There was a problem hiding this comment.
constructorInst is an unused variable.
If this is done because some particular array allocation intrinsics returns a tuple, you might want to make that a little more explicit at least in comments and have a small test case if it's not too hard.
There was a problem hiding this comment.
Is value required to be a result of definingInstruction?
| /// Return true iff the given value is a closure but is not a creation of a | ||
| /// closure e.g., through partial_apply or thin_to_thick_function or | ||
| /// convert_function. | ||
| static bool isFoldableClosure(SILValue value, SILInstruction *definingInst) { |
There was a problem hiding this comment.
It doesn't look like you actually need definingInst. Unless value comes from a different instruction altogether, in which case the meaning of these arguments should be explained.
There was a problem hiding this comment.
You can check a value's type directly. And if you really need its defining instruction, it's not a big deal to either dyn_cast it to SingleValueInstruction or just call getDefiningInstruction again.
ravikandhadai
force-pushed
the
oslog-closure-array-folding
branch
from
f7b489e to
99ce129
Compare
|
@swift-ci Please test |
|
Build failed |
ravikandhadai
force-pushed
the
oslog-closure-array-folding
branch
from
99ce129 to
e325d37
Compare
|
@swift-ci Please test |
1 similar comment
|
@swift-ci Please test |
|
@atrick Thanks for the comments. I have pushed a commit that addressed the comments. I have also added a test |
|
Build failed |
of the OSLogOptimization pass. This commit contain two changes: - It handles non-OSSA better (but it is meant to be phased out) so that array and closure folding can be supported - It fixes a bug in the OSSA folding by making sure that when an owned value replaces a guaranteed value, the owned value is borrowed and the borrow is used in place of the guaranteed value.
pass so that it constant folds array symbolic values inferred by the constant evaluator when evaluting os log calls.
fold a symbolic closure, which is the representation of a closure literal in the constant evaluator. This commit improves the function emitCodeForSymbolicValue so that given a symbolic closure it can emit SIL code for constructing the closure. This improvement enables folding the arguments array, which is an array of closures, by its constant value inferred by constant evaluating the new OSLog calls.
ravikandhadai
force-pushed
the
oslog-closure-array-folding
branch
from
e325d37 to
633bc79
Compare
|
@swift-ci Please test |
1 similar comment
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
This PR enhances the OSLogOptimization pass so that it can fold constant arrays and closure literals that are inferred by the constant evaluator while evaluating os log calls. The main changes of this PR are mostly confined to the function
emitCodeForSymbolicValueof theOSLogOptimizationpass. In addition to adding support for array and closure folding, this PR also contains a fix to a (minor) bug in the code that manages the lifetime of the folded value. For perspicuity, this PR is split into 3 self-contained commits as described below:Commit 9b57208 fixes a bug in the code that replaces a guaranteed value by an owned (constant) value. It also slightly improves the replacement logic for non-OSSA (which is meant to be phased out soon once OSSA is enabled in the mandatory pipeline) so that it can handle the folding of arrays and closures introduced in this PR. This commit updates
OSLogPrototypeCompileTest.siltest suite to reflect these changes.Commit fa0ec3d adds support for folding constant arrays. It also adds SIL tests for testing array folding logic to the file:
OSLogPrototypeCompileTest.sil.Commit f7b489e adds support for folding symbolic closures, which are representations of closure literals in the constant evaluator. It also adds SIL tests for testing the closure folding logic. Importantly, this commit updates the
OSLogPrototypeCompileTest.swiftto check whether the "array of closures" used by the new OSLog overlay is folded.