From 35f82ba402168eb22bf44dc43accc3045cce86b4 Mon Sep 17 00:00:00 2001 From: WeZZard Date: Wed, 5 Mar 2025 22:15:19 +0800 Subject: [PATCH 1/4] [SILOptimizer] Fixes the ValueDefUseWalker to handle potential unchecked_ref_cast between optional and non-optional class types. The `unchecked_ref_cast` is designed to be able to cast between `Optional` and `ClassType`. We need to handle these cases by checking if the type is optional and adjust the path accordingly. --- SwiftCompilerSources/Sources/SIL/Type.swift | 1 + .../Sources/SIL/Utilities/WalkUtils.swift | 47 ++++++++++++++++++- include/swift/AST/Types.h | 7 +++ include/swift/SIL/SILBridging.h | 1 + include/swift/SIL/SILBridgingImpl.h | 4 ++ include/swift/SIL/SILType.h | 5 ++ .../redundant_load_elim_with_casts.sil | 42 +++++++++++++++++ 7 files changed, 105 insertions(+), 2 deletions(-) diff --git a/SwiftCompilerSources/Sources/SIL/Type.swift b/SwiftCompilerSources/Sources/SIL/Type.swift index 98e358baeba84..82ecbf881fd5f 100644 --- a/SwiftCompilerSources/Sources/SIL/Type.swift +++ b/SwiftCompilerSources/Sources/SIL/Type.swift @@ -63,6 +63,7 @@ public struct Type : CustomStringConvertible, NoReflectionChildren { public var isFunction: Bool { bridged.isFunction() } public var isMetatype: Bool { bridged.isMetatype() } public var isClassExistential: Bool { bridged.isClassExistential() } + public var isOptional: Bool { bridged.isOptional() } public var isNoEscapeFunction: Bool { bridged.isNoEscapeFunction() } public var containsNoEscapeFunction: Bool { bridged.containsNoEscapeFunction() } public var isThickFunction: Bool { bridged.isThickFunction() } diff --git a/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift b/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift index c07251e9d7a48..069a25cc03aae 100644 --- a/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift +++ b/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift @@ -370,7 +370,28 @@ extension ValueDefUseWalker { // We need to ignore this because otherwise the path wouldn't contain the right `existential` field kind. return leafUse(value: operand, path: path) } - return walkDownUses(ofValue: urc, path: path) + // The `unchecked_ref_cast` is designed to be able to cast between + // `Optional` and `ClassType`. We need to handle these + // cases by checking if the type is optional and adjust the path + // accordingly. + switch (urc.type.isOptional, urc.fromInstance.type.isOptional) { + case (true, false): + if walkDownUses(ofValue: urc, path: path.push(.enumCase, index: 0)) == .abortWalk { + return .abortWalk + } + return walkDownUses(ofValue: urc, path: path.push(.enumCase, index: 1)) + case (false, true): + if let path = path.popIfMatches(.enumCase, index: 0) { + if walkDownUses(ofValue: urc, path: path) == .abortWalk { + return .abortWalk + } else if let path = path.popIfMatches(.enumCase, index: 1) { + return walkDownUses(ofValue: urc, path: path) + } + } + return .abortWalk + default: + return walkDownUses(ofValue: urc, path: path) + } case let beginDealloc as BeginDeallocRefInst: if operand.index == 0 { return walkDownUses(ofValue: beginDealloc, path: path) @@ -699,7 +720,29 @@ extension ValueUseDefWalker { // We need to ignore this because otherwise the path wouldn't contain the right `existential` field kind. return rootDef(value: urc, path: path) } - return walkUp(value: urc.fromInstance, path: path) + // The `unchecked_ref_cast` is designed to be able to cast between + // `Optional` and `ClassType`. We need to handle these + // cases by checking if the type is optional and adjust the path + // accordingly. + switch (urc.type.isOptional, urc.fromInstance.type.isOptional) { + case (true, false): + if let path = path.popIfMatches(.enumCase, index: 0) { + if walkUp(value: urc.fromInstance, path: path) == .abortWalk { + return .abortWalk + } else if let path = path.popIfMatches(.enumCase, index: 1) { + return walkUp(value: urc.fromInstance, path: path) + } + } + return .abortWalk + case (false, true): + if walkUp(value: urc.fromInstance, path: path.push(.enumCase, index: 0)) == .abortWalk { + return .abortWalk + } else { + return walkUp(value: urc.fromInstance, path: path.push(.enumCase, index: 1)) + } + default: + return walkUp(value: urc.fromInstance, path: path) + } case let arg as Argument: if let phi = Phi(arg) { for incoming in phi.incomingValues { diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index 2c37b5caf89c8..c42054599ab55 100644 --- a/include/swift/AST/Types.h +++ b/include/swift/AST/Types.h @@ -795,6 +795,9 @@ class alignas(1 << TypeAlignInBits) TypeBase /// Determine whether the type is an opened existential type with Error inside bool isOpenedExistentialWithError(); + /// Determine whether the type is an Optional type. + bool isOptional() const; + /// Retrieve the set of type parameter packs that occur within this type. void getTypeParameterPacks(SmallVectorImpl &rootParameterPacks); @@ -7926,6 +7929,10 @@ inline bool TypeBase::isClassExistentialType() { return false; } +inline bool TypeBase::isOptional() const { + return getCanonicalType()->isOptional(); +} + inline bool TypeBase::canDynamicallyBeOptionalType(bool includeExistential) { CanType T = getCanonicalType(); auto isArchetypeOrExistential = isa(T) || diff --git a/include/swift/SIL/SILBridging.h b/include/swift/SIL/SILBridging.h index 961726cae1697..c3ea986d340ca 100644 --- a/include/swift/SIL/SILBridging.h +++ b/include/swift/SIL/SILBridging.h @@ -263,6 +263,7 @@ struct BridgedType { BRIDGED_INLINE bool isFunction() const; BRIDGED_INLINE bool isMetatype() const; BRIDGED_INLINE bool isClassExistential() const; + BRIDGED_INLINE bool isOptional() const; BRIDGED_INLINE bool isNoEscapeFunction() const; BRIDGED_INLINE bool containsNoEscapeFunction() const; BRIDGED_INLINE bool isThickFunction() const; diff --git a/include/swift/SIL/SILBridgingImpl.h b/include/swift/SIL/SILBridgingImpl.h index 4ebd635c1c0b6..bed44984820ff 100644 --- a/include/swift/SIL/SILBridgingImpl.h +++ b/include/swift/SIL/SILBridgingImpl.h @@ -402,6 +402,10 @@ bool BridgedType::isClassExistential() const { return unbridged().isClassExistentialType(); } +bool BridgedType::isOptional() const { + return unbridged().isOptional(); +} + bool BridgedType::isNoEscapeFunction() const { return unbridged().isNoEscapeFunction(); } diff --git a/include/swift/SIL/SILType.h b/include/swift/SIL/SILType.h index 9b4aa899158c8..564a3741a53eb 100644 --- a/include/swift/SIL/SILType.h +++ b/include/swift/SIL/SILType.h @@ -458,6 +458,11 @@ class SILType { return getASTType()->hasOpenedExistential(); } + /// Returns true if the referenced type is an Optional type. + bool isOptional() const { + return getASTType()->isOptional(); + } + TypeTraitResult canBeClass() const { return getASTType()->canBeClass(); } diff --git a/test/SILOptimizer/redundant_load_elim_with_casts.sil b/test/SILOptimizer/redundant_load_elim_with_casts.sil index 8ebf561417281..bfc88c817f3a3 100644 --- a/test/SILOptimizer/redundant_load_elim_with_casts.sil +++ b/test/SILOptimizer/redundant_load_elim_with_casts.sil @@ -52,6 +52,7 @@ sil @use : $@convention(thin) (Builtin.Int32) -> () sil @use_a : $@convention(thin) (@in A) -> () sil @escaped_a_ptr : $@convention(thin) () -> @out A sil @escaped_a : $@convention(thin) () -> Builtin.RawPointer +sil @b_i_plus_one : $@convention(method) (@guaranteed B) -> () // *NOTE* This does not handle raw pointer since raw pointer is only layout compatible with heap references. // CHECK-FUTURE: sil @store_to_load_forward_unchecked_addr_cast_struct : $@convention(thin) (Optional) -> () { @@ -547,3 +548,44 @@ bb0(%0 : $*(I32, I32, I32), %1 : $*((I32, I32), I32)): %63 = tuple (%60 : $(I32, I32), %61 : $(I32, I32), %62 : $(I32, I32, I32)) return %63 : $((I32, I32), (I32, I32), (I32, I32, I32)) } + +// Tests unchecked_ref_cast between Optional and ClassType2. +// E? -> B is safe +// +// CHECK-FUTURE: sil @unchecked_ref_cast_from_optional_class +// CHECK: bb3(%6 : $Optional): +// CHECK: %8 = load %7 : $*Builtin.Int32 +// CHECK: %10 = apply %9(%5) : $@convention(method) (@guaranteed B) -> () +// CHECK: %12 = load %11 : $*Builtin.Int32 +// CHECK: return +sil @unchecked_ref_cast_from_optional_class : $@convention(thin) (Optional) -> () { +bb0(%0 : $Optional): + switch_enum %0 : $Optional, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2 + +bb1(%1 : $E): + %2 = enum $Optional, #Optional.some!enumelt, %1 : $E + br bb3(%2 : $Optional) + +bb2: + %3 = enum $Optional, #Optional.none!enumelt + br bb3(%3 : $Optional) + +bb3(%4 : $Optional): + %5 = unchecked_ref_cast %4 : $Optional to $B + + %6 = ref_element_addr %5 : $B, #B.i + %7 = begin_access [read] [dynamic] [no_nested_conflict] %6 : $* Builtin.Int32 + %8 = load %7 : $*Builtin.Int32 + end_access %7 : $*Builtin.Int32 + + %9 = function_ref @b_i_plus_one : $@convention(method) (@guaranteed B) -> () + %10 = apply %9(%5) : $@convention(method) (@guaranteed B) -> () + + %11 = begin_access [read] [dynamic] [no_nested_conflict] %6 : $*Builtin.Int32 + %12 = load %11 : $*Builtin.Int32 + end_access %11 : $*Builtin.Int32 + + release_value %4 : $Optional + %13 = tuple () + return %13 : $() +} From 775e28f7857793d6c1c5dd14da7f26f99a6f82a4 Mon Sep 17 00:00:00 2001 From: WeZZard Date: Tue, 11 Mar 2025 09:26:41 +0800 Subject: [PATCH 2/4] [SILOptimizer] Adds test cases for the escaping analysis of . --- test/SILOptimizer/escape_info.sil | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/SILOptimizer/escape_info.sil b/test/SILOptimizer/escape_info.sil index 00b241e56f22c..2230c83e8e223 100644 --- a/test/SILOptimizer/escape_info.sil +++ b/test/SILOptimizer/escape_info.sil @@ -1458,6 +1458,45 @@ bb0: return %2 : $X } +// CHECK-LABEL: Escape information for test_unchecked_ref_cast_from_optional_to_non_optional: +// CHECK: return[]: %0 = alloc_ref $Derived +// CHECK: - : %2 = alloc_ref $Derived +// CHECK: End function test_unchecked_ref_cast_from_optional_to_non_optional +sil @test_unchecked_ref_cast_from_optional_to_non_optional : $@convention(thin) () -> @owned X { +bb0: + %0 = alloc_ref $Derived + %1 = enum $Optional, #Optional.some!enumelt, %0 : $Derived + %2 = alloc_ref $Derived + %3 = enum $Optional, #Optional.some!enumelt, %2 : $Derived + %4 = unchecked_ref_cast %1 : $Optional to $X + %5 = unchecked_ref_cast %3 : $Optional to $X + return %4 : $X +} + +// CHECK-LABEL: Escape information for test_unchecked_ref_cast_from_optional_to_non_optional_2: +// CHECK-NEXT: End function test_unchecked_ref_cast_from_optional_to_non_optional_2 +sil @test_unchecked_ref_cast_from_optional_to_non_optional_2 : $@convention(thin) () -> @owned X { +bb0: + %0 = enum $Optional, #Optional.none!enumelt + %1 = enum $Optional, #Optional.none!enumelt + %2 = unchecked_ref_cast %0 : $Optional to $X + %3 = unchecked_ref_cast %1 : $Optional to $X + return %2 : $X +} + +// CHECK-LABEL: Escape information for test_unchecked_ref_cast_from_non_optional_to_optional: +// CHECK: return[e1]: %0 = alloc_ref $Derived +// CHECK: - : %1 = alloc_ref $Derived +// CHECK: End function test_unchecked_ref_cast_from_non_optional_to_optional +sil @test_unchecked_ref_cast_from_non_optional_to_optional : $@convention(thin) () -> @owned Optional { +bb0: + %0 = alloc_ref $Derived + %1 = alloc_ref $Derived + %2 = unchecked_ref_cast %0 : $Derived to $Optional + %3 = unchecked_ref_cast %1 : $Derived to $Optional + return %2 : $Optional +} + // CHECK-LABEL: Escape information for test_unchecked_addr_cast: // CHECK: global: %1 = alloc_ref $X // CHECK: End function test_unchecked_addr_cast From 2237faca04b9011c634250a392a3128d890827b7 Mon Sep 17 00:00:00 2001 From: WeZZard Date: Tue, 11 Mar 2025 09:29:11 +0800 Subject: [PATCH 3/4] [SILOptimizer] Refines the implementation of of the bridged SILType for Swift. --- SwiftCompilerSources/Sources/AST/Type.swift | 1 + SwiftCompilerSources/Sources/SIL/Type.swift | 2 +- include/swift/AST/ASTBridging.h | 1 + include/swift/AST/ASTBridgingImpl.h | 4 ++++ include/swift/AST/Types.h | 7 ------- include/swift/SIL/SILBridgingImpl.h | 3 ++- include/swift/SIL/SILType.h | 5 ----- 7 files changed, 9 insertions(+), 14 deletions(-) diff --git a/SwiftCompilerSources/Sources/AST/Type.swift b/SwiftCompilerSources/Sources/AST/Type.swift index 43bb247a4ac53..2259fdc90d1aa 100644 --- a/SwiftCompilerSources/Sources/AST/Type.swift +++ b/SwiftCompilerSources/Sources/AST/Type.swift @@ -93,6 +93,7 @@ extension TypeProperties { public var isEscapable: Bool { type.bridged.isEscapable() } public var isNoEscape: Bool { type.bridged.isNoEscape() } public var isInteger: Bool { type.bridged.isInteger() } + public var isOptional: Bool { type.bridged.isOptional() } public var isMetatypeType: Bool { type.bridged.isMetatypeType() } public var isExistentialMetatypeType: Bool { type.bridged.isExistentialMetatypeType() } public var representationOfMetatype: AST.`Type`.MetatypeRepresentation { diff --git a/SwiftCompilerSources/Sources/SIL/Type.swift b/SwiftCompilerSources/Sources/SIL/Type.swift index 82ecbf881fd5f..825f365df6ee2 100644 --- a/SwiftCompilerSources/Sources/SIL/Type.swift +++ b/SwiftCompilerSources/Sources/SIL/Type.swift @@ -63,7 +63,7 @@ public struct Type : CustomStringConvertible, NoReflectionChildren { public var isFunction: Bool { bridged.isFunction() } public var isMetatype: Bool { bridged.isMetatype() } public var isClassExistential: Bool { bridged.isClassExistential() } - public var isOptional: Bool { bridged.isOptional() } + public var isOptional: Bool { astType.isOptional } public var isNoEscapeFunction: Bool { bridged.isNoEscapeFunction() } public var containsNoEscapeFunction: Bool { bridged.containsNoEscapeFunction() } public var isThickFunction: Bool { bridged.isThickFunction() } diff --git a/include/swift/AST/ASTBridging.h b/include/swift/AST/ASTBridging.h index 19dc01eb0898c..a38d3370d745d 100644 --- a/include/swift/AST/ASTBridging.h +++ b/include/swift/AST/ASTBridging.h @@ -3021,6 +3021,7 @@ struct BridgedASTType { BRIDGED_INLINE bool isInteger() const; BRIDGED_INLINE bool isMetatypeType() const; BRIDGED_INLINE bool isExistentialMetatypeType() const; + BRIDGED_INLINE bool isOptional() const; BRIDGED_INLINE TraitResult canBeClass() const; SWIFT_IMPORT_UNSAFE BRIDGED_INLINE OptionalBridgedDeclObj getAnyNominal() const; SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedASTType getInstanceTypeOfMetatype() const; diff --git a/include/swift/AST/ASTBridgingImpl.h b/include/swift/AST/ASTBridgingImpl.h index b25606f162452..77ed258bb8400 100644 --- a/include/swift/AST/ASTBridgingImpl.h +++ b/include/swift/AST/ASTBridgingImpl.h @@ -430,6 +430,10 @@ bool BridgedASTType::isMetatypeType() const { return unbridged()->is(); } +bool BridgedASTType::isOptional() const { + return unbridged()->getCanonicalType()->isOptional(); +} + bool BridgedASTType::isExistentialMetatypeType() const { return unbridged()->is(); } diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index c42054599ab55..2c37b5caf89c8 100644 --- a/include/swift/AST/Types.h +++ b/include/swift/AST/Types.h @@ -795,9 +795,6 @@ class alignas(1 << TypeAlignInBits) TypeBase /// Determine whether the type is an opened existential type with Error inside bool isOpenedExistentialWithError(); - /// Determine whether the type is an Optional type. - bool isOptional() const; - /// Retrieve the set of type parameter packs that occur within this type. void getTypeParameterPacks(SmallVectorImpl &rootParameterPacks); @@ -7929,10 +7926,6 @@ inline bool TypeBase::isClassExistentialType() { return false; } -inline bool TypeBase::isOptional() const { - return getCanonicalType()->isOptional(); -} - inline bool TypeBase::canDynamicallyBeOptionalType(bool includeExistential) { CanType T = getCanonicalType(); auto isArchetypeOrExistential = isa(T) || diff --git a/include/swift/SIL/SILBridgingImpl.h b/include/swift/SIL/SILBridgingImpl.h index bed44984820ff..7910a5e85b2c8 100644 --- a/include/swift/SIL/SILBridgingImpl.h +++ b/include/swift/SIL/SILBridgingImpl.h @@ -403,7 +403,8 @@ bool BridgedType::isClassExistential() const { } bool BridgedType::isOptional() const { - return unbridged().isOptional(); + swift::CanType astType = unbridged().getASTType(); + return astType->isOptional(); } bool BridgedType::isNoEscapeFunction() const { diff --git a/include/swift/SIL/SILType.h b/include/swift/SIL/SILType.h index 564a3741a53eb..9b4aa899158c8 100644 --- a/include/swift/SIL/SILType.h +++ b/include/swift/SIL/SILType.h @@ -458,11 +458,6 @@ class SILType { return getASTType()->hasOpenedExistential(); } - /// Returns true if the referenced type is an Optional type. - bool isOptional() const { - return getASTType()->isOptional(); - } - TypeTraitResult canBeClass() const { return getASTType()->canBeClass(); } From 4c479824ef9b8365d83e8f58521003ac4d2e025b Mon Sep 17 00:00:00 2001 From: WeZZard Date: Tue, 11 Mar 2025 09:39:50 +0800 Subject: [PATCH 4/4] [SILOptimizer] Eliminate non-case handling for implicit Optional non-Optional casting of unchecked_ref_cast in ValueDefUseWalker. --- .../TestPasses/EscapeInfoDumper.swift | 2 +- .../Sources/SIL/Utilities/WalkUtils.swift | 29 +++++-------------- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/TestPasses/EscapeInfoDumper.swift b/SwiftCompilerSources/Sources/Optimizer/TestPasses/EscapeInfoDumper.swift index b00b6056e2197..873ba9bac330e 100644 --- a/SwiftCompilerSources/Sources/Optimizer/TestPasses/EscapeInfoDumper.swift +++ b/SwiftCompilerSources/Sources/Optimizer/TestPasses/EscapeInfoDumper.swift @@ -14,7 +14,7 @@ import SIL /// Dumps the results of escape analysis. /// -/// Dumps the EscapeInfo query results for all `alloc_stack` instructions in a function. +/// Dumps the EscapeInfo query results for all `alloc_ref` instructions in a function. /// /// This pass is used for testing EscapeInfo. let escapeInfoDumper = FunctionPass(name: "dump-escape-info") { diff --git a/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift b/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift index 069a25cc03aae..babcfbebc8b12 100644 --- a/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift +++ b/SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift @@ -376,19 +376,12 @@ extension ValueDefUseWalker { // accordingly. switch (urc.type.isOptional, urc.fromInstance.type.isOptional) { case (true, false): - if walkDownUses(ofValue: urc, path: path.push(.enumCase, index: 0)) == .abortWalk { - return .abortWalk - } return walkDownUses(ofValue: urc, path: path.push(.enumCase, index: 1)) case (false, true): - if let path = path.popIfMatches(.enumCase, index: 0) { - if walkDownUses(ofValue: urc, path: path) == .abortWalk { - return .abortWalk - } else if let path = path.popIfMatches(.enumCase, index: 1) { - return walkDownUses(ofValue: urc, path: path) - } + if let path = path.popIfMatches(.enumCase, index: 1) { + return walkDownUses(ofValue: urc, path: path) } - return .abortWalk + return unmatchedPath(value: operand, path: path) default: return walkDownUses(ofValue: urc, path: path) } @@ -726,20 +719,12 @@ extension ValueUseDefWalker { // accordingly. switch (urc.type.isOptional, urc.fromInstance.type.isOptional) { case (true, false): - if let path = path.popIfMatches(.enumCase, index: 0) { - if walkUp(value: urc.fromInstance, path: path) == .abortWalk { - return .abortWalk - } else if let path = path.popIfMatches(.enumCase, index: 1) { - return walkUp(value: urc.fromInstance, path: path) - } + if let path = path.popIfMatches(.enumCase, index: 1) { + return walkUp(value: urc.fromInstance, path: path) } - return .abortWalk + return unmatchedPath(value: urc.fromInstance, path: path) case (false, true): - if walkUp(value: urc.fromInstance, path: path.push(.enumCase, index: 0)) == .abortWalk { - return .abortWalk - } else { - return walkUp(value: urc.fromInstance, path: path.push(.enumCase, index: 1)) - } + return walkUp(value: urc.fromInstance, path: path.push(.enumCase, index: 1)) default: return walkUp(value: urc.fromInstance, path: path) }