Skip to content

Commit b555b10

Browse files
committed
Remove borrow scope adjustment for @guaranteed phi args
1 parent 0c2588e commit b555b10

File tree

4 files changed

+10
-76
lines changed

4 files changed

+10
-76
lines changed

include/swift/SILOptimizer/Utils/BasicBlockOptUtils.h

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ class BasicBlockCloner : public SILCloner<BasicBlockCloner> {
284284
/// WARNING: If client converts terminator results to phis (e.g. replaces a
285285
/// switch_enum with a branch), then it must call this before performing that
286286
/// transformation, or fix the OSSA representation of that value itself.
287-
void updateOSSAAfterCloning();
287+
void updateSSAAfterCloning();
288288

289289
/// Get the newly cloned block corresponding to `origBB`.
290290
SILBasicBlock *getNewBB() {
@@ -294,13 +294,6 @@ class BasicBlockCloner : public SILCloner<BasicBlockCloner> {
294294
bool wasCloned() { return isBlockCloned(origBB); }
295295

296296
protected:
297-
/// Helper function to perform SSA updates used by updateOSSAAfterCloning.
298-
void updateSSAAfterCloning(SmallVectorImpl<SILPhiArgument *> &newPhis);
299-
300-
/// Given a terminator result, either from the original or the cloned block,
301-
/// update OSSA for any phis created for the result during edge splitting.
302-
void updateOSSATerminatorResult(SILPhiArgument *termResult);
303-
304297
// MARK: CRTP overrides.
305298

306299
/// Override getMappedValue to allow values defined outside the block to be

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ bool SimplifyCFG::threadEdge(const ThreadInfo &ti) {
245245
dyn_cast<BranchInst>(ThreadedSuccessorBlock->getTerminator())) {
246246
simplifyBranchBlock(branchInst);
247247
}
248-
Cloner.updateOSSAAfterCloning();
248+
Cloner.updateSSAAfterCloning();
249249
return true;
250250
}
251251

@@ -1030,7 +1030,7 @@ bool SimplifyCFG::tryJumpThreading(BranchInst *BI) {
10301030
// Duplicate the destination block into this one, rewriting uses of the BBArgs
10311031
// to use the branch arguments as we go.
10321032
Cloner.cloneBranchTarget(BI);
1033-
Cloner.updateOSSAAfterCloning();
1033+
Cloner.updateSSAAfterCloning();
10341034

10351035
// Once all the instructions are copied, we can nuke BI itself. We also add
10361036
// the threaded and edge block to the worklist now that they (likely) can be
@@ -2856,7 +2856,7 @@ bool SimplifyCFG::tailDuplicateObjCMethodCallSuccessorBlocks() {
28562856
continue;
28572857

28582858
Cloner.cloneBranchTarget(Branch);
2859-
Cloner.updateOSSAAfterCloning();
2859+
Cloner.updateSSAAfterCloning();
28602860

28612861
Changed = true;
28622862
// Simplify the cloned block and continue tail duplicating through its new

lib/SILOptimizer/Utils/BasicBlockOptUtils.cpp

Lines changed: 2 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -147,67 +147,8 @@ bool swift::canCloneTerminator(TermInst *termInst) {
147147
return true;
148148
}
149149

150-
/// Given a terminator result, either from the original or the cloned block,
151-
/// update OSSA for any phis created for the result during edge splitting.
152-
void BasicBlockCloner::updateOSSATerminatorResult(SILPhiArgument *termResult) {
153-
assert(termResult->isTerminatorResult() && "precondition");
154-
155-
// If the terminator result is used by a phi, then it is invalid OSSA
156-
// which was created by edge splitting.
157-
for (Operand *termUse : termResult->getUses()) {
158-
if (auto phiOper = PhiOperand(termUse)) {
159-
createBorrowScopeForPhiOperands(phiOper.getValue());
160-
}
161-
}
162-
}
163-
164-
// Cloning does not invalidate ownership lifetime. When it clones values, it
165-
// also either clones the consumes, or creates the necessary phis that consume
166-
// the new values on all paths. However, cloning may create new phis of
167-
// inner guaranteed values. Since phis are reborrows, they are only allowed to
168-
// use BorrowedValues. Therefore, we must create nested borrow scopes for any
169-
// new phis whose arguments aren't BorrowedValues. Note that other newly created
170-
// phis are themselves BorrowedValues, so only one level of nested borrow is
171-
// needed per value, per new phi that the value reaches.
172-
void BasicBlockCloner::updateOSSAAfterCloning() {
150+
void BasicBlockCloner::updateSSAAfterCloning() {
173151
SmallVector<SILPhiArgument *, 4> updateSSAPhis;
174-
if (!origBB->getParent()->hasOwnership()) {
175-
updateSSAAfterCloning(updateSSAPhis);
176-
return;
177-
}
178-
179-
// If the original basic block has terminator results, then all phis in the
180-
// exit blocks are new phis that used to be terminator results.
181-
//
182-
// Create nested borrow scopes for terminator results that were converted to
183-
// phis during edge splitting. This is simpler to check before SSA update.
184-
//
185-
// This assumes that the phis introduced by update-SSA below cannot be users
186-
// of the phis that were created in exitBBs during block cloning. Otherwise
187-
// borrowPhiArguments would handle them twice.
188-
auto *termInst = origBB->getTerminator();
189-
// FIXME: cond_br args should not exist in OSSA
190-
if (!isa<BranchInst>(termInst) && !isa<CondBranchInst>(termInst)) {
191-
// Update all of the terminator results.
192-
for (auto *succBB : origBB->getSuccessorBlocks()) {
193-
for (SILArgument *termResult : succBB->getArguments()) {
194-
updateOSSATerminatorResult(cast<SILPhiArgument>(termResult));
195-
}
196-
}
197-
}
198-
199-
// Update SSA form before calling OSSA update utilities to maintain a layering
200-
// of SIL invariants.
201-
updateSSAAfterCloning(updateSSAPhis);
202-
203-
// Create nested borrow scopes for phis created during SSA update.
204-
for (auto *phi : updateSSAPhis) {
205-
createBorrowScopeForPhiOperands(phi);
206-
}
207-
}
208-
209-
void BasicBlockCloner::updateSSAAfterCloning(
210-
SmallVectorImpl<SILPhiArgument *> &newPhis) {
211152
// All instructions should have been checked by canCloneInstruction. But we
212153
// still need to check the arguments.
213154
for (auto arg : origBB->getArguments()) {
@@ -218,7 +159,7 @@ void BasicBlockCloner::updateSSAAfterCloning(
218159
if (!needsSSAUpdate)
219160
return;
220161

221-
SILSSAUpdater ssaUpdater(&newPhis);
162+
SILSSAUpdater ssaUpdater(&updateSSAPhis);
222163
for (auto availValPair : availVals) {
223164
auto inst = availValPair.first;
224165
if (inst->use_empty())

lib/SILOptimizer/Utils/CheckedCastBrJumpThreading.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ void CheckedCastBrJumpThreading::Edit::modifyCFGForFailurePreds(
321321
replaceBranchTarget(TI, CCBBlock, TargetFailureBB,
322322
/*PreserveArgs=*/true);
323323
}
324-
Cloner.updateOSSAAfterCloning();
324+
Cloner.updateSSAAfterCloning();
325325
}
326326

327327
/// Create a copy of the BB or reuse BB as a landing basic block for all
@@ -383,7 +383,7 @@ void CheckedCastBrJumpThreading::Edit::modifyCFGForSuccessPreds(
383383
SILBuilderWithScope Builder(clonedCCBI);
384384
Builder.createBranch(clonedCCBI->getLoc(), successBB, {SuccessArg});
385385
clonedCCBI->eraseFromParent();
386-
Cloner.updateOSSAAfterCloning();
386+
Cloner.updateSSAAfterCloning();
387387
return;
388388
}
389389
// Remove all uses from the failure path so RAUW can erase the
@@ -395,7 +395,7 @@ void CheckedCastBrJumpThreading::Edit::modifyCFGForSuccessPreds(
395395
// Create nested borrow scopes for new phis either created for the
396396
// checked_cast's results or during SSA update. This puts the SIL in
397397
// valid OSSA form before calling OwnershipRAUWHelper.
398-
Cloner.updateOSSAAfterCloning();
398+
Cloner.updateSSAAfterCloning();
399399

400400
auto *clonedSuccessArg = successBB->getArgument(0);
401401
OwnershipRAUWHelper rauwUtil(rauwContext, clonedSuccessArg, SuccessArg);
@@ -786,7 +786,7 @@ void CheckedCastBrJumpThreading::optimizeFunction() {
786786
edit->modifyCFGForSuccessPreds(Cloner, rauwContext);
787787

788788
if (Cloner.wasCloned()) {
789-
Cloner.updateOSSAAfterCloning();
789+
Cloner.updateSSAAfterCloning();
790790

791791
if (!Cloner.getNewBB()->pred_empty())
792792
BlocksForWorklist.push_back(Cloner.getNewBB());

0 commit comments

Comments
 (0)