Skip to content

SILMem2Reg: fix a problem with leaking enum values #41744

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.

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

Merged
merged 1 commit into from
Mar 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions lib/SILOptimizer/Transforms/SILMem2Reg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1757,6 +1757,14 @@ bool MemoryToRegisters::promoteSingleAllocation(AllocStackInst *alloc) {
b.createDeallocStack(next->getLoc(), alloc);
}
return true;
} else {
// For enums we require that all uses are in the same block.
// Otherwise there could be a switch_enum of an optional where the none-case
// does not have a destroy of the enum value.
// After transforming such an alloc_stack, the value would leak in the none-
// case block.
if (f.hasOwnership() && alloc->getType().isOrHasEnum())
return false;
}

LLVM_DEBUG(llvm::dbgs() << "*** Need to insert BB arguments for " << *alloc);
Expand Down
2 changes: 1 addition & 1 deletion test/SILOptimizer/mem2reg_lifetime.sil
Original file line number Diff line number Diff line change
Expand Up @@ -1328,9 +1328,9 @@ entry(%either : @owned $E):
%addr = alloc_stack [lexical] $E
store %either to [init] %addr : $*E
%either2 = load [take] %addr : $*E
dealloc_stack %addr : $*E
switch_enum %either2 : $E, case #E.one!enumelt: one
one(%instance : @owned $C):
dealloc_stack %addr : $*E
return %instance : $C
}

Expand Down
3 changes: 2 additions & 1 deletion test/SILOptimizer/mem2reg_lifetime_nontrivial.sil
Original file line number Diff line number Diff line change
Expand Up @@ -1101,7 +1101,8 @@ enum KlassOptional {
sil @return_optional_or_error : $@convention(thin) () -> (@owned KlassOptional, @error Error)

// CHECK-LABEL: sil [ossa] @test_deadphi4 :
// CHECK-NOT: alloc_stack
// mem2reg cannot optimize an enum location which spans over multiple blocks.
// CHECK: alloc_stack
// CHECK-LABEL: } // end sil function 'test_deadphi4'
sil [ossa] @test_deadphi4 : $@convention(thin) (@owned KlassOptional) -> () {
bb0(%0 : @owned $KlassOptional):
Expand Down
50 changes: 50 additions & 0 deletions test/SILOptimizer/mem2reg_ossa.sil
Original file line number Diff line number Diff line change
Expand Up @@ -488,3 +488,53 @@ bb0:
dealloc_stack %1 : $*((), ())
return %16 : $((), ())
}

// CHECK-LABEL: sil [ossa] @dont_optimize_optional_in_multiple_blocks :
// CHECK: alloc_stack
// CHECK: } // end sil function 'dont_optimize_optional_in_multiple_blocks'
sil [ossa] @dont_optimize_optional_in_multiple_blocks : $@convention(method) (@guaranteed Optional<Klass>) -> () {
bb0(%0 : @guaranteed $Optional<Klass>):
%1 = copy_value %0 : $Optional<Klass>
%32 = alloc_stack $Optional<Klass>
store %1 to [init] %32 : $*Optional<Klass>
switch_enum %0 : $Optional<Klass>, case #Optional.some!enumelt: bb6, case #Optional.none!enumelt: bb5

bb5:
dealloc_stack %32 : $*Optional<Klass>
br bb7

bb6(%50 : @guaranteed $Klass):
%53 = load [take] %32 : $*Optional<Klass>
destroy_value %53 : $Optional<Klass>
dealloc_stack %32 : $*Optional<Klass>
br bb7

bb7:
%r = tuple ()
return %r : $()
}

// CHECK-LABEL: sil [ossa] @optimize_optional_in_single_block :
// CHECK-NOT: alloc_stack
// CHECK: } // end sil function 'optimize_optional_in_single_block'
sil [ossa] @optimize_optional_in_single_block : $@convention(method) (@guaranteed Optional<Klass>) -> () {
bb0(%0 : @guaranteed $Optional<Klass>):
switch_enum %0 : $Optional<Klass>, case #Optional.some!enumelt: bb6, case #Optional.none!enumelt: bb5

bb5:
br bb7

bb6(%50 : @guaranteed $Klass):
%32 = alloc_stack $Optional<Klass>
%1 = copy_value %0 : $Optional<Klass>
store %1 to [init] %32 : $*Optional<Klass>
%53 = load [take] %32 : $*Optional<Klass>
destroy_value %53 : $Optional<Klass>
dealloc_stack %32 : $*Optional<Klass>
br bb7

bb7:
%r = tuple ()
return %r : $()
}

3 changes: 2 additions & 1 deletion test/SILOptimizer/mem2reg_ossa_nontrivial.sil
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,8 @@ enum KlassOptional {
sil @return_optional_or_error : $@convention(thin) () -> (@owned KlassOptional, @error Error)

// CHECK-LABEL: sil [ossa] @test_deadphi4 :
// CHECK-NOT: alloc_stack
// mem2reg cannot optimize an enum location which spans over multiple blocks.
// CHECK: alloc_stack
// CHECK-LABEL: } // end sil function 'test_deadphi4'
sil [ossa] @test_deadphi4 : $@convention(thin) (@owned KlassOptional) -> () {
bb0(%0 : @owned $KlassOptional):
Expand Down