From c3e91ce183a29d2f67720c1107b50d359cfc24b9 Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Thu, 13 Apr 2023 14:28:59 -0700 Subject: [PATCH 1/2] [SemanticARCOpts] Keep owned lexical lifetimes. Prevent joining lifetimes of copies with destroyed owned lexical values if there may be deinit barriers between the final consume of the copy and the destroy of the lexical value. rdar://108014714 --- .../SemanticARC/CopyValueOpts.cpp | 15 +++++ .../semantic-arc-opts-lifetime-joining.sil | 55 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp b/lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp index 6cdbe18af023b..b5566c9485a89 100644 --- a/lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp +++ b/lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp @@ -24,6 +24,7 @@ #include "SemanticARCOptVisitor.h" #include "swift/Basic/Defer.h" #include "swift/SIL/LinearLifetimeChecker.h" +#include "swift/SIL/MemAccessUtils.h" #include "swift/SIL/OwnershipUtils.h" #include "swift/SIL/Projection.h" @@ -392,6 +393,20 @@ static bool tryJoinIfDestroyConsumingUseInSameBlock( return true; } + // The lifetime of the original ends after the lifetime of the copy. If the + // original is lexical, its lifetime must not be shortened through deinit + // barriers. + if (cvi->getOperand()->isLexical()) { + // At this point, visitedInsts contains all the instructions between the + // consuming use of the copy and the destroy. If any of those instructions + // is a deinit barrier, it would be illegal to shorten the original lexical + // value's lifetime to end at that consuming use. Bail if any are. + if (llvm::any_of(visitedInsts, [](auto *inst) { + return mayBeDeinitBarrierNotConsideringSideEffects(inst); + })) + return false; + } + // If we reached this point, isUseBetweenInstAndBlockEnd succeeded implying // that we found destroy_value to be after our consuming use. Noting that // additionally, the routine places all instructions in between consuming use diff --git a/test/SILOptimizer/semantic-arc-opts-lifetime-joining.sil b/test/SILOptimizer/semantic-arc-opts-lifetime-joining.sil index c053d616ffbc3..8b42b22a8463a 100644 --- a/test/SILOptimizer/semantic-arc-opts-lifetime-joining.sil +++ b/test/SILOptimizer/semantic-arc-opts-lifetime-joining.sil @@ -116,6 +116,13 @@ struct NativeObjectWrapper { sil @owned_user_object_pair : $@convention(thin) (@owned NativeObjectPair) -> () +class X {} +struct S {} + +sil @getX : $@convention(thin) () -> @owned X +sil @getS : $@convention(thin) (@owned X) -> @out S +sil @loadWeakX_from : $@convention(thin) (@in_guaranteed S) -> @owned FakeOptional + /////////// // Tests // /////////// @@ -923,3 +930,51 @@ bb0: destroy_value %0 : ${ var Bool } return %11 : $Bool } + +// Don't do this optimization: +// Eliminate copy of lexical value which ends after lifetime of copy IF there +// may be deinit barriers between the final consume and the copy. +// +// CHECK-LABEL: sil [ossa] @testDestroyedLexicalValue : {{.*}} { +// CHECK: [[GET:%[^,]+]] = function_ref @getX +// CHECK: [[X:%[^,]+]] = apply [[GET]]() +// CHECK: [[MX:%[^,]+]] = move_value [lexical] [[X]] +// CHECK: destroy_value [[MX]] : $X +// CHECK-LABEL: } // end sil function 'testDestroyedLexicalValue' +sil [ossa] @testDestroyedLexicalValue : $@convention(thin) () -> @owned FakeOptional { +bb0: + %getX = function_ref @getX : $@convention(thin) () -> @owned X + %x = apply %getX() : $@convention(thin) () -> @owned X + %mx = move_value [lexical] %x : $X + %a = alloc_stack [lexical] $S, let, name "s" + %c = copy_value %mx : $X + %getS = function_ref @getS : $@convention(thin) (@owned X) -> @out S + %s = apply %getS(%a, %c) : $@convention(thin) (@owned X) -> @out S + %loadWeakX_from = function_ref @loadWeakX_from : $@convention(thin) (@in_guaranteed S) -> @owned FakeOptional + %o = apply %loadWeakX_from(%a) : $@convention(thin) (@in_guaranteed S) -> @owned FakeOptional + // ^^^^^ Deinit barrier + destroy_addr %a : $*S + dealloc_stack %a : $*S + destroy_value %mx : $X + return %o : $FakeOptional +} + +// CHECK-LABEL: sil [ossa] @testDestroyedLexicalValueNoBarriers : {{.*}} { +// CHECK-NOT: destroy_value +// CHECK-LABEL: } // end sil function 'testDestroyedLexicalValueNoBarriers' +sil [ossa] @testDestroyedLexicalValueNoBarriers : $@convention(thin) () -> () { +bb0: + %getX = function_ref @getX : $@convention(thin) () -> @owned X + %x = apply %getX() : $@convention(thin) () -> @owned X + %mx = move_value [lexical] %x : $X + %a = alloc_stack [lexical] $S, let, name "s" + %c = copy_value %mx : $X + %getS = function_ref @getS : $@convention(thin) (@owned X) -> @out S + %s = apply %getS(%a, %c) : $@convention(thin) (@owned X) -> @out S + destroy_addr %a : $*S + dealloc_stack %a : $*S + destroy_value %mx : $X + %r = tuple () + return %r : $() +} + From 807035dbc991fad6dd6a680545da5b19456b6a1c Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Fri, 14 Apr 2023 07:19:53 -0700 Subject: [PATCH 2/2] [MemAccessUtils] Add inst to visitAccessedAddress. The debug_step returns true for mayReadOrWriteMemory so it has to be handled there. rdar://108043268 --- lib/SIL/Utils/MemAccessUtils.cpp | 1 + test/SILOptimizer/deinit_barrier.sil | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/lib/SIL/Utils/MemAccessUtils.cpp b/lib/SIL/Utils/MemAccessUtils.cpp index 367b37ff79531..f3b03feba7d0d 100644 --- a/lib/SIL/Utils/MemAccessUtils.cpp +++ b/lib/SIL/Utils/MemAccessUtils.cpp @@ -2690,6 +2690,7 @@ void swift::visitAccessedAddress(SILInstruction *I, case SILInstructionKind::CopyBlockInst: case SILInstructionKind::CopyBlockWithoutEscapingInst: case SILInstructionKind::CopyValueInst: + case SILInstructionKind::DebugStepInst: case SILInstructionKind::DeinitExistentialAddrInst: case SILInstructionKind::DeinitExistentialValueInst: case SILInstructionKind::DestroyAddrInst: diff --git a/test/SILOptimizer/deinit_barrier.sil b/test/SILOptimizer/deinit_barrier.sil index 574a472142b6d..05f432ad9da83 100644 --- a/test/SILOptimizer/deinit_barrier.sil +++ b/test/SILOptimizer/deinit_barrier.sil @@ -103,3 +103,15 @@ sil [ossa] @test_hop_to_executor : $@convention(thin) () -> () { %retval = tuple () return %retval : $() } + +// CHECK-LABEL: begin running test 1 of 1 on test_instructions_1: is-deinit-barrier +// CHECK: debug_step +// CHECK: false +// CHECK-LABEL: end running test 1 of 1 on test_instructions_1: is-deinit-barrier +sil [ossa] @test_instructions_1 : $@convention(thin) () -> () { +entry: + test_specification "is-deinit-barrier @instruction" + debug_step + %retval = tuple () + return %retval : $() +}