Skip to content

Commit 8ba105f

Browse files
committed
Use AddressOwnership in OSSA RAUW. Improves optimization.
This mainly simplifies the utility, but also improves optimization as a side effect. Update OSSA RAUW after replacing BorrowedAddress with AddressOwnership. InteriorPointer is no longer needed. This simplifies the fixup context. Eventually the fixup context will be very lightweight. This is just the first step.
1 parent 767e094 commit 8ba105f

File tree

2 files changed

+57
-50
lines changed

2 files changed

+57
-50
lines changed

include/swift/SILOptimizer/Utils/OwnershipOptUtils.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,14 @@ struct OwnershipFixupContext {
7474
/// This is the interior pointer operand that the new value we want to RAUW
7575
/// is transitively derived from and enables us to know the underlying
7676
/// borrowed base value that we need to lifetime extend.
77-
InteriorPointerOperand intPtrOp;
77+
///
78+
/// This is only initialized when the interior pointer has uses that must be
79+
/// replaced.
80+
AccessBase base;
7881

7982
void clear() {
8083
allAddressUsesFromOldValue.clear();
81-
intPtrOp = InteriorPointerOperand();
84+
base = AccessBase();
8285
}
8386
};
8487
AddressFixupContext extraAddressFixupInfo;
@@ -90,14 +93,14 @@ struct OwnershipFixupContext {
9093
transitiveBorrowedUses.clear();
9194
recursiveReborrows.clear();
9295
extraAddressFixupInfo.allAddressUsesFromOldValue.clear();
93-
extraAddressFixupInfo.intPtrOp = InteriorPointerOperand();
96+
extraAddressFixupInfo.base = AccessBase();
9497
}
9598

9699
private:
97100
/// Helper method called to determine if we discovered we needed interior
98101
/// pointer fixups while simplifying.
99102
bool needsInteriorPointerFixups() const {
100-
return bool(extraAddressFixupInfo.intPtrOp);
103+
return bool(extraAddressFixupInfo.base);
101104
}
102105
};
103106

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

Lines changed: 50 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,6 @@ OwnershipLifetimeExtender::createPlusZeroBorrow(SILValue newValue,
672672
callbacks.createdNewInst(ebi);
673673
callbacks.createdNewInst(dvi);
674674
}
675-
676675
return borrow;
677676
}
678677

