Skip to content

Commit 35f82ba

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 737b5ec commit 35f82ba

File tree

7 files changed

+105
-2
lines changed

7 files changed

+105
-2
lines changed

SwiftCompilerSources/Sources/SIL/Type.swift

+1
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

+45-2
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

+7
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,9 @@ class alignas(1 << TypeAlignInBits) TypeBase
795795
/// Determine whether the type is an opened existential type with Error inside
796796
bool isOpenedExistentialWithError();
797797

798+
/// Determine whether the type is an Optional type.
799+
bool isOptional() const;
800+
798801
/// Retrieve the set of type parameter packs that occur within this type.
799802
void getTypeParameterPacks(SmallVectorImpl<Type> &rootParameterPacks);
800803

@@ -7926,6 +7929,10 @@ inline bool TypeBase::isClassExistentialType() {
79267929
return false;
79277930
}
79287931

7932+
inline bool TypeBase::isOptional() const {
7933+
return getCanonicalType()->isOptional();
7934+
}
7935+
79297936
inline bool TypeBase::canDynamicallyBeOptionalType(bool includeExistential) {
79307937
CanType T = getCanonicalType();
79317938
auto isArchetypeOrExistential = isa<ArchetypeType>(T) ||

include/swift/SIL/SILBridging.h

+1
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ struct BridgedType {
263263
BRIDGED_INLINE bool isFunction() const;
264264
BRIDGED_INLINE bool isMetatype() const;
265265
BRIDGED_INLINE bool isClassExistential() const;
266+
BRIDGED_INLINE bool isOptional() const;
266267
BRIDGED_INLINE bool isNoEscapeFunction() const;
267268
BRIDGED_INLINE bool containsNoEscapeFunction() const;
268269
BRIDGED_INLINE bool isThickFunction() const;

include/swift/SIL/SILBridgingImpl.h

+4
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,10 @@ bool BridgedType::isClassExistential() const {
402402
return unbridged().isClassExistentialType();
403403
}
404404

405+
bool BridgedType::isOptional() const {
406+
return unbridged().isOptional();
407+
}
408+
405409
bool BridgedType::isNoEscapeFunction() const {
406410
return unbridged().isNoEscapeFunction();
407411
}

include/swift/SIL/SILType.h

+5
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,11 @@ class SILType {
458458
return getASTType()->hasOpenedExistential();
459459
}
460460

461+
/// Returns true if the referenced type is an Optional type.
462+
bool isOptional() const {
463+
return getASTType()->isOptional();
464+
}
465+
461466
TypeTraitResult canBeClass() const {
462467
return getASTType()->canBeClass();
463468
}

test/SILOptimizer/redundant_load_elim_with_casts.sil

+42
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,44 @@ 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+
// Tests unchecked_ref_cast between Optional<ClassType1> and ClassType2.
553+
// E? -> B is safe
554+
//
555+
// CHECK-FUTURE: sil @unchecked_ref_cast_from_optional_class
556+
// CHECK: bb3(%6 : $Optional<AnyObject>):
557+
// CHECK: %8 = load %7 : $*Builtin.Int32
558+
// CHECK: %10 = apply %9(%5) : $@convention(method) (@guaranteed B) -> ()
559+
// CHECK: %12 = load %11 : $*Builtin.Int32
560+
// CHECK: return
561+
sil @unchecked_ref_cast_from_optional_class : $@convention(thin) (Optional<E>) -> () {
562+
bb0(%0 : $Optional<E>):
563+
switch_enum %0 : $Optional<E>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
564+
565+
bb1(%1 : $E):
566+
%2 = enum $Optional<E>, #Optional.some!enumelt, %1 : $E
567+
br bb3(%2 : $Optional<E>)
568+
569+
bb2:
570+
%3 = enum $Optional<E>, #Optional.none!enumelt
571+
br bb3(%3 : $Optional<E>)
572+
573+
bb3(%4 : $Optional<E>):
574+
%5 = unchecked_ref_cast %4 : $Optional<E> to $B
575+
576+
%6 = ref_element_addr %5 : $B, #B.i
577+
%7 = begin_access [read] [dynamic] [no_nested_conflict] %6 : $* Builtin.Int32
578+
%8 = load %7 : $*Builtin.Int32
579+
end_access %7 : $*Builtin.Int32
580+
581+
%9 = function_ref @b_i_plus_one : $@convention(method) (@guaranteed B) -> ()
582+
%10 = apply %9(%5) : $@convention(method) (@guaranteed B) -> ()
583+
584+
%11 = begin_access [read] [dynamic] [no_nested_conflict] %6 : $*Builtin.Int32
585+
%12 = load %11 : $*Builtin.Int32
586+
end_access %11 : $*Builtin.Int32
587+
588+
release_value %4 : $Optional<E>
589+
%13 = tuple ()
590+
return %13 : $()
591+
}

0 commit comments

Comments
 (0)