Skip to content

Commit a5b1b5c

Browse files
atrickatrick
committed
SILOptimizer OSSA support for switch_enum & checked_cast_br
To create OSSA terminator results, use: - OwnershipForwardingTermInst::createResult(SILType ValueOwnershipKind) - SwitchEnumInst::createDefaultResult() Add support for passing trivial values to nontrivial forwarding ownership. This effectively converts None to Guaranteed ownership. This is essential for handling ".none" enums as trivial values while extracting a nontrivial payload with switch_enum. This converts None to Guaranteed ownership. Generates a copy if needed to convert back to Owned ownership.
1 parent 29ee02d commit a5b1b5c

File tree

7 files changed

+49
-71
lines changed

7 files changed

+49
-71
lines changed

‎lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp‎

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -210,21 +210,15 @@ SILInstruction *SILCombiner::visitSwitchEnumAddrInst(SwitchEnumAddrInst *SEAI) {
210210
auto eltType = Addr->getType().getEnumElementType(
211211
c.first, Builder.getModule(), Builder.getTypeExpansionContext());
212212
eltType = eltType.getObjectType();
213-
if (eltType.isTrivial(Builder.getFunction())) {
214-
c.second->createPhiArgument(eltType, OwnershipKind::None);
215-
} else {
216-
c.second->createPhiArgument(eltType, OwnershipKind::Guaranteed);
217-
}
213+
sei->createResult(c.second, eltType);
218214
}
219215
Builder.setInsertionPoint(c.second->front().getIterator());
220-
Builder.emitEndBorrowOperation(SEAI->getLoc(), EnumVal);
216+
Builder.emitEndBorrowOperation(sei->getLoc(), EnumVal);
221217
}
222-
218+
sei->createDefaultResult();
223219
if (auto defaultBlock = sei->getDefaultBBOrNull()) {
224-
defaultBlock.get()->createPhiArgument(EnumVal->getType(),
225-
OwnershipKind::Guaranteed);
226220
Builder.setInsertionPoint(defaultBlock.get()->front().getIterator());
227-
Builder.emitEndBorrowOperation(SEAI->getLoc(), EnumVal);
221+
Builder.emitEndBorrowOperation(sei->getLoc(), EnumVal);
228222
}
229223
}
230224

