Enable ArrayElementValuePropagation on ownership SIL #34549
Conversation
|
@swift-ci test |
|
Build failed |
| return std::next(cast<SingleValueInstruction>(val)->getIterator()); | ||
| } | ||
| if (isa<SILArgument>(val)) { | ||
| return cast<SILArgument>(val)->getParentBlock()->begin(); |
There was a problem hiding this comment.
This is just a quick drive by, but there is an API that already supports this: getDefiningInsertionPt.
There was a problem hiding this comment.
Hmmm... I see why this is a little different.
| // Because we are creating copy_value in ossa, and the source may have been | ||
| // taken previously. So our insert point for copy_value is immediately after | ||
| // V, where we can be sure it is live. | ||
| auto InsertPt = V->getFunction()->hasOwnership() |
There was a problem hiding this comment.
Why not just this to always do this? Then we have one implementation? I haven't thought through it maybe there is a reason not to.
There was a problem hiding this comment.
We could, but the retain would be much earlier than when we need, and I was thinking it may effect arc optimizations.
| @@ -105,7 +105,6 @@ struct SemanticARCOpts : SILFunctionTransform { | |||
| if (!f.getModule().getOptions().EnableOSSAOptimizations || | |||
| !f.hasOwnership()) | |||
| return; | |||
|
|
|||
There was a problem hiding this comment.
Is this deleted empty line intention?
| SILValue CopiedVal = | ||
| SILBuilderWithScope(InsertPt.getValue()) | ||
| .emitCopyValueOperation(SemanticsCall->getLoc(), V); | ||
| SemanticsCall->replaceAllUsesWith(CopiedVal); | ||
| } else { | ||
| auto Dest = SemanticsCall->getArgument(0); |
There was a problem hiding this comment.
I think you can remove this else-branch completely. It stems from a time where the specializations of getElement did return an indirect result, even for loadable types. This should not be the case anymore. You can just bail if !hasGetElementDirectResult().
meg-gupta
force-pushed
the
arrayelempropossa
branch
from
a24b232 to
483321c
Compare
|
@swift-ci smoke test and merge |
No description provided.