Skip to content

Commit ffdb5db

Browse files
jckarterjckarter
committed
Optimizer/OptUtils: Don't trivially remove dead-looking unchecked_enum_data.
Under OSSA, the instruction may still be structurally responsible for consuming its operand even if the result is dead, so we can't remove it without breaking invariants. More generally, this should probably apply to any instruction which consumes one or more of its operands, has no side effects, and doesn't produce any nontrivial results that require further consumption to keep the value alive. I went with this targeted fix, since it addresses a problem that shows up in practice (rdar://125381446) and the more general change appears to disturb the optimizer pipeline while building the standard library.
1 parent 34c2f92 commit ffdb5db

File tree

6 files changed

+56
-7
lines changed

6 files changed

+56
-7
lines changed

‎SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,10 @@ extension Instruction {
289289
if bi.id == .OnFastPath {
290290
return false
291291
}
292+
case is UncheckedEnumDataInst:
293+
// Don't remove UncheckedEnumDataInst in OSSA in case it is responsible
294+
// for consuming an enum value.
295+
return !parentFunction.hasOwnership
292296
default:
293297
break
294298
}

‎test/SILOptimizer/moveonly_addresschecker_diagnostics.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %target-swift-emit-sil -O -sil-verify-all -verify -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyPartialReinitialization -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
2-
// RUN: %target-swift-emit-sil -enable-experimental-feature NoncopyableGenerics -O -sil-verify-all -verify -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyPartialReinitialization -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
1+
// RUN: %target-swift-emit-sil -O -sil-verify-all -verify -enable-experimental-feature BorrowingSwitch -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyPartialReinitialization -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
2+
// RUN: %target-swift-emit-sil -enable-experimental-feature BorrowingSwitch -enable-experimental-feature NoncopyableGenerics -O -sil-verify-all -verify -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyPartialReinitialization -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
33

44
//////////////////
55
// Declarations //

‎test/SILOptimizer/moveonly_objectchecker_diagnostics.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify -enable-experimental-feature MoveOnlyClasses %s
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature BorrowingSwitch -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify -enable-experimental-feature MoveOnlyClasses %s
22

33
//////////////////
44
// Declarations //

‎test/SILOptimizer/moveonly_trivial_addresschecker_diagnostics.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyPartialReinitialization -sil-verify-all -verify %s
2-
// RUN: %target-swift-emit-sil -enable-experimental-feature NoncopyableGenerics -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyPartialReinitialization -sil-verify-all -verify %s
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature BorrowingSwitch -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyPartialReinitialization -sil-verify-all -verify %s
2+
// RUN: %target-swift-emit-sil -enable-experimental-feature BorrowingSwitch -enable-experimental-feature NoncopyableGenerics -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature MoveOnlyPartialReinitialization -sil-verify-all -verify %s
33

44
//////////////////
55
// Declarations //

‎test/SILOptimizer/moveonly_trivial_objectchecker_diagnostics.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %target-swift-emit-sil -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify %s
2-
// RUN: %target-swift-emit-sil -enable-experimental-feature NoncopyableGenerics -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify %s
1+
// RUN: %target-swift-emit-sil -enable-experimental-feature BorrowingSwitch -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify %s
2+
// RUN: %target-swift-emit-sil -enable-experimental-feature BorrowingSwitch -enable-experimental-feature NoncopyableGenerics -enable-experimental-feature MoveOnlyPartialConsumption -sil-verify-all -verify %s
33

44
//////////////////
55
// Declarations //

‎test/SILOptimizer/onone_simplifications_trivial_enum.sil

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// RUN: %target-sil-opt --onone-simplification %s | %FileCheck %s
2+
3+
// REQUIRES: swift_in_compiler
4+
5+
import Builtin
6+
7+
class Klass {}
8+
9+
enum EnumTy: ~Copyable {
10+
case klass(Klass)
11+
case int(Builtin.Word)
12+
}
13+
14+
// CHECK-LABEL: sil [ossa] @test_switch_trivial_arm
15+
sil [ossa] @test_switch_trivial_arm : $@convention(thin) (@guaranteed EnumTy) -> () {
16+
entry(%a : @guaranteed $EnumTy):
17+
%b = explicit_copy_value %a : $EnumTy
18+
// CHECK: [[MOVE:%.*]] = move_value
19+
%c = move_value %b : $EnumTy
20+
// CHECK: [[BORROW:%.*]] = begin_borrow [[MOVE]]
21+
%d = begin_borrow %c : $EnumTy
22+
// CHECK: switch_enum [[BORROW]] : {{.*}}, case #EnumTy.int!enumelt: [[INTBB:bb[0-9]+]]
23+
switch_enum %d : $EnumTy, case #EnumTy.klass!enumelt: klass, case #EnumTy.int!enumelt: int
24+
25+
klass(%e : @guaranteed $Klass):
26+
end_borrow %d : $EnumTy
27+
%f = unchecked_enum_data %c : $EnumTy, #EnumTy.klass!enumelt
28+
destroy_value %f : $Klass
29+
br end
30+
31+
// We shouldn't remove the `unchecked_enum_data` here because it consumes the
32+
// original enum, even though it otherwise appears dead.
33+
// CHECK: [[INTBB]]({{.*}}):
34+
int(%p : $Builtin.Word):
35+
// CHECK: end_borrow [[BORROW]]
36+
end_borrow %d : $EnumTy
37+
// CHECK: unchecked_enum_data [[MOVE]]
38+
%q = unchecked_enum_data %c : $EnumTy, #EnumTy.int!enumelt
39+
br end
40+
41+
end:
42+
return undef : $()
43+
}
44+
45+

0 commit comments

Comments
 (0)