Skip to content

Commit ac92166

Browse files
committed
[silgen] Add support for BorrowAlways, eliminate some bogus asserts, and only use BorrowAlways with objects.
The reason why the asserts were bogus is that even with SILOwnership enabled, we will use the old pattern to emit address only code. In that case, the asserts will fire on good behavior. I also fixed a latent bug where when ownership was enabled we were treating address only values like objects. Found via inspection. rdar://29791263
1 parent c6865c0 commit ac92166

File tree

1 file changed

+15
-19
lines changed

1 file changed

+15
-19
lines changed

lib/SILGen/SILGenPattern.cpp

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -738,15 +738,19 @@ forwardIntoSubtree(SILGenFunction &SGF, SILLocation loc,
738738
ManagedValue outerMV = outerCMV.getFinalManagedValue();
739739
if (!outerMV.hasCleanup()) return outerCMV;
740740

741-
assert(outerCMV.getFinalConsumption() != CastConsumptionKind::CopyOnSuccess
742-
&& "copy-on-success value with cleanup?");
741+
auto consumptionKind = outerCMV.getFinalConsumption();
742+
(void)consumptionKind;
743+
assert((consumptionKind == CastConsumptionKind::TakeAlways ||
744+
consumptionKind == CastConsumptionKind::TakeOnSuccess) &&
745+
"non-+1 consumption with a cleanup?");
743746
scope.pushCleanupState(outerMV.getCleanup(),
744747
CleanupState::PersistentlyActive);
745748

746-
// If SILOwnership is enabled, we always forward down values as borrows that
747-
// are copied on success.
748-
if (SGF.F.getModule().getOptions().EnableSILOwnership) {
749-
return {outerMV.borrow(SGF, loc), CastConsumptionKind::CopyOnSuccess};
749+
// If SILOwnership is enabled and we have an object, borrow instead of take on
750+
// success.
751+
if (SGF.F.getModule().getOptions().EnableSILOwnership &&
752+
outerMV.getType().isObject()) {
753+
return {outerMV.borrow(SGF, loc), CastConsumptionKind::BorrowAlways};
750754
}
751755

752756
// Success means that we won't end up in the other branch,
@@ -766,9 +770,6 @@ static void forwardIntoIrrefutableSubtree(SILGenFunction &SGF,
766770

767771
assert(outerCMV.getFinalConsumption() != CastConsumptionKind::CopyOnSuccess
768772
&& "copy-on-success value with cleanup?");
769-
assert((!SGF.F.getModule().getOptions().EnableSILOwnership ||
770-
outerCMV.getFinalConsumption() == CastConsumptionKind::TakeAlways) &&
771-
"When semantic sil is enabled, we should never see TakeOnSuccess");
772773
scope.pushCleanupState(outerMV.getCleanup(),
773774
CleanupState::PersistentlyActive);
774775

@@ -899,7 +900,8 @@ class ArgUnforwarder {
899900

900901
static bool requiresUnforwarding(SILGenFunction &SGF,
901902
ConsumableManagedValue operand) {
902-
if (SGF.F.getModule().getOptions().EnableSILOwnership) {
903+
if (SGF.F.getModule().getOptions().EnableSILOwnership &&
904+
operand.getType().isObject()) {
903905
assert(operand.getFinalConsumption() !=
904906
CastConsumptionKind::TakeOnSuccess &&
905907
"When compiling with sil ownership take on success is disabled");
@@ -1356,9 +1358,6 @@ getManagedSubobject(SILGenFunction &SGF, SILValue value,
13561358
return {ManagedValue::forUnmanaged(value), consumption};
13571359
case CastConsumptionKind::TakeAlways:
13581360
case CastConsumptionKind::TakeOnSuccess:
1359-
assert((!SGF.F.getModule().getOptions().EnableSILOwnership ||
1360-
consumption != CastConsumptionKind::TakeOnSuccess) &&
1361-
"TakeOnSuccess should never be used when sil ownership is enabled");
13621361
return {SGF.emitManagedRValueWithCleanup(value, valueTL), consumption};
13631362
}
13641363
}
@@ -1882,8 +1881,6 @@ void PatternMatchEmission::emitEnumElementDispatch(
18821881
break;
18831882

18841883
case CastConsumptionKind::TakeOnSuccess:
1885-
assert(!SGF.F.getModule().getOptions().EnableSILOwnership &&
1886-
"TakeOnSuccess is not supported when compiling with ownership");
18871884
// If any of the specialization cases is refutable, we must copy.
18881885
if (!blocks.hasAnyRefutableCase())
18891886
break;
@@ -1966,8 +1963,6 @@ void PatternMatchEmission::emitEnumElementDispatch(
19661963
auto eltConsumption = src.getFinalConsumption();
19671964
if (caseInfo.Irrefutable &&
19681965
eltConsumption == CastConsumptionKind::TakeOnSuccess) {
1969-
assert(!SGF.F.getModule().getOptions().EnableSILOwnership &&
1970-
"TakeOnSuccess is not supported when compiling with ownership");
19711966
eltConsumption = CastConsumptionKind::TakeAlways;
19721967
}
19731968

@@ -2760,8 +2755,9 @@ void SILGenFunction::emitCatchDispatch(DoCatchStmt *S, ManagedValue exn,
27602755
// Set up an initial clause matrix.
27612756
ClauseMatrix clauseMatrix(clauseRows);
27622757
ConsumableManagedValue subject;
2763-
if (F.getModule().getOptions().EnableSILOwnership) {
2764-
subject = {exn.borrow(*this, S), CastConsumptionKind::CopyOnSuccess};
2758+
if (F.getModule().getOptions().EnableSILOwnership &&
2759+
exn.getType().isObject()) {
2760+
subject = {exn.borrow(*this, S), CastConsumptionKind::BorrowAlways};
27652761
} else {
27662762
subject = {exn, CastConsumptionKind::TakeOnSuccess};
27672763
}

0 commit comments

Comments
 (0)