Skip to content

Commit d74dbeb

Browse files
gottesmmgottesmm
committed
[ownership] Make checked_cast_br a true Ownership Forwarding inst instead of always inferring from the SILPhiArguments.
This is an analogous change to the previous change I just made to SwitchEnumInst.
1 parent 6f60a17 commit d74dbeb

File tree

5 files changed

+48
-199
lines changed

5 files changed

+48
-199
lines changed

‎include/swift/SIL/SILInstruction.h

Lines changed: 27 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8126,23 +8126,20 @@ class DynamicMethodBranchInst
81268126
};
81278127

81288128
/// The base class for cast instructions which are terminators.
8129-
class CastBranchInstBase : public TermInst {
8129+
template <typename BaseTy> class CastBranchInstBase : public BaseTy {
81308130
std::array<SILSuccessor, 2> DestBBs;
81318131

81328132
public:
8133-
8133+
template <typename... ArgTys>
81348134
CastBranchInstBase(SILInstructionKind K, SILDebugLocation DebugLoc,
81358135
SILBasicBlock *SuccessBB, SILBasicBlock *FailureBB,
8136-
ProfileCounter Target1Count = ProfileCounter(),
8137-
ProfileCounter Target2Count = ProfileCounter()) :
8138-
TermInst(K, DebugLoc),
8139-
DestBBs{{{this, SuccessBB, Target1Count},
8140-
{this, FailureBB, Target2Count}}}
8141-
{}
8136+
ProfileCounter Target1Count, ProfileCounter Target2Count,
8137+
ArgTys &&... args)
8138+
: BaseTy(K, DebugLoc, std::forward<ArgTys>(args)...),
8139+
DestBBs{{{this, SuccessBB, Target1Count},
8140+
{this, FailureBB, Target2Count}}} {}
81428141

8143-
SuccessorListTy getSuccessors() {
8144-
return DestBBs;
8145-
}
8142+
TermInst::SuccessorListTy getSuccessors() { return DestBBs; }
81468143

81478144
SILBasicBlock *getSuccessBB() { return DestBBs[0]; }
81488145
const SILBasicBlock *getSuccessBB() const { return DestBBs[0]; }
@@ -8157,7 +8154,7 @@ class CastBranchInstBase : public TermInst {
81578154

81588155
/// The base class for cast instructions which are terminators and have a
81598156
/// CastConsumptionKind.
8160-
class CastBranchWithConsumptionKindBase : public CastBranchInstBase {
8157+
class CastBranchWithConsumptionKindBase : public CastBranchInstBase<TermInst> {
81618158
CastConsumptionKind ConsumptionKind;
81628159

81638160
public:
@@ -8250,11 +8247,10 @@ class AddrCastInstBase
82508247
/// Perform a checked cast operation and branch on whether the cast succeeds.
82518248
/// The success branch destination block receives the cast result as a BB
82528249
/// argument.
8253-
class CheckedCastBranchInst final:
8254-
public UnaryInstructionWithTypeDependentOperandsBase<
8255-
SILInstructionKind::CheckedCastBranchInst,
8256-
CheckedCastBranchInst,
8257-
CastBranchInstBase> {
8250+
class CheckedCastBranchInst final
8251+
: public UnaryInstructionWithTypeDependentOperandsBase<
8252+
SILInstructionKind::CheckedCastBranchInst, CheckedCastBranchInst,
8253+
CastBranchInstBase<OwnershipForwardingTermInst>> {
82588254
friend SILBuilder;
82598255

82608256
SILType DestLoweredTy;
@@ -8266,12 +8262,12 @@ class CheckedCastBranchInst final:
82668262
ArrayRef<SILValue> TypeDependentOperands,
82678263
SILType DestLoweredTy, CanType DestFormalTy,
82688264
SILBasicBlock *SuccessBB, SILBasicBlock *FailureBB,
8269-
ProfileCounter Target1Count, ProfileCounter Target2Count)
8270-
: UnaryInstructionWithTypeDependentOperandsBase(DebugLoc, Operand,
8271-
TypeDependentOperands,
8272-
SuccessBB, FailureBB, Target1Count, Target2Count),
8273-
DestLoweredTy(DestLoweredTy),
8274-
DestFormalTy(DestFormalTy),
8265+
ProfileCounter Target1Count,
8266+
ProfileCounter Target2Count)
8267+
: UnaryInstructionWithTypeDependentOperandsBase(
8268+
DebugLoc, Operand, TypeDependentOperands, SuccessBB, FailureBB,
8269+
Target1Count, Target2Count, Operand.getOwnershipKind()),
8270+
DestLoweredTy(DestLoweredTy), DestFormalTy(DestFormalTy),
82758271
IsExact(IsExact) {}
82768272

82778273
static CheckedCastBranchInst *
@@ -8297,23 +8293,23 @@ class CheckedCastBranchInst final:
82978293
class CheckedCastValueBranchInst final
82988294
: public UnaryInstructionWithTypeDependentOperandsBase<
82998295
SILInstructionKind::CheckedCastValueBranchInst,
8300-
CheckedCastValueBranchInst,
8301-
CastBranchInstBase> {
8296+
CheckedCastValueBranchInst, CastBranchInstBase<TermInst>> {
83028297
friend SILBuilder;
83038298

83048299
CanType SourceFormalTy;
83058300
SILType DestLoweredTy;
83068301
CanType DestFormalTy;
83078302

8308-
CheckedCastValueBranchInst(SILDebugLocation DebugLoc,
8309-
SILValue Operand, CanType SourceFormalTy,
8303+
CheckedCastValueBranchInst(SILDebugLocation DebugLoc, SILValue Operand,
8304+
CanType SourceFormalTy,
83108305
ArrayRef<SILValue> TypeDependentOperands,
83118306
SILType DestLoweredTy, CanType DestFormalTy,
83128307
SILBasicBlock *SuccessBB, SILBasicBlock *FailureBB)
8313-
: UnaryInstructionWithTypeDependentOperandsBase(DebugLoc, Operand,
8314-
TypeDependentOperands, SuccessBB, FailureBB),
8315-
SourceFormalTy(SourceFormalTy),
8316-
DestLoweredTy(DestLoweredTy), DestFormalTy(DestFormalTy) {}
8308+
: UnaryInstructionWithTypeDependentOperandsBase(
8309+
DebugLoc, Operand, TypeDependentOperands, SuccessBB, FailureBB,
8310+
ProfileCounter(), ProfileCounter()),
8311+
SourceFormalTy(SourceFormalTy), DestLoweredTy(DestLoweredTy),
8312+
DestFormalTy(DestFormalTy) {}
83178313

83188314
static CheckedCastValueBranchInst *
83198315
create(SILDebugLocation DebugLoc,

‎lib/SIL/IR/OperandOwnership.cpp

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -458,31 +458,9 @@ OperandOwnershipKindClassifier::visitSwitchEnumInst(SwitchEnumInst *sei) {
458458
OperandOwnershipKindMap
459459
OperandOwnershipKindClassifier::visitCheckedCastBranchInst(
460460
CheckedCastBranchInst *ccbi) {
461-
// TODO: Simplify this using ValueOwnershipKind::merge.
462-
Optional<OperandOwnershipKindMap> map;
463-
for (auto argArray : ccbi->getSuccessorBlockArgumentLists()) {
464-
assert(!argArray.empty());
465-
466-
auto argOwnershipKind = argArray[getOperandIndex()]->getOwnershipKind();
467-
// If we do not have a map yet, initialize it and continue.
468-
if (!map) {
469-
auto lifetimeConstraint =
470-
argOwnershipKind.getForwardingLifetimeConstraint();
471-
map = Map::compatibilityMap(argOwnershipKind, lifetimeConstraint);
472-
continue;
473-
}
474-
475-
// Otherwise, make sure that we can accept the rest of our
476-
// arguments. If not, we return an empty ownership kind to make
477-
// sure that we flag everything as an error.
478-
if (map->canAcceptKind(argOwnershipKind)) {
479-
continue;
480-
}
481-
482-
return OperandOwnershipKindMap();
483-
}
484-
485-
return map.getValue();
461+
auto kind = getOwnershipKind();
462+
auto lifetimeConstraint = kind.getForwardingLifetimeConstraint();
463+
return Map::compatibilityMap(kind, lifetimeConstraint);
486464
}
487465

488466
//// FIX THIS HERE

‎lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3692,6 +3692,18 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
36923692
CBI->getOperand()->getType(),
36933693
"failure dest block argument must match type of original type in "
36943694
"ownership qualified sil");
3695+
auto succOwnershipKind =
3696+
CBI->getSuccessBB()->args_begin()[0]->getOwnershipKind();
3697+
require(succOwnershipKind.isCompatibleWith(
3698+
CBI->getOperand().getOwnershipKind()),
3699+
"succ dest block argument must have ownership compatible with "
3700+
"the checked_cast_br operand");
3701+
auto failOwnershipKind =
3702+
CBI->getFailureBB()->args_begin()[0]->getOwnershipKind();
3703+
require(failOwnershipKind.isCompatibleWith(
3704+
CBI->getOperand().getOwnershipKind()),
3705+
"failure dest block argument must have ownership compatible with "
3706+
"the checked_cast_br operand");
36953707
} else {
36963708
require(CBI->getFailureBB()->args_empty(),
36973709
"Failure dest of checked_cast_br must not take any argument in "

‎lib/SILOptimizer/SemanticARC/OwnershipLiveRange.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,12 @@ static void convertInstructionOwnership(SILInstruction *i,
280280
if (mvir->getOwnershipKind() == oldOwnership) {
281281
mvir->setOwnershipKind(newOwnership);
282282
}
283+
if (auto *base = dyn_cast<OwnershipForwardingMultipleValueInstruction>(
284+
mvir->getParent())) {
285+
if (base->getOwnershipKind() == oldOwnership) {
286+
base->setOwnershipKind(newOwnership);
287+
}
288+
}
283289
continue;
284290
}
285291

‎test/SIL/ownership-verifier/over_consume.sil

Lines changed: 0 additions & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -218,149 +218,6 @@ bb3:
218218
return %9999 : $()
219219
}
220220

221-
// CHECK-LABEL: Function: 'checked_cast_br_mismatching_argument_guaranteed_to_owned_1'
222-
// CHECK: Have operand with incompatible ownership?!
223-
// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
224-
// CHECK: User: checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
225-
// CHECK: Conv: guaranteed
226-
// CHECK: OwnershipMap:
227-
// CHECK: -- OperandOwnershipKindMap --
228-
// CHECK: unowned: No.
229-
// CHECK: owned: Yes. Liveness: LifetimeEnding
230-
// CHECK: guaranteed: No.
231-
// CHECK: any: Yes. Liveness: NonLifetimeEnding
232-
sil [ossa] @checked_cast_br_mismatching_argument_guaranteed_to_owned_1 : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
233-
bb0(%0 : @guaranteed $Builtin.NativeObject):
234-
checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
235-
236-
bb1(%1 : @owned $SuperKlass):
237-
destroy_value %1 : $SuperKlass
238-
br bb3
239-
240-
bb2(%2 : @owned $Builtin.NativeObject):
241-
destroy_value %2 : $Builtin.NativeObject
242-
br bb3
243-
244-
bb3:
245-
%9999 = tuple()
246-
return %9999 : $()
247-
}
248-
249-
// CHECK-LABEL: Function: 'checked_cast_br_mismatching_argument_guaranteed_to_owned_2'
250-
// CHECK: Ill-formed SIL! Unable to compute ownership kind map for user?!
251-
// CHECK: For terminator users, check that successors have compatible ownership kinds.
252-
// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
253-
// CHECK: User: checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
254-
// CHECK: Conv: guaranteed
255-
sil [ossa] @checked_cast_br_mismatching_argument_guaranteed_to_owned_2 : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
256-
bb0(%0 : @guaranteed $Builtin.NativeObject):
257-
checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
258-
259-
bb1(%1 : @guaranteed $SuperKlass):
260-
br bb3
261-
262-
bb2(%2 : @owned $Builtin.NativeObject):
263-
destroy_value %2 : $Builtin.NativeObject
264-
br bb3
265-
266-
bb3:
267-
%9999 = tuple()
268-
return %9999 : $()
269-
}
270-
271-
// CHECK-LABEL: Function: 'checked_cast_br_mismatching_argument_guaranteed_to_owned_3'
272-
// CHECK: Ill-formed SIL! Unable to compute ownership kind map for user?!
273-
// CHECK: For terminator users, check that successors have compatible ownership kinds.
274-
// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
275-
// CHECK: User: checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
276-
// CHECK: Conv: guaranteed
277-
sil [ossa] @checked_cast_br_mismatching_argument_guaranteed_to_owned_3 : $@convention(thin) (@guaranteed Builtin.NativeObject) -> () {
278-
bb0(%0 : @guaranteed $Builtin.NativeObject):
279-
checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
280-
281-
bb1(%1 : @owned $SuperKlass):
282-
destroy_value %1 : $SuperKlass
283-
br bb3
284-
285-
bb2(%2 : @guaranteed $Builtin.NativeObject):
286-
br bb3
287-
288-
bb3:
289-
%9999 = tuple()
290-
return %9999 : $()
291-
}
292-
293-
// CHECK-LABEL: Function: 'checked_cast_br_mismatching_argument_owned_to_guaranteed_1'
294-
// CHECK: Have operand with incompatible ownership?!
295-
// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
296-
// CHECK: User: checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
297-
// CHECK: Conv: owned
298-
// CHECK: OwnershipMap:
299-
// CHECK: -- OperandOwnershipKindMap --
300-
// CHECK: unowned: No.
301-
// CHECK: owned: No.
302-
// CHECK: guaranteed: Yes. Liveness: NonLifetimeEnding
303-
// CHECK: any: Yes. Liveness: NonLifetimeEnding
304-
sil [ossa] @checked_cast_br_mismatching_argument_owned_to_guaranteed_1 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
305-
bb0(%0 : @owned $Builtin.NativeObject):
306-
checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
307-
308-
bb1(%1 : @guaranteed $SuperKlass):
309-
br bb3
310-
311-
bb2(%2 : @guaranteed $Builtin.NativeObject):
312-
br bb3
313-
314-
bb3:
315-
%9999 = tuple()
316-
return %9999 : $()
317-
}
318-
319-
320-
// CHECK-LABEL: Function: 'checked_cast_br_mismatching_argument_owned_to_guaranteed_2'
321-
// CHECK: Ill-formed SIL! Unable to compute ownership kind map for user?!
322-
// CHECK: For terminator users, check that successors have compatible ownership kinds.
323-
// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
324-
// CHECK: User: checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
325-
// CHECK: Conv: owned
326-
sil [ossa] @checked_cast_br_mismatching_argument_owned_to_guaranteed_2 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
327-
bb0(%0 : @owned $Builtin.NativeObject):
328-
checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
329-
330-
bb1(%1 : @guaranteed $SuperKlass):
331-
br bb3
332-
333-
bb2(%2 : @owned $Builtin.NativeObject):
334-
destroy_value %2 : $Builtin.NativeObject
335-
br bb3
336-
337-
bb3:
338-
%9999 = tuple()
339-
return %9999 : $()
340-
}
341-
342-
// CHECK-LABEL: Function: 'checked_cast_br_mismatching_argument_owned_to_guaranteed_3'
343-
// CHECK: Ill-formed SIL! Unable to compute ownership kind map for user?!
344-
// CHECK: For terminator users, check that successors have compatible ownership kinds.
345-
// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject
346-
// CHECK: User: checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
347-
// CHECK: Conv: owned
348-
sil [ossa] @checked_cast_br_mismatching_argument_owned_to_guaranteed_3 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
349-
bb0(%0 : @owned $Builtin.NativeObject):
350-
checked_cast_br %0 : $Builtin.NativeObject to SuperKlass, bb1, bb2
351-
352-
bb1(%1 : @owned $SuperKlass):
353-
destroy_value %1 : $SuperKlass
354-
br bb3
355-
356-
bb2(%2 : @guaranteed $Builtin.NativeObject):
357-
br bb3
358-
359-
bb3:
360-
%9999 = tuple()
361-
return %9999 : $()
362-
}
363-
364221
// CHECK-LABEL: Function: 'checked_cast_br_guaranteed_arg_outlives_original_value'
365222
// CHECK: Found outside of lifetime use?!
366223
// CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject

0 commit comments

Comments
 (0)