@@ -2005,16 +1999,15 @@ SILInstruction *SILCombiner::visitCondBranchInst(CondBranchInst *CBI) {
20051999
//
20062000
// 3. In each destination block, we need to create an argument and end the
20072001
// lifetime of that argument.
2008-
auto enumOperandType = SEI->getEnumOperand()->getType();
2009-
if (DefaultBB) {
2010-
auto *defaultArg =
2011-
DefaultBB->createPhiArgument(enumOperandType, OwnershipKind::Owned);
2012-
SILBuilderWithScope innerBuilder(defaultArg->getNextInstruction(),
2013-
Builder);
2014-
auto loc = RegularLocation::getAutoGeneratedLocation();
2015-
innerBuilder.emitDestroyValueOperation(loc, defaultArg);
2002+
SILValue selectEnumOperand = SEI->getEnumOperand();
2003+
SILValue switchEnumOperand = selectEnumOperand;
2004+
if (selectEnumOperand.getOwnershipKind() != OwnershipKind::None) {
2005+
switchEnumOperand =
2006+
makeCopiedValueAvailable(selectEnumOperand, Builder.getInsertionBB());
20162007
}
2017-
2008+
auto *switchEnum = Builder.createSwitchEnum(
2009+
SEI->getLoc(), switchEnumOperand, DefaultBB, Cases);
2010+
auto enumOperandType = SEI->getEnumOperand()->getType();
20182011
for (auto pair : Cases) {
20192012
// We only need to create the phi argument if our case doesn't have an
20202013
// associated value.
@@ -2026,22 +2019,24 @@ SILInstruction *SILCombiner::visitCondBranchInst(CondBranchInst *CBI) {
20262019

20272020
auto enumEltType =
20282021
enumOperandType.getEnumElementType(enumEltDecl, block->getParent());
2029-
auto *arg = block->createPhiArgument(enumEltType, OwnershipKind::Owned);
2022+
auto *arg = switchEnum->createResult(block, enumEltType);
20302023
SILBuilderWithScope innerBuilder(arg->getNextInstruction(), Builder);
2031-
auto loc = RegularLocation::getAutoGeneratedLocation();
2032-
innerBuilder.emitDestroyValueOperation(loc, arg);
2024+
// The switch enum may change ownership resulting in Guaranteed or None.
2025+
if (arg->getOwnershipKind() == OwnershipKind::Owned) {
2026+
auto loc = RegularLocation::getAutoGeneratedLocation();
2027+
innerBuilder.emitDestroyValueOperation(loc, arg);
2028+
}
20332029
}
2034-
2035-
SILValue selectEnumOperand = SEI->getEnumOperand();
2036-
SILValue switchEnumOperand = selectEnumOperand;
2037-
if (selectEnumOperand.getOwnershipKind() != OwnershipKind::None) {
2038-
switchEnumOperand = makeCopiedValueAvailable(selectEnumOperand,
2039-
Builder.getInsertionBB());
2030+
if (auto defaultArg = switchEnum->createDefaultResult()) {
2031+
SILBuilderWithScope innerBuilder(defaultArg->getNextInstruction(), SEI);
2032+
// The switch enum may change ownership resulting in Guaranteed or None.
2033+
if (defaultArg->getOwnershipKind() == OwnershipKind::Owned) {
2034+
auto loc = RegularLocation::getAutoGeneratedLocation();
2035+
innerBuilder.emitDestroyValueOperation(loc, defaultArg);
2036+
}
20402037
}
2041-
return Builder.createSwitchEnum(SEI->getLoc(), switchEnumOperand, DefaultBB,
2042-
Cases);
2038+
return switchEnum;
20432039
}
2044-
20452040
return nullptr;
20462041
}
20472042

‎lib/SILOptimizer/Transforms/SimplifyCFG.cpp‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2940,16 +2940,13 @@ bool SimplifyCFG::canonicalizeSwitchEnums() {
29402940
if (!SWI.hasDefault())
29412941
continue;
29422942

2943-
NullablePtr<EnumElementDecl> elementDecl = SWI.getUniqueCaseForDefault();
2944-
if (!elementDecl)
2943+
NullablePtr<EnumElementDecl> defaultDecl = SWI.getUniqueCaseForDefault();
2944+
if (!defaultDecl)
29452945
continue;
29462946

29472947
if (!EnableOSSARewriteTerminator && Fn.hasOwnership()) {
29482948
if (!SWI.getOperand()->getType().isTrivial(Fn)) {
2949-
// TODO: OSSA. In OSSA, the default switch_enum case passes the original
2950-
// enum as a block argument. This needs to check that the block argument
2951-
// is dead, then replace it with the a new argument for the default
2952-
// payload.
2949+
// TODO: Test and enable this case.
29532950
continue;
29542951
}
29552952
}
@@ -2962,24 +2959,9 @@ bool SimplifyCFG::canonicalizeSwitchEnums() {
29622959
}
29632960
// Add the default-entry of the original instruction as case-entry.
29642961
auto *defaultBB = SWI.getDefaultBB();
2965-
CaseBBs.push_back(std::make_pair(elementDecl.get(), defaultBB));
2962+
CaseBBs.push_back(std::make_pair(defaultDecl.get(), defaultBB));
29662963

29672964
if (isa<SwitchEnumInst>(*SWI)) {
2968-
if (Fn.hasOwnership()) {
2969-
assert(defaultBB->getNumArguments() == 1);
2970-
defaultBB->getArgument(0)->replaceAllUsesWith(SWI.getOperand());
2971-
defaultBB->eraseArgument(0);
2972-
// TODO: handle non-trivial payloads. The new block argument must be
2973-
// destroyed. The old default argument may need to be copied. But it may
2974-
// also be possible to optimize the common case on-the-fly without the
2975-
// extra copy/destroy.
2976-
if (elementDecl.get()->hasAssociatedValues()) {
2977-
// Note: this is not really a phi.
2978-
auto elementTy = SWI.getOperand()->getType().getEnumElementType(
2979-
elementDecl.get(), Fn.getModule(), Fn.getTypeExpansionContext());
2980-
defaultBB->createPhiArgument(elementTy, OwnershipKind::None);
2981-
}
2982-
}
29832965
SILBuilderWithScope(SWI).createSwitchEnum(SWI->getLoc(), SWI.getOperand(),
29842966
nullptr, CaseBBs);
29852967
} else {

‎lib/SILOptimizer/Utils/CFGOptUtils.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,8 @@ void swift::replaceBranchTarget(TermInst *t, SILBasicBlock *oldDest,
332332
SmallVector<std::pair<EnumElementDecl *, SILBasicBlock *>, 8> cases;
333333
auto *defaultBB = replaceSwitchDest(sei, cases, oldDest, newDest);
334334
builder.createSwitchEnum(sei->getLoc(), sei->getOperand(), defaultBB,
335-
cases);
335+
cases, None, ProfileCounter(),
336+
sei->getForwardingOwnershipKind());
336337
sei->eraseFromParent();
337338
return;
338339
}
@@ -373,7 +374,8 @@ void swift::replaceBranchTarget(TermInst *t, SILBasicBlock *oldDest,
373374
builder.createCheckedCastBranch(
374375
cbi->getLoc(), cbi->isExact(), cbi->getOperand(),
375376
cbi->getTargetLoweredType(), cbi->getTargetFormalType(),
376-
successBB, failureBB, cbi->getTrueBBCount(), cbi->getFalseBBCount());
377+
successBB, failureBB, cbi->getForwardingOwnershipKind(),
378+
cbi->getTrueBBCount(), cbi->getFalseBBCount());
377379
cbi->eraseFromParent();
378380
return;
379381
}

‎lib/SILOptimizer/Utils/ConstExpr.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1928,13 +1928,13 @@ ConstExprFunctionState::evaluateInstructionAndGetNext(
19281928
// tuple-typed argument.
19291929
assert(caseBB->getNumArguments() == 1);
19301930

1931-
if (caseBB->getParent()->hasOwnership() &&
1932-
switchInst.getDefaultBBOrNull() == caseBB) {
1933-
// If we are visiting the default block and we are in ossa, then we may
1934-
// have uses of the failure parameter. That means we need to map the
1935-
// original value to the argument.
1936-
setValue(caseBB->getArgument(0), value);
1937-
return {caseBB->begin(), None};
1931+
if (caseBB == switchInst.getDefaultBBOrNull().getPtrOrNull()) {
1932+
if (!switchInst.getUniqueCaseForDefault()) {
1933+
// In OSSA, the default block forward the original enum value whenever
1934+
// it does not correspond to a unique case.
1935+
setValue(caseBB->getArgument(0), value);
1936+
return {caseBB->begin(), None};
1937+
}
19381938
}
19391939

19401940
assert(value.getKind() == SymbolicValue::EnumWithPayload);

‎lib/SILOptimizer/Utils/InstOptUtils.cpp‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,16 +1023,13 @@ swift::castValueToABICompatibleType(SILBuilder *builder, SILLocation loc,
10231023
SmallVector<std::pair<EnumElementDecl *, SILBasicBlock *>, 1> caseBBs;
10241024
caseBBs.push_back(std::make_pair(someDecl, someBB));
10251025
builder->setInsertionPoint(curBB);
1026-
builder->createSwitchEnum(loc, value, noneBB, caseBBs);
1026+
auto *switchEnum = builder->createSwitchEnum(loc, value, noneBB, caseBBs);
10271027
// In OSSA switch_enum destinations have terminator results.
10281028
//
10291029
// TODO: This should be in a switchEnum utility.
10301030
SILValue unwrappedValue;
10311031
if (builder->hasOwnership()) {
1032-
// Create a terminator result, NOT a phi, despite the API name.
1033-
noneBB->createPhiArgument(value->getType(), OwnershipKind::None);
1034-
unwrappedValue =
1035-
someBB->createPhiArgument(optionalSrcTy, value.getOwnershipKind());
1032+
unwrappedValue = switchEnum->createOptionalSomeResult();
10361033
builder->setInsertionPoint(someBB);
10371034
} else {
10381035
builder->setInsertionPoint(someBB);

‎test/SILOptimizer/cse_ossa_nontrivial.sil‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ sil [ossa] @cse_ossa_utilstest : $@convention(thin) (@guaranteed WrapperStruct)
913913
bb0(%0 : @guaranteed $WrapperStruct):
914914
%f = function_ref @use_nontrivialstruct3 : $@convention(thin) (@in_guaranteed NonTrivialStruct) -> ()
915915
%none = enum $FakeOptional3, #FakeOptional3.none
916-
switch_enum %none : $FakeOptional3, case #FakeOptional3.some!enumelt:bb1, case #FakeOptional3.none!enumelt:bb2
916+
switch_enum %none : $FakeOptional3, case #FakeOptional3.some!enumelt:bb1, case #FakeOptional3.none!enumelt:bb2, forwarding: @guaranteed
917917

918918
bb1(%arg : @guaranteed $WrapperStruct):
919919
%ex = struct_extract %arg : $WrapperStruct, #WrapperStruct.val

‎test/SILOptimizer/redundant_load_elim_nontrivial_ossa.sil‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,14 +258,16 @@ bb2:
258258
bb3:
259259
br bb5
260260

261-
bb4(%11 : $(Klass, Klass)):
262-
%12 = tuple_extract %11 : $(Klass, Klass), 0
261+
bb4(%11 : @owned $(Klass, Klass)):
262+
%12 = begin_borrow %11 : $(Klass, Klass)
263+
%13 = tuple_extract %12 : $(Klass, Klass), 0
264+
end_borrow %12 : $(Klass, Klass)
263265
br bb5
264266

265267
bb5:
266268
br bb5
267269

268-
bb6(%15 : $Klass):
270+
bb6(%15 : @owned $Klass):
269271
br bb5
270272

271273
bb7:

0 commit comments

Comments
 (0)