From 8c197ebd8ec4595fa41d09df2d7adcb37a0487e3 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Tue, 30 Sep 2025 13:03:26 +0100 Subject: [PATCH 1/3] [cxx-interop] Delay lowering unowned convention until ownership elimination Unowned result conventions do not work well with OSSA. Retain the result right after the call when we come out of OSSA so we can treat the returned value as if it was owned when we do optimizations. This fix a miscompilation due to the DestroyAddrHoisting pass hoisting destroys above copies with unowned sources. When the destroyed object was the last reference to the pointed memory the copy is happening too late resulting in a use after free. rdar://160462854 --- lib/SIL/IR/SILType.cpp | 5 ++++ lib/SILGen/SILGenApply.cpp | 7 +++-- .../Mandatory/OwnershipModelEliminator.cpp | 12 +++++++++ .../foreign-reference/Inputs/logging-frts.h | 25 +++++++++++++++++- .../get-frt-from-smart-ptr.swift | 26 +++++++++++++++++++ 5 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 test/Interop/Cxx/foreign-reference/get-frt-from-smart-ptr.swift diff --git a/lib/SIL/IR/SILType.cpp b/lib/SIL/IR/SILType.cpp index 700d04bcd6e84..9062c96d8fb0e 100644 --- a/lib/SIL/IR/SILType.cpp +++ b/lib/SIL/IR/SILType.cpp @@ -655,6 +655,11 @@ SILResultInfo::getOwnershipKind(SILFunction &F, case ResultConvention::Owned: return OwnershipKind::Owned; case ResultConvention::Unowned: + // We insert a retain right after the call returning an unowned value in + // OwnershipModelEliminator. So treat the result as owned. + if (IsTrivial) + return OwnershipKind::None; + return OwnershipKind::Owned; case ResultConvention::UnownedInnerPointer: if (IsTrivial) return OwnershipKind::None; diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 4b6896abb7826..1eda4abd7466d 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -6188,13 +6188,12 @@ RValue SILGenFunction::emitApply( B.getDefaultAtomicity()); hasAlreadyLifetimeExtendedSelf = true; } - LLVM_FALLTHROUGH; - - case ResultConvention::Unowned: - // Unretained. Retain the value. result = resultTL.emitCopyValue(B, loc, result); break; + case ResultConvention::Unowned: + // Handled in OwnershipModelEliminator. + break; case ResultConvention::GuaranteedAddress: case ResultConvention::Guaranteed: llvm_unreachable("borrow accessor is not yet implemented"); diff --git a/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp b/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp index 66184f6c8b21c..f7d3e79047e26 100644 --- a/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp +++ b/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp @@ -22,6 +22,8 @@ /// //===----------------------------------------------------------------------===// +#include "swift/AST/Types.h" +#include "swift/SIL/SILValue.h" #define DEBUG_TYPE "sil-ownership-model-eliminator" #include "swift/Basic/Assertions.h" @@ -404,6 +406,16 @@ bool OwnershipModelEliminatorVisitor::visitApplyInst(ApplyInst *ai) { changed = true; } + // Insert a retain for unowned results. + SILBuilderWithScope builder(ai->getNextInstruction(), builderCtx); + builder.emitDestructureValueOperation( + ai->getLoc(), ai, [&](unsigned idx, SILValue v) { + auto resultsIt = fnConv.getDirectSILResults().begin(); + if (resultsIt->getConvention() == ResultConvention::Unowned) + builder.emitCopyValueOperation(ai->getLoc(), v); + ++resultsIt; + }); + return changed; } diff --git a/test/Interop/Cxx/foreign-reference/Inputs/logging-frts.h b/test/Interop/Cxx/foreign-reference/Inputs/logging-frts.h index dcee735b5ad94..08ff830d54ed7 100644 --- a/test/Interop/Cxx/foreign-reference/Inputs/logging-frts.h +++ b/test/Interop/Cxx/foreign-reference/Inputs/logging-frts.h @@ -7,12 +7,14 @@ class SharedFRT { public: SharedFRT() : _refCount(1) { logMsg("Ctor"); } +protected: + virtual ~SharedFRT() { logMsg("Dtor"); } + private: void logMsg(const char *s) const { printf("RefCount: %d, message: %s\n", _refCount, s); } - ~SharedFRT() { logMsg("Dtor"); } SharedFRT(const SharedFRT &) = delete; SharedFRT &operator=(const SharedFRT &) = delete; SharedFRT(SharedFRT &&) = delete; @@ -62,3 +64,24 @@ inline LargeStructWithRefCountedField getStruct() { inline LargeStructWithRefCountedFieldNested getNestedStruct() { return {0, {0, 0, 0, 0, new SharedFRT()}}; } + +template +struct Ref { + T *_Nonnull ptr() const { return ref; } + T *ref; +}; + +class Payload final : public SharedFRT { +public: + static Ref create(int value) { + Ref ref; + ref.ref = new Payload(value); + return ref; + } + + int value() const { return m_value; } + +private: + explicit Payload(int value) : m_value(value) {} + int m_value; +}; diff --git a/test/Interop/Cxx/foreign-reference/get-frt-from-smart-ptr.swift b/test/Interop/Cxx/foreign-reference/get-frt-from-smart-ptr.swift new file mode 100644 index 0000000000000..3020e8fd55d91 --- /dev/null +++ b/test/Interop/Cxx/foreign-reference/get-frt-from-smart-ptr.swift @@ -0,0 +1,26 @@ +// RUN: %target-run-simple-swift(-I %swift_src_root/lib/ClangImporter/SwiftBridging -I %S/Inputs -cxx-interoperability-mode=default -Xfrontend -disable-availability-checking -O) | %FileCheck %s + +// REQUIRES: executable_test + +import LoggingFrts + +@inline(never) +func use(_ x: CInt) { + print("Value is \(x).") +} + +func testRefIssues() { + var a2 = Optional.some(Payload.create(0)); + let b2 = a2!.ptr(); + a2 = Optional.none; + let v2 = b2.value(); + use(v2) +} +testRefIssues() + +// CHECK: RefCount: 1, message: Ctor +// CHECK-NEXT: RefCount: 2, message: retain +// CHECK-NEXT: RefCount: 1, message: release +// CHECK-NEXT: Value is 0. +// CHECK-NEXT: RefCount: 0, message: release +// CHECK-NEXT: RefCount: 0, message: Dtor From 5a638082351dbcfbac695864f45ea1c5b969171b Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Thu, 2 Oct 2025 16:13:20 +0100 Subject: [PATCH 2/3] Change the convention for reabstractions as well. Remove SIL tests that are testing result conventions that are never emitted by the compiler. --- lib/SILGen/SILGenPoly.cpp | 3 +- test/SIL/OwnershipVerifier/use_verifier.sil | 38 ------------- test/SILOptimizer/cse_objc_ossa.sil | 13 ----- test/SILOptimizer/cse_ossa.sil | 13 ----- test/SILOptimizer/cse_ossa_nontrivial.sil | 13 ----- test/SILOptimizer/ossa_rauw_tests.sil | 57 ------------------- test/SILOptimizer/sil_combine_ossa.sil | 33 ----------- .../simplify_unconditional_check_cast.sil | 10 ---- .../simplify_value_to_bridge_object.sil | 17 ------ 9 files changed, 1 insertion(+), 196 deletions(-) diff --git a/lib/SILGen/SILGenPoly.cpp b/lib/SILGen/SILGenPoly.cpp index 9050585a39769..b2cfdcf866747 100644 --- a/lib/SILGen/SILGenPoly.cpp +++ b/lib/SILGen/SILGenPoly.cpp @@ -5290,6 +5290,7 @@ void ResultPlanner::execute(SmallVectorImpl &innerDirectResultStack, LLVM_FALLTHROUGH; case ResultConvention::Owned: case ResultConvention::Autoreleased: + case ResultConvention::Unowned: // Handled in OwnershipModelEliminator. return SGF.emitManagedRValueWithCleanup(resultValue, resultTL); case ResultConvention::Pack: llvm_unreachable("shouldn't have direct result with pack results"); @@ -5299,8 +5300,6 @@ void ResultPlanner::execute(SmallVectorImpl &innerDirectResultStack, // originally 'self'. SGF.SGM.diagnose(Loc.getSourceLoc(), diag::not_implemented, "reabstraction of returns_inner_pointer function"); - LLVM_FALLTHROUGH; - case ResultConvention::Unowned: return SGF.emitManagedCopy(Loc, resultValue, resultTL); case ResultConvention::GuaranteedAddress: case ResultConvention::Guaranteed: diff --git a/test/SIL/OwnershipVerifier/use_verifier.sil b/test/SIL/OwnershipVerifier/use_verifier.sil index aa6744e38a5a3..66c0a1725e7ce 100644 --- a/test/SIL/OwnershipVerifier/use_verifier.sil +++ b/test/SIL/OwnershipVerifier/use_verifier.sil @@ -828,30 +828,6 @@ bb5(%5 : @owned $ThreeDifferingPayloadEnum): return %5 : $ThreeDifferingPayloadEnum } -sil [ossa] @enum_cases_with_trivial_unowned_cases_arg_into_phi : $@convention(thin) (Builtin.NativeObject) -> ThreeDifferingPayloadEnum { -bb0(%0 : @unowned $Builtin.NativeObject): - cond_br undef, bb1, bb2 - -bb1: - cond_br undef, bb3, bb4 - -bb2: - %1 = enum $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.nopayload!enumelt - br bb5(%1 : $ThreeDifferingPayloadEnum) - -bb3: - %2 = enum $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.nontrivial_payload!enumelt, %0 : $Builtin.NativeObject - br bb5(%2 : $ThreeDifferingPayloadEnum) - -bb4: - %3 = integer_literal $Builtin.Int32, 0 - %4 = enum $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.trivial_payload!enumelt, %3 : $Builtin.Int32 - br bb5(%4 : $ThreeDifferingPayloadEnum) - -bb5(%5 : @unowned $ThreeDifferingPayloadEnum): - return %5 : $ThreeDifferingPayloadEnum -} - sil [ossa] @enum_cases_with_trivial_guaranteed_cases_arg_into_phi : $@convention(thin) (@guaranteed Builtin.NativeObject) -> @owned ThreeDifferingPayloadEnum { bb0(%0 : @guaranteed $Builtin.NativeObject): cond_br undef, bb1, bb2 @@ -1383,20 +1359,6 @@ bb3(%fUnknown : @owned $@callee_owned () -> ()): return %9999 : $() } -sil [ossa] @unowned_to_ref_is_unowned_instant_use : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Builtin.NativeObject { -bb0(%0 : @guaranteed $Builtin.NativeObject): - %1 = ref_to_unowned %0 : $Builtin.NativeObject to $@sil_unowned Builtin.NativeObject - %2 = unowned_to_ref %1 : $@sil_unowned Builtin.NativeObject to $Builtin.NativeObject - return %2 : $Builtin.NativeObject -} - -sil [ossa] @unmanaged_to_ref_is_unowned_instant_use : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Builtin.NativeObject { -bb0(%0 : @guaranteed $Builtin.NativeObject): - %1 = ref_to_unmanaged %0 : $Builtin.NativeObject to $@sil_unmanaged Builtin.NativeObject - %2 = unmanaged_to_ref %1 : $@sil_unmanaged Builtin.NativeObject to $Builtin.NativeObject - return %2 : $Builtin.NativeObject -} - sil [ossa] @nontrivial_enum_unchecked_enum_data_trivial_payload_owned : $@convention(thin) (@owned ThreeDifferingPayloadEnum) -> Builtin.Int32 { bb0(%0 : @owned $ThreeDifferingPayloadEnum): // NOTE: It may be surprising that %0 is consumed by this unchecked_enum_data diff --git a/test/SILOptimizer/cse_objc_ossa.sil b/test/SILOptimizer/cse_objc_ossa.sil index 8be52c7a1a9d4..e96733995e93c 100644 --- a/test/SILOptimizer/cse_objc_ossa.sil +++ b/test/SILOptimizer/cse_objc_ossa.sil @@ -9,19 +9,6 @@ import Foundation @objc(XX) protocol XX { } -// CHECK-LABEL: sil [ossa] @cse_objc_protocol : -// CHECK: objc_protocol #XX : $Protocol -// CHECK-NOT: objc_protocol -// CHECK: tuple (%0 : $Protocol, %0 : $Protocol) -// CHECK-LABEL: } // end sil function 'cse_objc_protocol' -sil [ossa] @cse_objc_protocol : $@convention(thin) () -> (Protocol, Protocol) { -bb0: - %0 = objc_protocol #XX : $Protocol - %1 = objc_protocol #XX : $Protocol - %2 = tuple (%0: $Protocol, %1: $Protocol) - return %2 : $(Protocol, Protocol) -} - @objc protocol Walkable { func walk() } diff --git a/test/SILOptimizer/cse_ossa.sil b/test/SILOptimizer/cse_ossa.sil index 3dc4fb034747d..5b923c6ec84a6 100644 --- a/test/SILOptimizer/cse_ossa.sil +++ b/test/SILOptimizer/cse_ossa.sil @@ -695,19 +695,6 @@ bb0(%0 : $*Builtin.Int8): return %4 : $Builtin.Int64 } -// CHECK-LABEL: sil [ossa] @cse_raw_pointer_to_ref : -// CHECK: raw_pointer_to_ref -// CHECK-NOT: raw_pointer_to_ref -// CHECK: tuple -// CHECK-LABEL: } // end sil function 'cse_raw_pointer_to_ref' -sil [ossa] @cse_raw_pointer_to_ref : $@convention(thin) (Builtin.RawPointer) -> (C, C) { -bb0(%0 : $Builtin.RawPointer): - %1 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $C - %2 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $C - %6 = tuple(%1: $C, %2: $C) - return %6 : $(C, C) -} - // CHECK-LABEL: sil [ossa] @cse_unchecked_addr_cast : // CHECK: unchecked_addr_cast // CHECK-NOT: unchecked_addr_cast diff --git a/test/SILOptimizer/cse_ossa_nontrivial.sil b/test/SILOptimizer/cse_ossa_nontrivial.sil index 061b0d58cdf00..bb119dcb00f0b 100644 --- a/test/SILOptimizer/cse_ossa_nontrivial.sil +++ b/test/SILOptimizer/cse_ossa_nontrivial.sil @@ -255,19 +255,6 @@ bb0(%0 : @owned $Klass): return %6 : $() } -// CHECK-LABEL: sil [ossa] @cse_raw_pointer_to_ref : -// CHECK: raw_pointer_to_ref -// CHECK-NOT: raw_pointer_to_ref -// CHECK: tuple -// CHECK-LABEL: } // end sil function 'cse_raw_pointer_to_ref' -sil [ossa] @cse_raw_pointer_to_ref : $@convention(thin) (Builtin.RawPointer) -> (Klass, Klass) { -bb0(%0 : $Builtin.RawPointer): - %1 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $Klass - %2 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $Klass - %6 = tuple(%1: $Klass, %2: $Klass) - return %6 : $(Klass, Klass) -} - enum Enum1 { case Case1 case Case2 diff --git a/test/SILOptimizer/ossa_rauw_tests.sil b/test/SILOptimizer/ossa_rauw_tests.sil index 0a24c7af567e3..0e818f5373dca 100644 --- a/test/SILOptimizer/ossa_rauw_tests.sil +++ b/test/SILOptimizer/ossa_rauw_tests.sil @@ -302,52 +302,6 @@ bbExitBlock(%result : @owned $FakeOptional): return %result : $FakeOptional } -// CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2 : $@convention(thin) (@guaranteed Klass) -> (Klass, Klass) { -// CHECK: bb0( -// CHECK-NEXT: tuple -// CHECK-NEXT: return -// CHECK: } // end sil function 'unowned_to_guaranteed_rauw_2' -sil [ossa] @unowned_to_guaranteed_rauw_2 : $@convention(thin) (@guaranteed Klass) -> (Klass, Klass) { -bb0(%0 : @guaranteed $Klass): - %1 = unchecked_bitwise_cast %0 : $Klass to $SubKlass - %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass - %3 = tuple(%2 : $Klass, %2 : $Klass) - return %3 : $(Klass, Klass) -} - -// CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2a : $@convention(thin) (@guaranteed Builtin.NativeObject) -> (Klass, Klass) { -// CHECK: bb0( -// CHECK-NEXT: unchecked_ref_cast -// CHECK-NEXT: tuple -// CHECK-NEXT: return -// CHECK: } // end sil function 'unowned_to_guaranteed_rauw_2a' -sil [ossa] @unowned_to_guaranteed_rauw_2a : $@convention(thin) (@guaranteed Builtin.NativeObject) -> (Klass, Klass) { -bb0(%0 : @guaranteed $Builtin.NativeObject): - %0a = unchecked_ref_cast %0 : $Builtin.NativeObject to $Klass - %1 = unchecked_bitwise_cast %0a : $Klass to $SubKlass - %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass - %3 = tuple(%2 : $Klass, %2 : $Klass) - return %3 : $(Klass, Klass) -} - -// We need the unchecked_ownership_conversion since our base value is -// guaranteed, not a function argument, and our user is a function exiting -// terminator. -// -// CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2b : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Klass { -// CHECK: bb0( -// CHECK-NEXT: unchecked_ref_cast -// CHECK-NEXT: return -// CHECK: } // end sil function 'unowned_to_guaranteed_rauw_2b' -sil [ossa] @unowned_to_guaranteed_rauw_2b : $@convention(thin) (@guaranteed Builtin.NativeObject) -> Klass { -bb0(%0 : @guaranteed $Builtin.NativeObject): - %0a = unchecked_ref_cast %0 : $Builtin.NativeObject to $Klass - %1 = unchecked_bitwise_cast %0a : $Klass to $SubKlass - %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass - return %2 : $Klass -} - - // CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2_loop : $@convention(thin) (@guaranteed Klass) -> @owned FakeOptional<(Klass, Klass)> { // CHECK: bb0([[ARG:%.*]] : @guaranteed $Klass): // CHECK-NOT: unchecked_bitwise_cast @@ -404,17 +358,6 @@ bbExitBlock(%result : @owned $FakeOptional<(Klass, Klass)>): return %result : $FakeOptional<(Klass, Klass)> } -// CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_3 : $@convention(thin) (@guaranteed Klass) -> Klass { -// CHECK: bb0( -// CHECK-NEXT: return -// CHECK: } // end sil function 'unowned_to_guaranteed_rauw_3' -sil [ossa] @unowned_to_guaranteed_rauw_3 : $@convention(thin) (@guaranteed Klass) -> Klass { -bb0(%0 : @guaranteed $Klass): - %1 = unchecked_bitwise_cast %0 : $Klass to $SubKlass - %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass - return %2 : $Klass -} - //===--- // Guaranteed Tests // diff --git a/test/SILOptimizer/sil_combine_ossa.sil b/test/SILOptimizer/sil_combine_ossa.sil index 58005376fce19..4ff822fef292b 100644 --- a/test/SILOptimizer/sil_combine_ossa.sil +++ b/test/SILOptimizer/sil_combine_ossa.sil @@ -1604,18 +1604,6 @@ bb0(%0 : @guaranteed $B): return %3 : $F } -// CHECK-LABEL: sil [ossa] @unchecked_ref_cast_formation_unowned : $@convention(thin) (B) -> F { -// CHECK: bb0([[INPUT_REF:%[0-9]+]] : -// CHECK: ref_to_raw_pointer -// CHECK: raw_pointer_to_ref -// CHECK: } // end sil function 'unchecked_ref_cast_formation_unowned' -sil [ossa] @unchecked_ref_cast_formation_unowned : $@convention(thin) (B) -> F { -bb0(%0 : @unowned $B): - %1 = ref_to_raw_pointer %0 : $B to $Builtin.RawPointer - %2 = raw_pointer_to_ref %1 : $Builtin.RawPointer to $F - return %2 : $F -} - // CHECK-LABEL: sil [ossa] @upcast_unchecked_ref_cast_roundtrip : $@convention(thin) (@owned B) -> @owned B { // CHECK-NOT: unchecked_ref_cast // CHECK-NOT: upcast @@ -2745,27 +2733,6 @@ class XXImpl : XX { init() } -// CHECK-LABEL: sil [ossa] @unowned_round_trips : $@convention(thin) (@guaranteed B, @guaranteed @sil_unowned B, @guaranteed AnyObject, @sil_unmanaged AnyObject) -> (B, @sil_unowned B, AnyObject, @sil_unmanaged AnyObject) { -// CHECK: bb0( -// CHECK-NEXT: tuple -// CHECK-NEXT: return -// CHECK: } // end sil function 'unowned_round_trips' -sil [ossa] @unowned_round_trips : $@convention(thin) (@guaranteed B, @guaranteed @sil_unowned B, @guaranteed AnyObject, @sil_unmanaged AnyObject) -> (B, @sil_unowned B, AnyObject, @sil_unmanaged AnyObject) { -bb0(%0 : @guaranteed $B, %1 : @guaranteed $@sil_unowned B, %2 : @guaranteed $AnyObject, %3 : $@sil_unmanaged AnyObject): - %4 = ref_to_unowned %0 : $B to $@sil_unowned B - %5 = unowned_to_ref %4 : $@sil_unowned B to $B - %6 = unowned_to_ref %1 : $@sil_unowned B to $B - %7 = ref_to_unowned %6 : $B to $@sil_unowned B - - %8 = ref_to_unmanaged %2 : $AnyObject to $@sil_unmanaged AnyObject - %9 = unmanaged_to_ref %8 : $@sil_unmanaged AnyObject to $AnyObject - %10 = unmanaged_to_ref %3 : $@sil_unmanaged AnyObject to $AnyObject - %11 = ref_to_unmanaged %10 : $AnyObject to $@sil_unmanaged AnyObject - - %9999 = tuple(%5 : $B, %7 : $@sil_unowned B, %9 : $AnyObject, %11 : $@sil_unmanaged AnyObject) - return %9999 : $(B, @sil_unowned B, AnyObject, @sil_unmanaged AnyObject) -} - // CHECK-LABEL: sil [ossa] @collapse_existential_pack_unpack_unchecked_ref_cast : // CHECK: bb0([[Ref:%.*]] : @guaranteed $MyClass): // CHECK-NOT: init_existential_ref diff --git a/test/SILOptimizer/simplify_unconditional_check_cast.sil b/test/SILOptimizer/simplify_unconditional_check_cast.sil index 1b667b2e60da5..e31a7f3b4e660 100644 --- a/test/SILOptimizer/simplify_unconditional_check_cast.sil +++ b/test/SILOptimizer/simplify_unconditional_check_cast.sil @@ -38,16 +38,6 @@ bb0(%0 : $@thick T.Type): } -// CHECK-LABEL: sil [ossa] @test_non_metatype : -// CHECK: %1 = unconditional_checked_cast %0 -// CHECK-NEXT: return %1 -// CHECK: } // end sil function 'test_non_metatype' -sil [ossa] @test_non_metatype : $@convention(thin) (@guaranteed C) -> any PC { -bb0(%0 : @guaranteed $C): - %1 = unconditional_checked_cast %0 to any PC - return %1 -} - // CHECK-LABEL: sil [ossa] @test_non_existential_target : // CHECK: %1 = unconditional_checked_cast %0 // CHECK-NEXT: return %1 diff --git a/test/SILOptimizer/simplify_value_to_bridge_object.sil b/test/SILOptimizer/simplify_value_to_bridge_object.sil index e83df68070631..ca80e88330707 100644 --- a/test/SILOptimizer/simplify_value_to_bridge_object.sil +++ b/test/SILOptimizer/simplify_value_to_bridge_object.sil @@ -17,20 +17,3 @@ bb0(%0 : $Builtin.Int64): %4 = value_to_bridge_object %3 : $Builtin.Int64 return %4 : $Builtin.BridgeObject } - -// CHECK-LABEL: sil [ossa] @keep_both_vtbo_instructions -// CHECK: %1 = value_to_bridge_object %0 -// CHECK: %2 = value_to_bridge_object %0 -// CHECK: %3 = tuple (%1 : $Builtin.BridgeObject, %2 : $Builtin.BridgeObject) -// CHECK: return %3 -// CHECK: } // end sil function 'keep_both_vtbo_instructions' -sil [ossa] @keep_both_vtbo_instructions : $@convention(thin) (Builtin.Int64) -> (Builtin.BridgeObject, Builtin.BridgeObject) { -bb0(%0 : $Builtin.Int64): - %1 = value_to_bridge_object %0 : $Builtin.Int64 - %2 = unchecked_trivial_bit_cast %1 : $Builtin.BridgeObject to $UInt64 - %3 = struct_extract %2 : $UInt64, #UInt64._value - %4 = value_to_bridge_object %3 : $Builtin.Int64 - %5 = tuple (%1 : $Builtin.BridgeObject, %4 : $Builtin.BridgeObject) - return %5 : $(Builtin.BridgeObject, Builtin.BridgeObject) -} - From 8b3b16cb5b0d6487d05047e3c24c7cafd00c73aa Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Mon, 6 Oct 2025 15:25:23 +0100 Subject: [PATCH 3/3] Fix some fallout. --- .../Mandatory/OwnershipModelEliminator.cpp | 18 ++++-- test/SILOptimizer/cse_objc_ossa.sil | 14 ++++ test/SILOptimizer/cse_ossa.sil | 14 ++++ test/SILOptimizer/ossa_rauw_tests.sil | 64 +++++++++++++++++++ .../simplify_unconditional_check_cast.sil | 11 ++++ .../simplify_value_to_bridge_object.sil | 18 ++++++ 6 files changed, 132 insertions(+), 7 deletions(-) diff --git a/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp b/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp index f7d3e79047e26..1339507d3fe6c 100644 --- a/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp +++ b/lib/SILOptimizer/Mandatory/OwnershipModelEliminator.cpp @@ -408,13 +408,17 @@ bool OwnershipModelEliminatorVisitor::visitApplyInst(ApplyInst *ai) { // Insert a retain for unowned results. SILBuilderWithScope builder(ai->getNextInstruction(), builderCtx); - builder.emitDestructureValueOperation( - ai->getLoc(), ai, [&](unsigned idx, SILValue v) { - auto resultsIt = fnConv.getDirectSILResults().begin(); - if (resultsIt->getConvention() == ResultConvention::Unowned) - builder.emitCopyValueOperation(ai->getLoc(), v); - ++resultsIt; - }); + auto resultIt = fnConv.getDirectSILResults().begin(); + auto copyValue = [&](unsigned idx, SILValue v) { + auto result = *resultIt; + if (result.getConvention() == ResultConvention::Unowned) + builder.emitCopyValueOperation(ai->getLoc(), v); + ++resultIt; + }; + if (fnConv.getNumDirectSILResults() == 1) + copyValue(0, ai); + else + builder.emitDestructureValueOperation(ai->getLoc(), ai, copyValue); return changed; } diff --git a/test/SILOptimizer/cse_objc_ossa.sil b/test/SILOptimizer/cse_objc_ossa.sil index e96733995e93c..dd02d72cb6b2e 100644 --- a/test/SILOptimizer/cse_objc_ossa.sil +++ b/test/SILOptimizer/cse_objc_ossa.sil @@ -9,6 +9,20 @@ import Foundation @objc(XX) protocol XX { } +// CHECK-LABEL: sil [ossa] @cse_objc_protocol : +// CHECK: objc_protocol #XX : $Protocol +// CHECK-NOT: objc_protocol +// CHECK: tuple (%0 : $Protocol, %0 : $Protocol) +// CHECK-LABEL: } // end sil function 'cse_objc_protocol' +sil [ossa] @cse_objc_protocol : $@convention(thin) () -> @owned (Protocol, Protocol) { +bb0: + %0 = objc_protocol #XX : $Protocol + %1 = objc_protocol #XX : $Protocol + %2 = tuple (%0: $Protocol, %1: $Protocol) + %3 = copy_value %2 + return %3 +} + @objc protocol Walkable { func walk() } diff --git a/test/SILOptimizer/cse_ossa.sil b/test/SILOptimizer/cse_ossa.sil index 5b923c6ec84a6..eb36b26620d22 100644 --- a/test/SILOptimizer/cse_ossa.sil +++ b/test/SILOptimizer/cse_ossa.sil @@ -695,6 +695,20 @@ bb0(%0 : $*Builtin.Int8): return %4 : $Builtin.Int64 } +// CHECK-LABEL: sil [ossa] @cse_raw_pointer_to_ref : +// CHECK: raw_pointer_to_ref +// CHECK-NOT: raw_pointer_to_ref +// CHECK: tuple +// CHECK-LABEL: } // end sil function 'cse_raw_pointer_to_ref' +sil [ossa] @cse_raw_pointer_to_ref : $@convention(thin) (Builtin.RawPointer) -> @owned (C, C) { +bb0(%0 : $Builtin.RawPointer): + %1 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $C + %2 = raw_pointer_to_ref %0 : $Builtin.RawPointer to $C + %6 = tuple(%1: $C, %2: $C) + %7 = copy_value %6 + return %7 +} + // CHECK-LABEL: sil [ossa] @cse_unchecked_addr_cast : // CHECK: unchecked_addr_cast // CHECK-NOT: unchecked_addr_cast diff --git a/test/SILOptimizer/ossa_rauw_tests.sil b/test/SILOptimizer/ossa_rauw_tests.sil index 0e818f5373dca..d6bf4638b8660 100644 --- a/test/SILOptimizer/ossa_rauw_tests.sil +++ b/test/SILOptimizer/ossa_rauw_tests.sil @@ -302,6 +302,57 @@ bbExitBlock(%result : @owned $FakeOptional): return %result : $FakeOptional } +// CHECK-LABEL: sil [ossa] @owned_to_guaranteed_rauw_2 : $@convention(thin) (@guaranteed Klass) -> @owned (Klass, Klass) { +// CHECK: bb0( +// CHECK-NEXT: tuple +// CHECK-NEXT: copy_value +// CHECK-NEXT: return +// CHECK: } // end sil function 'owned_to_guaranteed_rauw_2' +sil [ossa] @owned_to_guaranteed_rauw_2 : $@convention(thin) (@guaranteed Klass) -> @owned (Klass, Klass) { +bb0(%0 : @guaranteed $Klass): + %1 = unchecked_bitwise_cast %0 : $Klass to $SubKlass + %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass + %3 = tuple(%2 : $Klass, %2 : $Klass) + %4 = copy_value %3 + return %4 : $(Klass, Klass) +} + +// CHECK-LABEL: sil [ossa] @owned_to_guaranteed_rauw_2a : $@convention(thin) (@guaranteed Builtin.NativeObject) -> @owned (Klass, Klass) { +// CHECK: bb0( +// CHECK-NEXT: unchecked_ref_cast +// CHECK-NEXT: tuple +// CHECK-NEXT: copy_value +// CHECK-NEXT: return +// CHECK: } // end sil function 'owned_to_guaranteed_rauw_2a' +sil [ossa] @owned_to_guaranteed_rauw_2a : $@convention(thin) (@guaranteed Builtin.NativeObject) -> @owned (Klass, Klass) { +bb0(%0 : @guaranteed $Builtin.NativeObject): + %0a = unchecked_ref_cast %0 : $Builtin.NativeObject to $Klass + %1 = unchecked_bitwise_cast %0a : $Klass to $SubKlass + %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass + %3 = tuple(%2 : $Klass, %2 : $Klass) + %4 = copy_value %3 + return %4 : $(Klass, Klass) +} + +// We need the unchecked_ownership_conversion since our base value is +// guaranteed, not a function argument, and our user is a function exiting +// terminator. +// +// CHECK-LABEL: sil [ossa] @owned_to_guaranteed_rauw_2b : $@convention(thin) (@guaranteed Builtin.NativeObject) -> @owned Klass { +// CHECK: bb0( +// CHECK-NEXT: unchecked_ref_cast +// CHECK-NEXT: copy_value +// CHECK-NEXT: return +// CHECK: } // end sil function 'owned_to_guaranteed_rauw_2b' +sil [ossa] @owned_to_guaranteed_rauw_2b : $@convention(thin) (@guaranteed Builtin.NativeObject) -> @owned Klass { +bb0(%0 : @guaranteed $Builtin.NativeObject): + %0a = unchecked_ref_cast %0 : $Builtin.NativeObject to $Klass + %1 = unchecked_bitwise_cast %0a : $Klass to $SubKlass + %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass + %3 = copy_value %2 + return %3 : $Klass +} + // CHECK-LABEL: sil [ossa] @unowned_to_guaranteed_rauw_2_loop : $@convention(thin) (@guaranteed Klass) -> @owned FakeOptional<(Klass, Klass)> { // CHECK: bb0([[ARG:%.*]] : @guaranteed $Klass): // CHECK-NOT: unchecked_bitwise_cast @@ -358,6 +409,19 @@ bbExitBlock(%result : @owned $FakeOptional<(Klass, Klass)>): return %result : $FakeOptional<(Klass, Klass)> } +// CHECK-LABEL: sil [ossa] @owned_to_guaranteed_rauw_3 : $@convention(thin) (@guaranteed Klass) -> @owned Klass { +// CHECK: bb0( +// CHECK-NEXT: copy_value +// CHECK-NEXT: return +// CHECK: } // end sil function 'owned_to_guaranteed_rauw_3' +sil [ossa] @owned_to_guaranteed_rauw_3 : $@convention(thin) (@guaranteed Klass) -> @owned Klass { +bb0(%0 : @guaranteed $Klass): + %1 = unchecked_bitwise_cast %0 : $Klass to $SubKlass + %2 = unchecked_bitwise_cast %1 : $SubKlass to $Klass + %3 = copy_value %2 + return %3 : $Klass +} + //===--- // Guaranteed Tests // diff --git a/test/SILOptimizer/simplify_unconditional_check_cast.sil b/test/SILOptimizer/simplify_unconditional_check_cast.sil index e31a7f3b4e660..eaa17e98451df 100644 --- a/test/SILOptimizer/simplify_unconditional_check_cast.sil +++ b/test/SILOptimizer/simplify_unconditional_check_cast.sil @@ -37,6 +37,17 @@ bb0(%0 : $@thick T.Type): return %1 } +// CHECK-LABEL: sil [ossa] @test_non_metatype : +// CHECK: %1 = unconditional_checked_cast %0 +// CHECK-NEXT: %2 = copy_value %1 +// CHECK-NEXT: return %2 +// CHECK: } // end sil function 'test_non_metatype' +sil [ossa] @test_non_metatype : $@convention(thin) (@guaranteed C) -> @owned any PC { +bb0(%0 : @guaranteed $C): + %1 = unconditional_checked_cast %0 to any PC + %2 = copy_value %1 + return %2 +} // CHECK-LABEL: sil [ossa] @test_non_existential_target : // CHECK: %1 = unconditional_checked_cast %0 diff --git a/test/SILOptimizer/simplify_value_to_bridge_object.sil b/test/SILOptimizer/simplify_value_to_bridge_object.sil index ca80e88330707..848ed70e82a0a 100644 --- a/test/SILOptimizer/simplify_value_to_bridge_object.sil +++ b/test/SILOptimizer/simplify_value_to_bridge_object.sil @@ -17,3 +17,21 @@ bb0(%0 : $Builtin.Int64): %4 = value_to_bridge_object %3 : $Builtin.Int64 return %4 : $Builtin.BridgeObject } + +// CHECK-LABEL: sil [ossa] @keep_both_vtbo_instructions +// CHECK: %1 = value_to_bridge_object %0 +// CHECK: %2 = value_to_bridge_object %0 +// CHECK: %3 = tuple (%1 : $Builtin.BridgeObject, %2 : $Builtin.BridgeObject) +// CHECK: %4 = copy_value %3 +// CHECK: return %4 +// CHECK: } // end sil function 'keep_both_vtbo_instructions' +sil [ossa] @keep_both_vtbo_instructions : $@convention(thin) (Builtin.Int64) -> @owned (Builtin.BridgeObject, Builtin.BridgeObject) { +bb0(%0 : $Builtin.Int64): + %1 = value_to_bridge_object %0 : $Builtin.Int64 + %2 = unchecked_trivial_bit_cast %1 : $Builtin.BridgeObject to $UInt64 + %3 = struct_extract %2 : $UInt64, #UInt64._value + %4 = value_to_bridge_object %3 : $Builtin.Int64 + %5 = tuple (%1 : $Builtin.BridgeObject, %4 : $Builtin.BridgeObject) + %6 = copy_value %5 + return %6 +}