Skip to content

Commit 21757d0

Browse files
committed
[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<ClassType>` and `ClassType`. We need to handle these cases by checking if the type is optional and adjust the path accordingly.
1 parent ef3e804 commit 21757d0

File tree

6 files changed

+137
-2
lines changed

6 files changed

+137
-2
lines changed

SwiftCompilerSources/Sources/SIL/Type.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ public struct Type : CustomStringConvertible, NoReflectionChildren {
6363
public var isFunction: Bool { bridged.isFunction() }
6464
public var isMetatype: Bool { bridged.isMetatype() }
6565
public var isClassExistential: Bool { bridged.isClassExistential() }
66+
public var isOptional: Bool { bridged.isOptional() }
6667
public var isNoEscapeFunction: Bool { bridged.isNoEscapeFunction() }
6768
public var containsNoEscapeFunction: Bool { bridged.containsNoEscapeFunction() }
6869
public var isThickFunction: Bool { bridged.isThickFunction() }

SwiftCompilerSources/Sources/SIL/Utilities/WalkUtils.swift

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,28 @@ extension ValueDefUseWalker {
370370
// We need to ignore this because otherwise the path wouldn't contain the right `existential` field kind.
371371
return leafUse(value: operand, path: path)
372372
}
373-
return walkDownUses(ofValue: urc, path: path)
373+
// The `unchecked_ref_cast` is designed to be able to cast between
374+
// `Optional<ClassType>` and `ClassType`. We need to handle these
375+
// cases by checking if the type is optional and adjust the path
376+
// accordingly.
377+
switch (urc.type.isOptional, urc.fromInstance.type.isOptional) {
378+
case (true, false):
379+
if walkDownUses(ofValue: urc, path: path.push(.enumCase, index: 0)) == .abortWalk {
380+
return .abortWalk
381+
}
382+
return walkDownUses(ofValue: urc, path: path.push(.enumCase, index: 1))
383+
case (false, true):
384+
if let path = path.popIfMatches(.enumCase, index: 0) {
385+
if walkDownUses(ofValue: urc, path: path) == .abortWalk {
386+
return .abortWalk
387+
} else if let path = path.popIfMatches(.enumCase, index: 1) {
388+
return walkDownUses(ofValue: urc, path: path)
389+
}
390+
}
391+
return .abortWalk
392+
default:
393+
return walkDownUses(ofValue: urc, path: path)
394+
}
374395
case let beginDealloc as BeginDeallocRefInst:
375396
if operand.index == 0 {
376397
return walkDownUses(ofValue: beginDealloc, path: path)
@@ -699,7 +720,29 @@ extension ValueUseDefWalker {
699720
// We need to ignore this because otherwise the path wouldn't contain the right `existential` field kind.
700721
return rootDef(value: urc, path: path)
701722
}
702-
return walkUp(value: urc.fromInstance, path: path)
723+
// The `unchecked_ref_cast` is designed to be able to cast between
724+
// `Optional<ClassType>` and `ClassType`. We need to handle these
725+
// cases by checking if the type is optional and adjust the path
726+
// accordingly.
727+
switch (urc.type.isOptional, urc.fromInstance.type.isOptional) {
728+
case (true, false):
729+
if let path = path.popIfMatches(.enumCase, index: 0) {
730+
if walkUp(value: urc.fromInstance, path: path) == .abortWalk {
731+
return .abortWalk
732+
} else if let path = path.popIfMatches(.enumCase, index: 1) {
733+
return walkUp(value: urc.fromInstance, path: path)
734+
}
735+
}
736+
return .abortWalk
737+
case (false, true):
738+
if walkUp(value: urc.fromInstance, path: path.push(.enumCase, index: 0)) == .abortWalk {
739+
return .abortWalk
740+
} else {
741+
return walkUp(value: urc.fromInstance, path: path.push(.enumCase, index: 1))
742+
}
743+
default:
744+
return walkUp(value: urc.fromInstance, path: path)
745+
}
703746
case let arg as Argument:
704747
if let phi = Phi(arg) {
705748
for incoming in phi.incomingValues {

include/swift/AST/Types.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,9 @@ class alignas(1 << TypeAlignInBits) TypeBase
799799
/// Determine whether the type is an opened existential type with Error inside
800800
bool isOpenedExistentialWithError();
801801

802+
/// Determine whether the type is an Optional type.
803+
bool isOptional() const;
804+
802805
/// Retrieve the set of type parameter packs that occur within this type.
803806
void getTypeParameterPacks(SmallVectorImpl<Type> &rootParameterPacks);
804807

@@ -7909,6 +7912,10 @@ inline bool TypeBase::isOpenedExistential() const {
79097912
return isa<OpenedArchetypeType>(T);
79107913
}
79117914

7915+
inline bool TypeBase::isOptional() const {
7916+
return getCanonicalType()->isOptional();
7917+
}
7918+
79127919
inline bool TypeBase::canDynamicallyBeOptionalType(bool includeExistential) {
79137920
CanType T = getCanonicalType();
79147921
auto isArchetypeOrExistential = isa<ArchetypeType>(T) ||

include/swift/SIL/SILBridgingImpl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,10 @@ bool BridgedType::isClassExistential() const {
397397
return unbridged().isClassExistentialType();
398398
}
399399

400+
bool BridgedType::isOptional() const {
401+
return unbridged().isOptional();
402+
}
403+
400404
bool BridgedType::isNoEscapeFunction() const {
401405
return unbridged().isNoEscapeFunction();
402406
}

include/swift/SIL/SILType.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,11 @@ class SILType {
464464
return getASTType()->hasOpenedExistential();
465465
}
466466

467+
/// Returns true if the referenced type is an Optional type.
468+
bool isOptional() const {
469+
return getASTType()->isOptional();
470+
}
471+
467472
TypeTraitResult canBeClass() const {
468473
return getASTType()->canBeClass();
469474
}

test/SILOptimizer/redundant_load_elim_with_casts.sil

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ sil @use : $@convention(thin) (Builtin.Int32) -> ()
5252
sil @use_a : $@convention(thin) (@in A) -> ()
5353
sil @escaped_a_ptr : $@convention(thin) () -> @out A
5454
sil @escaped_a : $@convention(thin) () -> Builtin.RawPointer
55+
sil @b_i_plus_one : $@convention(method) (@guaranteed B) -> ()
5556

5657
// *NOTE* This does not handle raw pointer since raw pointer is only layout compatible with heap references.
5758
// CHECK-FUTURE: sil @store_to_load_forward_unchecked_addr_cast_struct : $@convention(thin) (Optional<A>) -> () {
@@ -547,3 +548,77 @@ bb0(%0 : $*(I32, I32, I32), %1 : $*((I32, I32), I32)):
547548
%63 = tuple (%60 : $(I32, I32), %61 : $(I32, I32), %62 : $(I32, I32, I32))
548549
return %63 : $((I32, I32), (I32, I32), (I32, I32, I32))
549550
}
551+
552+
// Promote unchecked_ref_cast between Optional<ClassType1> and ClassType2.
553+
// E? -> B is safe
554+
// B -> E? is safe
555+
//
556+
// CHECK-FUTURE: sil @unchecked_ref_cast_from_optional_class
557+
// CHECK: bb3(%6 : $Optional<AnyObject>):
558+
// CHECK: %8 = load %7 : $*Builtin.Int32
559+
// CHECK: %10 = apply %9(%5) : $@convention(method) (@guaranteed B) -> ()
560+
// CHECK: %12 = load %11 : $*Builtin.Int32
561+
// CHECK: return
562+
sil @unchecked_ref_cast_from_optional_class : $@convention(thin) (Optional<E>) -> () {
563+
bb0(%0 : $Optional<E>):
564+
switch_enum %0 : $Optional<E>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
565+
566+
bb1(%1 : $E):
567+
%2 = enum $Optional<E>, #Optional.some!enumelt, %1 : $E
568+
br bb3(%2 : $Optional<E>)
569+
570+
bb2:
571+
%3 = enum $Optional<E>, #Optional.none!enumelt
572+
br bb3(%3 : $Optional<E>)
573+
574+
bb3(%4 : $Optional<E>):
575+
%5 = unchecked_ref_cast %4 : $Optional<E> to $B
576+
577+
%6 = ref_element_addr %5 : $B, #B.i
578+
%7 = begin_access [read] [dynamic] [no_nested_conflict] %6 : $* Builtin.Int32
579+
%8 = load %7 : $*Builtin.Int32
580+
end_access %7 : $*Builtin.Int32
581+
582+
%9 = function_ref @b_i_plus_one : $@convention(method) (@guaranteed B) -> ()
583+
%10 = apply %9(%5) : $@convention(method) (@guaranteed B) -> ()
584+
585+
%11 = begin_access [read] [dynamic] [no_nested_conflict] %6 : $*Builtin.Int32
586+
%12 = load %11 : $*Builtin.Int32
587+
end_access %11 : $*Builtin.Int32
588+
589+
release_value %4 : $Optional<E>
590+
%13 = tuple ()
591+
return %13 : $()
592+
}
593+
594+
sil @unchecked_ref_cast_to_optional_class : $@convention(thin) (Optional<E>) -> () {
595+
bb0(%0 : $Optional<E>):
596+
switch_enum %0 : $Optional<E>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
597+
598+
bb1(%1 : $E):
599+
%2 = enum $Optional<E>, #Optional.some!enumelt, %1 : $E
600+
br bb3(%2 : $Optional<E>)
601+
602+
bb2:
603+
%3 = enum $Optional<E>, #Optional.none!enumelt
604+
br bb3(%3 : $Optional<E>)
605+
606+
bb3(%4 : $Optional<E>):
607+
%5 = unchecked_ref_cast %4 : $Optional<E> to $B
608+
609+
%6 = ref_element_addr %5 : $B, #B.i
610+
%7 = begin_access [read] [dynamic] [no_nested_conflict] %6 : $* Builtin.Int32
611+
%8 = load %7 : $*Builtin.Int32
612+
end_access %7 : $*Builtin.Int32
613+
614+
%9 = function_ref @b_i_plus_one : $@convention(method) (@guaranteed B) -> ()
615+
%10 = apply %9(%5) : $@convention(method) (@guaranteed B) -> ()
616+
617+
%11 = begin_access [read] [dynamic] [no_nested_conflict] %6 : $*Builtin.Int32
618+
%12 = load %11 : $*Builtin.Int32
619+
end_access %11 : $*Builtin.Int32
620+
621+
release_value %4 : $Optional<E>
622+
%13 = tuple ()
623+
return %13 : $()
624+
}

0 commit comments

Comments
 (0)