Skip to content

Commit 0cfe74b

Browse files
committed
SIL API for OSSA terminator results.
Add OwnershipForwardingTermInst::createResult(SILBasicBlock, SILType) Add SwitchEnumInst::createDefaultResult() Add SwitchEnumInst::createOptionalSomeResult(), which handles most compiler-generated switches. Basic API for creating terminator results with consistent ownership. This allows enabling OSSA verification on terminator results. It fixes current issues, but is also a prerequisite for OSSA simplify-cfg. For switch_enum, this ensures that the default argument consistently either forwards the original value, or handles the payload of the unique case (the unique payload was already being inferred for ownership, but the block argument was inconsistent with that fact). switch_enum and checked_cast_br specify their forwarding ownership. This can differ from their operand ownership. For example: %e = enum $Optional<AnyObject>, #Optional.none!enumelt switch_enum %e : $Optional<AnyObject>, case #Optional.some!enumelt: bb2... bb2(%arg : @owned T): Independent forwarding ownership is only supported with terminators in this change, but in the near term it will be used for all forwarding operations to support implicit borrow scopes.
1 parent 33fecc9 commit 0cfe74b

File tree

5 files changed

+86
-3
lines changed

5 files changed

+86
-3
lines changed

include/swift/SIL/SILInstruction.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,7 @@ class OwnershipForwardingMixin {
10561056
ValueOwnershipKind ownershipKind)
10571057
: ownershipKind(ownershipKind) {
10581058
assert(isa(kind) && "Invalid subclass?!");
1059+
assert(ownershipKind && "invalid forwarding ownership");
10591060
}
10601061

10611062
public:
@@ -7877,6 +7878,7 @@ class TermInst : public NonValueInstruction {
78777878
}
78787879
};
78797880

7881+
// Forwards the first operand to a result in each successor block.
78807882
class OwnershipForwardingTermInst : public TermInst,
78817883
public OwnershipForwardingMixin {
78827884
protected:
@@ -7903,6 +7905,11 @@ class OwnershipForwardingTermInst : public TermInst,
79037905
return kind == SILInstructionKind::SwitchEnumInst ||
79047906
kind == SILInstructionKind::CheckedCastBranchInst;
79057907
}
7908+
7909+
SILValue getOperand() const { return getAllOperands()[0].get(); }
7910+
7911+
/// Create a result for this terminator on the given successor block.
7912+
SILPhiArgument *createResult(SILBasicBlock *succ, SILType resultTy);
79067913
};
79077914

79087915
/// UnreachableInst - Position in the code which would be undefined to reach.
@@ -8642,6 +8649,14 @@ class SwitchEnumInst
86428649
SILFunction &F, Optional<ArrayRef<ProfileCounter>> CaseCounts,
86438650
ProfileCounter DefaultCount,
86448651
ValueOwnershipKind forwardingOwnershipKind);
8652+
8653+
public:
8654+
/// Create the default result for a partially built switch_enum.
8655+
/// Returns nullptr if no default argument is needed.
8656+
SILPhiArgument *createDefaultResult();
8657+
8658+
/// Create the .some result for an optional switch_enum.
8659+
SILPhiArgument *createOptionalSomeResult();
86458660
};
86468661
/// A switch on an enum's discriminator in memory.
86478662
class SwitchEnumAddrInst

