Skip to content

[SILGen] Use temporary initializer for switch address subject. #32072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[SILGen] Use temporary initializer for switch address subject. #32072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions lib/SILGen/SILGenPattern.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2126,8 +2126,12 @@ void PatternMatchEmission::emitEnumElementDispatch(
llvm_unreachable("not allowed");
case CastConsumptionKind::CopyOnSuccess: {
auto copy = SGF.emitTemporaryAllocation(loc, srcValue->getType());
// If the element is trivial it won't be destroyed so we need to make
// sure we destroy the enum here.
if (eltTL->isTrivial())
copy = SGF.emitManagedBufferWithCleanup(copy).getValue();
SGF.B.createCopyAddr(loc, srcValue, copy, IsNotTake, IsInitialization);
// We can always take from the copy.
// We can always take from the managedCopy.
eltConsumption = CastConsumptionKind::TakeAlways;
eltValue = SGF.B.createUncheckedTakeEnumDataAddr(loc, copy, elt, eltTy);
break;
Expand Down Expand Up @@ -2758,8 +2762,17 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {

// Emit the subject value. If at +1, dispatching will consume it. If it is at
// +0, we just forward down borrows.
ManagedValue subjectMV = emitRValueAsSingleValue(
S->getSubjectExpr(), SGFContext::AllowGuaranteedPlusZero);
// If this is an address type, use an initializer.
ManagedValue subjectMV;
if (getTypeLowering(S->getSubjectExpr()->getType()).isAddress()) {
auto init = emitTemporary(S->getSubjectExpr(), getTypeLowering(S->getSubjectExpr()->getType()));
emitExprInto(S->getSubjectExpr(), init.get());
subjectMV = emitManagedRValueWithCleanup(init->getAddressForInPlaceInitialization(*this, S->getSubjectExpr()));
subjectMV = ManagedValue::forUnmanaged(subjectMV.forward(*this));
} else {
subjectMV = emitRValueAsSingleValue(S->getSubjectExpr(),
SGFContext::AllowGuaranteedPlusZero);
}

// Inline constructor for subject.
auto subject = ([&]() -> ConsumableManagedValue {
Expand Down Expand Up @@ -2787,7 +2800,7 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
// If we have an address only type returned without a cleanup, we
// need to do a copy just to be safe. So for efficiency we pass it
// down take_always.
return {subjectMV.copy(*this, S), CastConsumptionKind::TakeAlways};
return {subjectMV, CastConsumptionKind::CopyOnSuccess};
}());

// If we need to diagnose an unexpected enum case or unexpected enum case
Expand Down
20 changes: 20 additions & 0 deletions test/Interpreter/switch.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,4 +282,24 @@ SwitchTestSuite.test("Enum Initialization Leaks") {
}
}

SwitchTestSuite.test("Destroy all elements: protocol and trivial") {
enum ProtocolAndTrivial {
case a(P)
case b(Int)
case c(LifetimeTracked)
}

func testEnumWithProtocolAndTrivial(_ e: ProtocolAndTrivial) -> Bool {
switch (e) {
case .a(_): return true
case .b(_): return true
case .c(_): return true
}
}

do {
testEnumWithProtocolAndTrivial(.c(LifetimeTracked(0)))
}
}

runAllTests()
46 changes: 46 additions & 0 deletions test/SILGen/destroy_enum_in_all_cases.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// RUN: %target-swift-emit-silgen -module-name switch %s > %t.sil
// RUN: %target-sil-opt %t.sil -sil-combine -enable-sil-verify-all | %FileCheck %s

protocol P { }

enum ProtocolAndTrivial {
case a(P)
case b(Int)
}

// This test is mainly checking the verifier doesn't error but, just in case,
// there's also a filecheck test.
// CHECK-LABEL: @$s6switch4testyyAA18ProtocolAndTrivialO_ADtF
// CHECK: [[C1:%[0-9]+]] = alloc_stack $(ProtocolAndTrivial, ProtocolAndTrivial)

// CHECK: [[CA:%[0-9]+]] = tuple_element_addr [[C1]] : $*(ProtocolAndTrivial, ProtocolAndTrivial), 0
// CHECK: [[CB:%[0-9]+]] = tuple_element_addr [[C1]] : $*(ProtocolAndTrivial, ProtocolAndTrivial), 1

// CHECK: copy_addr %0 to [initialization] [[CA]]
// CHECK: copy_addr %1 to [initialization] [[CB]]

// CHECK: [[CA:%[0-9]+]] = tuple_element_addr [[C1]] : $*(ProtocolAndTrivial, ProtocolAndTrivial), 0
// CHECK: [[CB:%[0-9]+]] = tuple_element_addr [[C1]] : $*(ProtocolAndTrivial, ProtocolAndTrivial), 1

// CHECK: switch_enum_addr [[CA]] : $*ProtocolAndTrivial, case #ProtocolAndTrivial.a!enumelt: [[BBA:bb[0-9]+]], case #ProtocolAndTrivial.b!enumelt: [[BBB:bb[0-9]+]]

// We're only testing the trivial case.
// CHECK: [[BBB]]:
// CHECK: [[TMP:%[0-9]+]] = alloc_stack $ProtocolAndTrivial
// CHECK: switch_enum_addr [[CB]] : $*ProtocolAndTrivial, case #ProtocolAndTrivial.b!enumelt: [[BOTH_B:bb[0-9]+]], default [[DEFAULT:bb[0-9]+]]

// Make sure that we destroy everything.
// CHECK: [[BOTH_B]]:
// CHECK: [[TMP2:%[0-9]+]] = alloc_stack $ProtocolAndTrivial
// CHECK: destroy_addr [[TMP2]]
// CHECK: destroy_addr [[TMP]]
// CHECK: destroy_addr [[C1]]

// CHECK-LABEL: end sil function '$s6switch4testyyAA18ProtocolAndTrivialO_ADtF'
func test(_ a : ProtocolAndTrivial, _ b : ProtocolAndTrivial) {
switch ((a, b)) {
case (.a(_), .a(_)): _ = ()
case (.b(_), .b(_)): _ = ()
default: _ = ()
}
}
16 changes: 13 additions & 3 deletions test/SILGen/enum_resilience.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,36 @@ import resilient_enum
// a default case

// CHECK-LABEL: sil hidden [ossa] @$s15enum_resilience15resilientSwitchyy0c1_A06MediumOF : $@convention(thin) (@in_guaranteed Medium) -> ()
// CHECK: [[BOX:%.*]] = alloc_stack $Medium
// CHECK: [[BOX:%.*]] = alloc_stack $Medium
// CHECK-NEXT: copy_addr %0 to [initialization] [[BOX]]
// CHECK-NEXT: [[METATYPE:%.+]] = value_metatype $@thick Medium.Type, [[BOX]] : $*Medium
// CHECK-NEXT: switch_enum_addr [[BOX]] : $*Medium, case #Medium.Paper!enumelt: bb1, case #Medium.Canvas!enumelt: bb2, case #Medium.Pamphlet!enumelt: bb3, case #Medium.Postcard!enumelt: bb4, default bb5
// CHECK: bb1:
// CHECK-NEXT: destroy_addr [[BOX]]
// CHECK-NEXT: dealloc_stack [[BOX]]
// CHECK-NEXT: br bb6
// CHECK: bb2:
// CHECK-NEXT: destroy_addr [[BOX]]
// CHECK-NEXT: dealloc_stack [[BOX]]
// CHECK-NEXT: br bb6
// CHECK: bb3:
// CHECK-NEXT: [[INDIRECT_ADDR:%.*]] = unchecked_take_enum_data_addr [[BOX]]
// CHECK-NEXT: [[TMP:%.*]] = alloc_stack $Medium
// CHECK-NEXT: copy_addr
// CHECK-NEXT: [[INDIRECT_ADDR:%.*]] = unchecked_take_enum_data_addr [[TMP]]
// CHECK-NEXT: [[INDIRECT:%.*]] = load [take] [[INDIRECT_ADDR]]
// CHECK-NEXT: [[PAYLOAD:%.*]] = project_box [[INDIRECT]]
// CHECK-NEXT: destroy_value [[INDIRECT]]
// CHECK-NEXT: dealloc_stack [[TMP]]
// CHECK-NEXT: destroy_addr [[BOX]]
// CHECK-NEXT: dealloc_stack [[BOX]]
// CHECK-NEXT: br bb6
// CHECK: bb4:
// CHECK-NEXT: [[PAYLOAD_ADDR:%.*]] = unchecked_take_enum_data_addr [[BOX]]
// CHECK-NEXT: [[TMP:%.*]] = alloc_stack $Medium
// CHECK-NEXT: copy_addr
// CHECK-NEXT: [[PAYLOAD_ADDR:%.*]] = unchecked_take_enum_data_addr [[TMP]]
// CHECK-NEXT: destroy_addr [[PAYLOAD_ADDR]]
// CHECK-NEXT: dealloc_stack [[TMP]]
// CHECK-NEXT: destroy_addr [[BOX]]
// CHECK-NEXT: dealloc_stack [[BOX]]
// CHECK-NEXT: br bb6
// CHECK: bb5:
Expand Down
14 changes: 12 additions & 2 deletions test/SILGen/enum_resilience_testable.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,31 @@
// CHECK-NEXT: [[METATYPE:%.+]] = value_metatype $@thick Medium.Type, [[BOX]] : $*Medium
// CHECK-NEXT: switch_enum_addr [[BOX]] : $*Medium, case #Medium.Paper!enumelt: bb1, case #Medium.Canvas!enumelt: bb2, case #Medium.Pamphlet!enumelt: bb3, case #Medium.Postcard!enumelt: bb4, default bb5
// CHECK: bb1:
// CHECK-NEXT: destroy_addr [[BOX]]
// CHECK-NEXT: dealloc_stack [[BOX]]
// CHECK-NEXT: br bb6
// CHECK: bb2:
// CHECK-NEXT: destroy_addr [[BOX]]
// CHECK-NEXT: dealloc_stack [[BOX]]
// CHECK-NEXT: br bb6
// CHECK: bb3:
// CHECK-NEXT: [[INDIRECT_ADDR:%.*]] = unchecked_take_enum_data_addr [[BOX]]
// CHECK-NEXT: [[TMP:%.*]] = alloc_stack $Medium
// CHECK-NEXT: copy_addr
// CHECK-NEXT: [[INDIRECT_ADDR:%.*]] = unchecked_take_enum_data_addr [[TMP]]
// CHECK-NEXT: [[INDIRECT:%.*]] = load [take] [[INDIRECT_ADDR]]
// CHECK-NEXT: [[PAYLOAD:%.*]] = project_box [[INDIRECT]]
// CHECK-NEXT: destroy_value [[INDIRECT]]
// CHECK-NEXT: dealloc_stack [[TMP]]
// CHECK-NEXT: destroy_addr [[BOX]]
// CHECK-NEXT: dealloc_stack [[BOX]]
// CHECK-NEXT: br bb6
// CHECK: bb4:
// CHECK-NEXT: [[PAYLOAD_ADDR:%.*]] = unchecked_take_enum_data_addr [[BOX]]
// CHECK-NEXT: [[TMP:%.*]] = alloc_stack $Medium
// CHECK-NEXT: copy_addr
// CHECK-NEXT: [[PAYLOAD_ADDR:%.*]] = unchecked_take_enum_data_addr [[TMP]]
// CHECK-NEXT: destroy_addr [[PAYLOAD_ADDR]]
// CHECK-NEXT: dealloc_stack [[TMP]]
// CHECK-NEXT: destroy_addr [[BOX]]
// CHECK-NEXT: dealloc_stack [[BOX]]
// CHECK-NEXT: br bb6
// CHECK: bb5:
Expand Down
3 changes: 2 additions & 1 deletion test/SILGen/indirect_enum.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ func switchTreeA<T>(_ x: TreeA<T>) {

// CHECK-LABEL: sil hidden [ossa] @$s13indirect_enum11switchTreeB{{[_0-9a-zA-Z]*}}F
func switchTreeB<T>(_ x: TreeB<T>) {
// CHECK: copy_addr %0 to [initialization] [[SCRATCH:%.*]] :
// CHECK: [[SCRATCH:%.*]] = alloc_stack
// CHECK: copy_addr %0 to [initialization] [[SCRATCH]]
// CHECK: switch_enum_addr [[SCRATCH]]
switch x {

Expand Down
Loading