Skip to content

Commit 418e9d4

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 8436db3 commit 418e9d4

File tree

2 files changed

+22
-28
lines changed

2 files changed

+22
-28
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)

0 commit comments

Comments
 (0)