include/swift/SIL/SILInstructionWorklist.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,10 @@ class SILInstructionWorklist : SILInstructionWorklistBase {
185185
/// Intended to be called during visitation after \p instruction has been
186186
/// removed from the worklist.
187187
///
188-
/// \p instruction the instruction whose usages will be replaced
188+
/// \p instruction the instruction whose usages will be replaced. This
189+
/// instruction may already be deleted to maintain valid SIL. For example, if
190+
/// it was a block terminator.
191+
///
189192
/// \p result the instruction whose usages will replace \p instruction
190193
///
191194
/// \return whether the instruction was deleted or modified.

lib/SIL/IR/SILInstructions.cpp

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2895,3 +2895,68 @@ ReturnInst::ReturnInst(SILFunction &func, SILDebugLocation debugLoc,
28952895
"Conflicting ownership kinds when creating term inst from function "
28962896
"result info?!");
28972897
}
2898+
2899+
// This may be called in an invalid SIL state. SILCombine creates new
2900+
// terminators in non-terminator position and defers deleting the original
2901+
// terminator until after all modification.
2902+
SILPhiArgument *OwnershipForwardingTermInst::createResult(SILBasicBlock *succ,
2903+
SILType resultTy) {
2904+
// The forwarding instruction declares a forwarding ownership kind that
2905+
// determines the ownership of its results.
2906+
auto resultOwnership = getForwardingOwnershipKind();
2907+
2908+
// Trivial results have no ownership. Although it is valid for a trivially
2909+
// typed value to have ownership, it is never necessary and less efficient.
2910+
if (resultTy.isTrivial(*getFunction())) {
2911+
resultOwnership = OwnershipKind::None;
2912+
2913+
} else if (resultOwnership == OwnershipKind::None) {
2914+
// switch_enum strangely allows results to acquire ownership out of thin
2915+
// air whenever the operand has no ownership and result is nontrivial:
2916+
// %e = enum $Optional<AnyObject>, #Optional.none!enumelt
2917+
// switch_enum %e : $Optional<AnyObject>,
2918+
// case #Optional.some!enumelt: bb2...
2919+
// bb2(%arg : @guaranteed T):
2920+
//
2921+
// We can either use None or Guaranteed. None would correctly propagate
2922+
// ownership and would maintain the invariant that guaranteed values are
2923+
// always within a borrow scope. However it would result in a nontrivial
2924+
// type without ownership. The lifetime verifier does not like that.
2925+
resultOwnership = OwnershipKind::Guaranteed;
2926+
}
2927+
return succ->createPhiArgument(resultTy, resultOwnership);
2928+
}
2929+
2930+
SILPhiArgument *SwitchEnumInst::createDefaultResult() {
2931+
auto *f = getFunction();
2932+
if (!f->hasOwnership())
2933+
return nullptr;
2934+
2935+
if (!hasDefault())
2936+
return nullptr;
2937+
2938+
assert(getDefaultBB()->getNumArguments() == 0 && "precondition");
2939+
2940+
auto enumTy = getOperand()->getType();
2941+
NullablePtr<EnumElementDecl> uniqueCase = getUniqueCaseForDefault();
2942+
2943+
// Without a unique default case, the OSSA result simply forwards the
2944+
// switch_enum operand.
2945+
if (!uniqueCase)
2946+
return createResult(getDefaultBB(), enumTy);
2947+
2948+
// With a unique default case, the result is materialized exactly the same way
2949+
// as a matched result. It has a value iff the unique case has a payload.
2950+
if (!uniqueCase.get()->hasAssociatedValues())
2951+
return nullptr;
2952+
2953+
auto resultTy = enumTy.getEnumElementType(uniqueCase.get(), f->getModule(),
2954+
f->getTypeExpansionContext());
2955+
return createResult(getDefaultBB(), resultTy);
2956+
}
2957+
2958+
SILPhiArgument *SwitchEnumInst::createOptionalSomeResult() {
2959+
auto someDecl = getModule().getASTContext().getOptionalSomeDecl();
2960+
auto someBB = getCaseDestination(someDecl);
2961+
return createResult(someBB, getOperand()->getType().unwrapOptionalType());
2962+
}

lib/SIL/IR/SILValue.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ ValueOwnershipKind::ValueOwnershipKind(StringRef S)
219219
.Case("unowned", OwnershipKind::Unowned)
220220
.Case("owned", OwnershipKind::Owned)
221221
.Case("guaranteed", OwnershipKind::Guaranteed)
222-
.Case("any", OwnershipKind::None)
222+
.Case("none", OwnershipKind::None)
223223
.Default(None);
224224
if (!Result.hasValue())
225225
llvm_unreachable("Invalid string representation of ValueOwnershipKind");

lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1190,7 +1190,7 @@ void ForwardingOperand::setOwnershipKind(ValueOwnershipKind newKind) const {
11901190

11911191
if (auto *ofti = dyn_cast<OwnershipForwardingTermInst>(user)) {
11921192
assert(ofti->getNumOperands() == 1);
1193-
if (!ofti->getOperand(0)->getType().isTrivial(*ofti->getFunction())) {
1193+
if (!ofti->getOperand()->getType().isTrivial(*ofti->getFunction())) {
11941194
ofti->setForwardingOwnershipKind(newKind);
11951195

11961196
// Then convert all of its incoming values that are owned to be guaranteed.

0 commit comments

Comments
 (0)