@@ -1032,7 +1031,7 @@ OwnershipRAUWHelper::replaceAddressUses(SingleValueInstruction *oldValue,
10321031
// If we are replacing addresses, see if we need to handle interior pointer
10331032
// fixups. If we don't have any extra info, then we know that we can just RAUW
10341033
// without any further work.
1035-
if (!ctx->extraAddressFixupInfo.intPtrOp)
1034+
if (!ctx->extraAddressFixupInfo.base)
10361035
return replaceAllUsesAndErase(oldValue, newValue, ctx->callbacks);
10371036

10381037
// We are RAUWing two addresses and we found that:
@@ -1045,25 +1044,26 @@ OwnershipRAUWHelper::replaceAddressUses(SingleValueInstruction *oldValue,
10451044
// interior pointer instruction and change the clone to use our new borrowed
10461045
// value. Then we RAUW as appropriate.
10471046
OwnershipLifetimeExtender extender{*ctx};
1048-
auto &extraInfo = ctx->extraAddressFixupInfo;
1049-
auto intPtr = *extraInfo.intPtrOp;
1047+
auto base = ctx->extraAddressFixupInfo.base;
1048+
// FIXME: why does this use allAddressUsesFromOldValue instead of
1049+
// guaranteedUsePoints?
10501050
BeginBorrowInst *bbi = extender.createPlusZeroBorrow(
1051-
intPtr->get(), llvm::makeArrayRef(extraInfo.allAddressUsesFromOldValue));
1051+
base.getReference(),
1052+
llvm::makeArrayRef(
1053+
ctx->extraAddressFixupInfo.allAddressUsesFromOldValue));
10521054
auto bbiNext = &*std::next(bbi->getIterator());
1053-
auto *newIntPtrUser =
1054-
cast<SingleValueInstruction>(intPtr->getUser()->clone(bbiNext));
1055-
ctx->callbacks.createdNewInst(newIntPtrUser);
1056-
newIntPtrUser->setOperand(0, bbi);
1055+
auto *refProjection = cast<SingleValueInstruction>(base.getBaseAddress());
1056+
auto *newBase = refProjection->clone(bbiNext);
1057+
ctx->callbacks.createdNewInst(newBase);
1058+
newBase->setOperand(0, bbi);
10571059

10581060
// Now that we have extended our lifetime as appropriate, we need to recreate
1059-
// the access path from newValue to intPtr but upon newIntPtr. Then we make it
1060-
// use newIntPtr.
1061-
auto *intPtrUser = cast<SingleValueInstruction>(intPtr->getUser());
1062-
1061+
// the access path from newValue to refProjection but upon newBase.
1062+
//
10631063
// This cloner invocation must match the canCloneUseDefChain check in the
10641064
// constructor.
10651065
auto checkBase = [&](SILValue srcAddr) {
1066-
return (srcAddr == intPtrUser) ? SILValue(newIntPtrUser) : SILValue();
1066+
return (srcAddr == refProjection) ? SILValue(newBase) : SILValue();
10671067
};
10681068
SILValue clonedAddr =
10691069
cloneUseDefChain(newValue, /*insetPt*/ oldValue, checkBase);
@@ -1141,58 +1141,62 @@ OwnershipRAUWHelper::OwnershipRAUWHelper(OwnershipFixupContext &inputCtx,
11411141
if (!addressOwnership.hasLocalOwnershipLifetime())
11421142
return;
11431143

1144-
auto intPtrOp =
1145-
InteriorPointerOperand::inferFromResult(
1146-
addressOwnership.base.getBaseAddress());
1147-
if (!intPtrOp) {
1144+
ctx->extraAddressFixupInfo.base = addressOwnership.base;
1145+
1146+
// For now, just gather up uses
1147+
//
1148+
// FIXME: get rid of allAddressUsesFromOldValue. Shouldn't this already be
1149+
// included in guaranteedUsePoints?
1150+
auto &oldValueUses = ctx->extraAddressFixupInfo.allAddressUsesFromOldValue;
1151+
// FIXME: The return value of findTransitiveUsesForAddress is currently
1152+
// inverted.
1153+
if (findTransitiveUsesForAddress(oldValue, oldValueUses)) {
11481154
invalidate();
11491155
return;
11501156
}
1151-
1152-
ctx->extraAddressFixupInfo.intPtrOp = intPtrOp;
1153-
auto borrowedValue = intPtrOp.getSingleBorrowedValue();
1154-
if (!borrowedValue) {
1155-
invalidate();
1157+
if (addressOwnership.areUsesWithinLifetime(oldValueUses, ctx->deBlocks)) {
1158+
// We do not need to copy the base value! Clear the extra info we have.
1159+
ctx->extraAddressFixupInfo.clear();
11561160
return;
11571161
}
1158-
1159-
auto *intPtrInst =
1160-
cast<SingleValueInstruction>(intPtrOp.getUser());
1162+
// This cloner check must match the later cloner invocation in
1163+
// getReplacementAddress()
1164+
SILValue baseAddress = ctx->extraAddressFixupInfo.base.getBaseAddress();
1165+
auto *baseInst = cast<SingleValueInstruction>(baseAddress);
11611166
auto checkBase = [&](SILValue srcAddr) {
1162-
return (srcAddr == intPtrInst) ? SILValue(intPtrInst) : SILValue();
1167+
return (srcAddr == baseInst) ? SILValue(baseInst) : SILValue();
11631168
};
1164-
// This cloner check must match the later cloner invocation in
1165-
// replaceAddressUses()
11661169
if (!canCloneUseDefChain(newValue, checkBase)) {
11671170
invalidate();
11681171
return;
11691172
}
1170-
1171-
// For now, just gather up uses
1172-
auto &oldValueUses = ctx->extraAddressFixupInfo.allAddressUsesFromOldValue;
1173-
if (findTransitiveUsesForAddress(oldValue, oldValueUses)) {
1174-
invalidate();
1175-
return;
1176-
}
1177-
1178-
// Ok, at this point we know that we can optimize. The only question is if we
1179-
// need to perform the copy or not when we actually RAUW. So perform the is
1180-
// within region check. If we succeed, clear our extra state so we perform a
1181-
// normal RAUW.
1182-
if (borrowedValue.areUsesWithinLocalScope(oldValueUses, ctx->deBlocks)) {
1183-
// We do not need to copy the base value! Clear the extra info we have.
1184-
ctx->extraAddressFixupInfo.clear();
1185-
}
11861173
}
11871174

11881175
SILBasicBlock::iterator
11891176
OwnershipRAUWHelper::perform(SingleValueInstruction *maybeTransformedNewValue) {
11901177
assert(isValid() && "OwnershipRAUWHelper invalid?!");
11911178

11921179
SILValue actualNewValue = newValue;
1193-
if (maybeTransformedNewValue)
1180+
if (maybeTransformedNewValue) {
1181+
// Temporary variable for rebasing
1182+
SILValue rewrittenNewValue = maybeTransformedNewValue;
1183+
// Everything about \n newValue that the constructor checks should also be
1184+
// true for rewrittenNewValue.
1185+
// FIXME: enable these...
1186+
// assert(rewrittenNewValue->getType() == newValue->getType());
1187+
// assert(rewrittenNewValue->getOwnershipKind()
1188+
// == newValue->getOwnershipKind());
1189+
// assert(rewrittenNewValue->getParentBlock() == newValue->getParentBlock());
1190+
assert(!newValue->getType().isAddress() ||
1191+
AddressOwnership(rewrittenNewValue) == AddressOwnership(newValue));
1192+
11941193
actualNewValue = maybeTransformedNewValue;
11951194

1195+
// TODO: newValue = rewrittenNewValue; (remove actualNewValue)
1196+
}
1197+
assert(newValue && "prepareReplacement can only be called once");
1198+
SWIFT_DEFER { newValue = SILValue(); };
1199+
11961200
if (!oldValue->getFunction()->hasOwnership())
11971201
return replaceAllUsesAndErase(oldValue, actualNewValue, ctx->callbacks);
11981202

0 commit comments

Comments
 (0)