Skip to content

Commit a06733b

Browse files
committed
SILCombine: disable unchecked_bitwise_cast -> unchekced_ref_cast conversion in OSSA
In ownership converting an unowned bitwise cast to a "real" ownership forwarding instruction can cause various troubles. Fixes a SIL verifier crash.
1 parent e265eb0 commit a06733b

File tree

3 files changed

+23
-14
lines changed

3 files changed

+23
-14
lines changed

lib/SILOptimizer/SILCombiner/SILCombinerCastVisitors.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,12 @@ visitUncheckedBitwiseCastInst(UncheckedBitwiseCastInst *UBCI) {
611611
UBCI->getLoc(), UBCI->getOperand(), UBCI->getType());
612612
}
613613

614+
// In ownership converting an unowned bitwise cast to a "real" ownership
615+
// forwarding instruction can cause various troubles.
616+
// Let's don't go into this business.
617+
if (Builder.hasOwnership())
618+
return nullptr;
619+
614620
if (!SILType::canRefCast(UBCI->getOperand()->getType(), UBCI->getType(),
615621
Builder.getModule()))
616622
return nullptr;
@@ -623,12 +629,6 @@ visitUncheckedBitwiseCastInst(UncheckedBitwiseCastInst *UBCI) {
623629
// an extra copy in the case that UBCI->getOperand() is Owned.
624630
auto *refCast = Builder.createUncheckedRefCast(
625631
UBCI->getLoc(), UBCI->getOperand(), UBCI->getType());
626-
if (Builder.hasOwnership()) {
627-
// A bitwise cast is always unowned, so we can safely force the reference
628-
// cast to forward as unowned and no ownership adjustment is needed.
629-
assert(UBCI->getOwnershipKind() == OwnershipKind::Unowned);
630-
refCast->setForwardingOwnershipKind(OwnershipKind::Unowned);
631-
}
632632
return refCast;
633633
}
634634

test/SILOptimizer/sil_combine_nocanonicalize_ossa.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ bb0:
8585
sil @get_native_object : $@convention(thin) () -> @owned Builtin.NativeObject
8686

8787
// CHECK-LABEL: sil [ossa] @bitwise_combines_guaranteed :
88-
// CHECK: unchecked_ref_cast
88+
// CHECK: unchecked_bitwise_cast
8989
// CHECK-LABEL: } // end sil function 'bitwise_combines_guaranteed'
9090
sil [ossa] @bitwise_combines_guaranteed : $@convention(thin) () -> @owned Optional<Builtin.NativeObject> {
9191
bb0:

test/SILOptimizer/sil_combine_ossa.sil

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2185,7 +2185,7 @@ bb0(%0 : @owned $Builtin.NativeObject, %1 : $Builtin.RawPointer):
21852185
// CHECK: bb0(
21862186
// CHECK-NEXT: unchecked_bitwise_cast
21872187
// CHECK-NEXT: unchecked_trivial_bit_cast
2188-
// CHECK-NEXT: unchecked_ref_cast
2188+
// CHECK-NEXT: unchecked_bitwise_cast
21892189
// CHECK-NOT: unchecked_bitwise_cast
21902190
// CHECK-NOT: unchecked_ownership_conversion
21912191
// CHECK: } // end sil function 'bitwise_combines_owned'
@@ -2212,7 +2212,7 @@ bb0(%0 : @owned $Builtin.NativeObject, %1 : $Builtin.RawPointer):
22122212
// CHECK: bb0(
22132213
// CHECK-NEXT: unchecked_bitwise_cast
22142214
// CHECK-NEXT: unchecked_trivial_bit_cast
2215-
// CHECK-NEXT: unchecked_ref_cast
2215+
// CHECK-NEXT: unchecked_bitwise_cast
22162216
// CHECK-NOT: unchecked_bitwise_cast
22172217
// CHECK-NOT: unchecked_ownership_conversion
22182218
// CHECK: } // end sil function 'bitwise_combines_guaranteed'
@@ -2225,14 +2225,25 @@ bb0(%0 : @guaranteed $Builtin.NativeObject, %1 : $Builtin.RawPointer):
22252225
// promote trivial
22262226
%4 = unchecked_bitwise_cast %0 : $Builtin.NativeObject to $Builtin.RawPointer
22272227

2228-
// promote ref
2228+
// don't promote ref
22292229
%5 = unchecked_bitwise_cast %0 : $Builtin.NativeObject to $Optional<Builtin.NativeObject>
22302230

22312231
%6 = tuple(%3 : $Optional<Builtin.NativeObject>, %4 : $Builtin.RawPointer, %5 : $Optional<Builtin.NativeObject>)
22322232
%6a = copy_value %6 : $(Optional<Builtin.NativeObject>, Builtin.RawPointer, Optional<Builtin.NativeObject>)
22332233
return %6a : $(Optional<Builtin.NativeObject>, Builtin.RawPointer, Optional<Builtin.NativeObject>)
22342234
}
22352235

2236+
// CHECK-LABEL: @dont_fold_unchecked_bitwise_cast_to_unowned :
2237+
// CHECK: %1 = unchecked_bitwise_cast %0 : $E to $B
2238+
// CHECK: } // end sil function 'dont_fold_unchecked_bitwise_cast_to_unowned'
2239+
sil [ossa] @dont_fold_unchecked_bitwise_cast_to_unowned : $@convention(thin) (@owned E) -> @owned B {
2240+
bb0(%0 : @owned $E):
2241+
%1 = unchecked_bitwise_cast %0 to $B
2242+
%2 = copy_value %1
2243+
destroy_value %0
2244+
return %2
2245+
}
2246+
22362247
// CHECK-LABEL: @remove_pointer_compare_to_zero :
22372248
// CHECK-NOT: apply
22382249
// CHECK-NOT: cond_fail
@@ -4669,11 +4680,9 @@ bb0:
46694680
}
46704681

46714682

4672-
// Check for disabled optimization of unchecked_bitwise_cast to unchecked_ref_cast in ossa
4673-
// This test can be optimized when ossa is supported in the SILCombine for unchecked_bitwise_cast
46744683
// CHECK-LABEL: sil [ossa] @refcast :
46754684
// CHECK: bb0
4676-
// CHECK-NEXT: unchecked_ref_cast
4685+
// CHECK-NEXT: unchecked_bitwise_cast
46774686
// CHECK-NEXT: copy_value
46784687
// CHECK-NEXT: destroy_value
46794688
// CHECK-NEXT: return
@@ -5072,7 +5081,7 @@ bb2(%error : @owned $Error):
50725081
sil [ossa] @takeKlass : $@convention(thin) (@owned Optional<Klass>) -> ()
50735082

50745083
// CHECK_FORWARDING_OWNERSHIP_KIND-LABEL: sil shared [ossa] @$s8refcast24main5KlassC_Tg5 :
5075-
// CHECK_FORWARDING_OWNERSHIP_KIND: unchecked_ref_cast %1 : $Klass to $Optional<Klass>, forwarding: @unowned
5084+
// CHECK_FORWARDING_OWNERSHIP_KIND: unchecked_bitwise_cast
50765085
// CHECK_FORWARDING_OWNERSHIP_KIND-LABEL: } // end sil function '$s8refcast24main5KlassC_Tg5'
50775086
sil [ossa] @refcast2 : $@convention(thin) <T> (@in T, @owned Klass) -> () {
50785087
bb0(%0 : $*T, %1 : @owned $Klass):

0 commit comments

Comments
 (0)