Skip to content

Commit 4b46200

Browse files
committed
Optimizer: Fix InstructionRange's begin instruction
Just don't store the begin instruction. This led to problem if the "begin" was not actually an instruction but a block argument. Using the first instruction of that block is not correct in case the range ends immediately at the first instruction, e.g. ``` bb0(%0 : @owned $C): destroy_value %0 ```
1 parent 6a6cc48 commit 4b46200

File tree

5 files changed

+25
-50
lines changed

5 files changed

+25
-50
lines changed

SwiftCompilerSources/Sources/Optimizer/DataStructures/InstructionRange.swift

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,6 @@ import SIL
4040
/// destruct this data structure, e.g. in a `defer {}` block.
4141
struct InstructionRange : CustomStringConvertible, NoReflectionChildren {
4242

43-
/// The dominating begin instruction.
44-
let begin: Instruction
45-
4643
/// The underlying block range.
4744
private(set) var blockRange: BasicBlockRange
4845

@@ -52,36 +49,37 @@ struct InstructionRange : CustomStringConvertible, NoReflectionChildren {
5249
private var inExclusiveRange: InstructionSet
5350

5451
init(begin beginInst: Instruction, _ context: some Context) {
55-
self.begin = beginInst
56-
self.blockRange = BasicBlockRange(begin: beginInst.parentBlock, context)
57-
self.insertedInsts = InstructionSet(context)
58-
self.inExclusiveRange = InstructionSet(context)
52+
self = InstructionRange(beginBlock: beginInst.parentBlock, context)
5953
self.inExclusiveRange.insert(beginInst)
6054
}
6155

6256
init(for value: Value, _ context: some Context) {
63-
self = InstructionRange(begin: InstructionRange.beginningInstruction(for: value), context)
57+
if let inst = value.definingInstruction {
58+
self = InstructionRange(begin: inst, context)
59+
} else if let arg = value as? Argument {
60+
self = InstructionRange(beginBlock: arg.parentBlock, context)
61+
} else {
62+
fatalError("cannot build an instruction range for \(value)")
63+
}
6464
}
6565

66-
static func beginningInstruction(for value: Value) -> Instruction {
67-
if let def = value.definingInstructionOrTerminator {
68-
return def
69-
}
70-
assert(Phi(value) != nil || value is FunctionArgument)
71-
return value.parentBlock.instructions.first!
66+
private init(beginBlock: BasicBlock, _ context: some Context) {
67+
self.blockRange = BasicBlockRange(begin: beginBlock, context)
68+
self.insertedInsts = InstructionSet(context)
69+
self.inExclusiveRange = InstructionSet(context)
7270
}
7371

7472
/// Insert a potential end instruction.
7573
mutating func insert(_ inst: Instruction) {
7674
insertedInsts.insert(inst)
7775
insertIntoRange(instructions: ReverseInstructionList(first: inst.previous))
7876
blockRange.insert(inst.parentBlock)
79-
if inst.parentBlock != begin.parentBlock {
77+
if inst.parentBlock != blockRange.begin {
8078
// The first time an instruction is inserted in another block than the begin-block we need to insert
8179
// instructions from the begin instruction to the end of the begin block.
8280
// For subsequent insertions this is a no-op: `insertIntoRange` will return immediately because those
8381
// instruction are already inserted.
84-
insertIntoRange(instructions: begin.parentBlock.instructions.reversed())
82+
insertIntoRange(instructions: blockRange.begin.instructions.reversed())
8583
}
8684
}
8785

@@ -98,22 +96,14 @@ struct InstructionRange : CustomStringConvertible, NoReflectionChildren {
9896
return true
9997
}
10098
let block = inst.parentBlock
101-
return block != begin.parentBlock && blockRange.contains(block)
99+
return block != blockRange.begin && blockRange.contains(block)
102100
}
103101

104102
/// Returns true if the inclusive range contains `inst`.
105103
func inclusiveRangeContains (_ inst: Instruction) -> Bool {
106104
contains(inst) || insertedInsts.contains(inst)
107105
}
108106

109-
/// Returns true if the range is valid and that's iff the begin instruction
110-
/// dominates all instructions of the range.
111-
var isValid: Bool {
112-
blockRange.isValid &&
113-
// Check if there are any inserted instructions before the begin instruction in its block.
114-
!ReverseInstructionList(first: begin).dropFirst().contains { insertedInsts.contains($0) }
115-
}
116-
117107
/// Returns the end instructions.
118108
///
119109
/// Warning: this returns `begin` if no instructions were inserted.
@@ -155,6 +145,10 @@ struct InstructionRange : CustomStringConvertible, NoReflectionChildren {
155145
}
156146
}
157147

148+
var begin: Instruction? {
149+
blockRange.begin.instructions.first(where: inExclusiveRange.contains)
150+
}
151+
158152
private mutating func insertIntoRange(instructions: ReverseInstructionList) {
159153
for inst in instructions {
160154
if !inExclusiveRange.insert(inst) {
@@ -164,9 +158,9 @@ struct InstructionRange : CustomStringConvertible, NoReflectionChildren {
164158
}
165159

166160
var description: String {
167-
return (isValid ? "" : "<invalid>\n") +
161+
return (blockRange.isValid ? "" : "<invalid>\n") +
168162
"""
169-
begin: \(begin)
163+
begin: \(begin?.description ?? blockRange.begin.name)
170164
ends: \(ends.map { $0.description }.joined(separator: "\n "))
171165
exits: \(exits.map { $0.description }.joined(separator: "\n "))
172166
interiors:\(interiors.map { $0.description }.joined(separator: "\n "))

SwiftCompilerSources/Sources/Optimizer/Utilities/OwnershipLiveness.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ struct LivenessBoundary : CustomStringConvertible {
913913

914914
// Compute the boundary of a singly-defined range.
915915
init(value: Value, range: InstructionRange, _ context: Context) {
916-
assert(range.isValid)
916+
assert(range.blockRange.isValid)
917917

918918
lastUsers = Stack<Instruction>(context)
919919
boundaryEdges = Stack<BasicBlock>(context)

test/SILOptimizer/lifetime_dependence/lifetime_dependence_util.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ entry(%0 : @owned $C, %1 : @owned $D, %2 : @guaranteed $D, %3 : $*D, %4 : $*D):
157157
// CHECK-LABEL: dependence_scope: lifetime_dependence_scope with: %inguaranteed_arg_mark
158158
// CHECK-NEXT: Initialized: %3 = argument of bb0 : $*D
159159
// CHECK-NEXT: Dependent: %{{.*}} = mark_dependence [nonescaping] %{{.*}} : $C on %3 : $*D
160-
// CHECK-NEXT: begin: %{{.*}} = move_value %{{.*}} : $D
160+
// CHECK-NEXT: begin: bb0
161161
// CHECK-NEXT: ends:
162162
// CHECK: dependence_scope: lifetime_dependence_scope with: %inguaranteed_arg_mark
163163

test/SILOptimizer/ownership_liveness_unit.sil

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ bb0(%0 : $*D, %1 : @guaranteed $D):
433433

434434
// CHECK-LABEL: begin running test 2 of 2 on testInteriorDropDeinit: interior_liveness_swift with: %0
435435
// CHECK: Interior liveness: %0 = argument of bb0 : $NCInt
436-
// CHECK-NEXT: begin: [[DD:%.*]] = drop_deinit %0 : $NCInt
436+
// CHECK-NEXT: begin: bb0
437437
// CHECK-NEXT: ends: [[DD]] = drop_deinit %0 : $NCInt
438438
// CHECK-NEXT: exits:
439439
// CHECK-NEXT: interiors:
@@ -1122,7 +1122,7 @@ bb4(%inner4 : @guaranteed $D):
11221122

11231123
// CHECK-LABEL: testInnerNonAdjacentReborrow: interior_liveness_swift with: %outer3
11241124
// CHECK: Interior liveness: %{{.*}} = argument of bb3 : $D
1125-
// CHECK-NEXT: begin: {{.*}} borrowed {{.*}} from
1125+
// CHECK-NEXT: begin: bb3
11261126
// CHECK-NEXT: ends:
11271127
// CHECK-NEXT: exits:
11281128
// CHECK-NEXT: interiors:

test/SILOptimizer/ranges.sil

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -129,22 +129,3 @@ bb3:
129129
return %r : $()
130130
}
131131

132-
// CHECK-LABEL:Instruction range in valid_blocks_invalid_instructions:
133-
// CHECK-NEXT: <invalid>
134-
// CHECK: Block range in valid_blocks_invalid_instructions:
135-
// CHECK-NEXT: begin: bb0
136-
// CHECK-NEXT: range: []
137-
// CHECK-NEXT: inclrange: [bb0]
138-
// CHECK-NEXT: ends: [bb0]
139-
// CHECK-NEXT: exits: []
140-
// CHECK-NEXT: interiors: []
141-
// CHECK-NEXT: End function valid_blocks_invalid_instructions
142-
sil @valid_blocks_invalid_instructions : $@convention(thin) () -> () {
143-
bb0:
144-
%0 = string_literal utf8 "end"
145-
%1 = string_literal utf8 "begin"
146-
%2 = string_literal utf8 "end"
147-
%r = tuple()
148-
return %r : $()
149-
}
150-

0 commit comments

Comments
 (0)