Skip to content

Commit 70d09b5

Browse files
atrickatrick
committed
OSSA: Disable cond_br arguments
In OSSA, there is no reason for the cond_br instruction to take arguments. In OSSA utilities, we have two types of block arguments: terminator results and phis. cond_br arguments don't fit into either one of these categories. Supporting them would be prohibitively complex. In fact, I have no idea model it. I have already made the assumption in several places that we don't support cond_br args. It would be impossible to test all the code that makes this assumption, so even if we wanted to support cond_br args, they would always be broken in practice. Simply mark them illegal in the SILVerifier and make sure no OSSA passes generate them.
1 parent ee0e69b commit 70d09b5

9 files changed

+55
-46
lines changed

‎lib/SIL/Verifier/SILVerifier.cpp‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -955,14 +955,17 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
955955
// Verify that the `isPhiArgument` property is sound:
956956
// - Phi arguments come from branches.
957957
// - Non-phi arguments have a single predecessor.
958-
if (arg->isPhiArgument()) {
959-
for (SILBasicBlock *predBB : arg->getParent()->getPredecessorBlocks()) {
960-
auto *TI = predBB->getTerminator();
961-
// FIXME: when critical edges are removed, only allow BranchInst.
962-
require(isa <BranchInst>(TI) || isa<CondBranchInst>(TI),
958+
assert(arg->isPhiArgument() && "precondition");
959+
for (SILBasicBlock *predBB : arg->getParent()->getPredecessorBlocks()) {
960+
auto *TI = predBB->getTerminator();
961+
if (F.hasOwnership()) {
962+
require(isa<BranchInst>(TI), "All phi inputs must be branch operands.");
963+
} else {
964+
// FIXME: when critical edges are removed and cond_br arguments are
965+
// disallowed, only allow BranchInst.
966+
require(isa<BranchInst>(TI) || isa<CondBranchInst>(TI),
963967
"All phi argument inputs must be from branches.");
964968
}
965-
} else {
966969
}
967970
if (arg->isPhiArgument() && prohibitAddressPhis()) {
968971
// As a property of well-formed SIL, we disallow address-type

‎test/SILOptimizer/accessed_storage.sil‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -389,13 +389,13 @@ bb1(%5 : $Builtin.RawPointer):
389389
%9 = integer_literal $Builtin.Word, 1
390390
%10 = index_addr %6 : $*AnyObject, %9 : $Builtin.Word
391391
%11 = address_to_pointer %10 : $*AnyObject to $Builtin.RawPointer
392-
cond_br undef, bb2, bb3(%11 : $Builtin.RawPointer)
392+
cond_br undef, bb2, bb3
393393

394394
bb2:
395395
br bb1(%11 : $Builtin.RawPointer)
396396

397-
bb3(%14 : $Builtin.RawPointer):
398-
%15 = struct $UnsafeMutablePointer<AnyObject> (%14 : $Builtin.RawPointer)
397+
bb3:
398+
%15 = struct $UnsafeMutablePointer<AnyObject> (%11 : $Builtin.RawPointer)
399399
return %15 : $UnsafeMutablePointer<AnyObject>
400400
}
401401

‎test/SILOptimizer/accessed_storage_ossa.sil‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -401,14 +401,14 @@ bb1(%5 : $Builtin.RawPointer):
401401
%9 = integer_literal $Builtin.Word, 1
402402
%10 = index_addr %6 : $*AnyObject, %9 : $Builtin.Word
403403
%11 = address_to_pointer %10 : $*AnyObject to $Builtin.RawPointer
404-
cond_br undef, bb2, bb3(%11 : $Builtin.RawPointer)
404+
cond_br undef, bb2, bb3
405405

406406
bb2:
407407
br bb1(%11 : $Builtin.RawPointer)
408408

409-
bb3(%14 : $Builtin.RawPointer):
409+
bb3:
410410
destroy_value %3 : $AnyObject
411-
%15 = struct $UnsafeMutablePointer<AnyObject> (%14 : $Builtin.RawPointer)
411+
%15 = struct $UnsafeMutablePointer<AnyObject> (%11 : $Builtin.RawPointer)
412412
return %15 : $UnsafeMutablePointer<AnyObject>
413413
}
414414

‎test/SILOptimizer/accesspath_uses.sil‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ struct MyStruct {
3434
// CHECK-NEXT: Path: (@Unknown)
3535
// CHECK-NEXT: store %7 to %10 : $*AnyObject
3636
// CHECK-NEXT: Path: (@Unknown)
37-
// CHECK-NEXT: cond_br undef, bb2, bb3(%12 : $Builtin.RawPointer)
37+
// CHECK-NEXT: %16 = load %15 : $*AnyObject
3838
// CHECK-NEXT: Path: (@Unknown)
3939
// CHECK-NEXT: }
4040
// CHECK: ###For MemOp: %7 = load %6 : $*AnyObject
@@ -51,7 +51,7 @@ struct MyStruct {
5151
// CHECK-NEXT: Path: (@Unknown)
5252
// CHECK-NEXT: store %7 to %10 : $*AnyObject
5353
// CHECK-NEXT: Path: (@Unknown)
54-
// CHECK-NEXT: cond_br undef, bb2, bb3(%12 : $Builtin.RawPointer)
54+
// CHECK-NEXT: %16 = load %15 : $*AnyObject
5555
// CHECK-NEXT: Path: (@Unknown)
5656
// CHECK-NEXT: }
5757
// CHECK: ###For MemOp: store %7 to %6 : $*AnyObject
@@ -68,7 +68,7 @@ struct MyStruct {
6868
// CHECK-NEXT: Path: (@Unknown)
6969
// CHECK-NEXT: store %7 to %10 : $*AnyObject
7070
// CHECK-NEXT: Path: (@Unknown)
71-
// CHECK-NEXT: cond_br undef, bb2, bb3(%12 : $Builtin.RawPointer)
71+
// CHECK-NEXT: %16 = load %15 : $*AnyObject
7272
// CHECK-NEXT: Path: (@Unknown)
7373
// CHECK-NEXT: }
7474
// CHECK: ###For MemOp: store %7 to %10 : $*AnyObject
@@ -85,10 +85,10 @@ struct MyStruct {
8585
// CHECK-NEXT: Path: (@Unknown)
8686
// CHECK-NEXT: store %7 to %10 : $*AnyObject
8787
// CHECK-NEXT: Path: (@Unknown)
88-
// CHECK-NEXT: cond_br undef, bb2, bb3(%12 : $Builtin.RawPointer)
88+
// CHECK-NEXT: %16 = load %15 : $*AnyObject
8989
// CHECK-NEXT: Path: (@Unknown)
9090
// CHECK-NEXT: }
91-
// CHECK: ###For MemOp: %17 = load %16 : $*AnyObject
91+
// CHECK: ###For MemOp: %16 = load %15 : $*AnyObject
9292
// CHECK: Storage: Unidentified %1 = struct_extract %0 : $UnsafeMutablePointer<AnyObject>, #UnsafeMutablePointer._rawValue
9393
// CHECK: Path: (@Unknown)
9494
// CHECK: Exact Uses {
@@ -102,7 +102,7 @@ struct MyStruct {
102102
// CHECK-NEXT: Path: (@Unknown)
103103
// CHECK-NEXT: store %7 to %10 : $*AnyObject
104104
// CHECK-NEXT: Path: (@Unknown)
105-
// CHECK-NEXT: cond_br undef, bb2, bb3(%12 : $Builtin.RawPointer)
105+
// CHECK-NEXT: %16 = load %15 : $*AnyObject
106106
// CHECK-NEXT: Path: (@Unknown)
107107
// CHECK-NEXT: }
108108
sil shared @testIndexLoop : $@convention(thin) (UnsafeMutablePointer<AnyObject>) -> AnyObject {
@@ -120,13 +120,13 @@ bb1(%5 : $Builtin.RawPointer):
120120
%10 = index_addr %6 : $*AnyObject, %9 : $Builtin.Word
121121
store %7 to %10 : $*AnyObject
122122
%12 = address_to_pointer %10 : $*AnyObject to $Builtin.RawPointer
123-
cond_br undef, bb2, bb3(%12 : $Builtin.RawPointer)
123+
cond_br undef, bb2, bb3
124124

125125
bb2:
126126
br bb1(%12 : $Builtin.RawPointer)
127127

128-
bb3(%15 : $Builtin.RawPointer):
129-
%16 = pointer_to_address %15 : $Builtin.RawPointer to [strict] $*AnyObject
128+
bb3:
129+
%16 = pointer_to_address %12 : $Builtin.RawPointer to [strict] $*AnyObject
130130
%17 = load %16 : $*AnyObject
131131
return %17 : $AnyObject
132132
}

‎test/SILOptimizer/accesspath_uses_ossa.sil‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ bb0(%0 : $Builtin.RawPointer):
305305
// CHECK-NEXT: Path: (@Unknown)
306306
// CHECK-NEXT: store %8 to [assign] %11 : $*AnyObject
307307
// CHECK-NEXT: Path: (@Unknown)
308-
// CHECK-NEXT: cond_br undef, bb2, bb3(%13 : $Builtin.RawPointer)
308+
// CHECK-NEXT: %17 = load [copy] %16 : $*AnyObject
309309
// CHECK-NEXT: Path: (@Unknown)
310310
// CHECK-NEXT: }
311311
// CHECK: ###For MemOp: %7 = load [copy] %6 : $*AnyObject
@@ -322,7 +322,7 @@ bb0(%0 : $Builtin.RawPointer):
322322
// CHECK-NEXT: Path: (@Unknown)
323323
// CHECK-NEXT: store %8 to [assign] %11 : $*AnyObject
324324
// CHECK-NEXT: Path: (@Unknown)
325-
// CHECK-NEXT: cond_br undef, bb2, bb3(%13 : $Builtin.RawPointer)
325+
// CHECK-NEXT: %17 = load [copy] %16 : $*AnyObject
326326
// CHECK-NEXT: Path: (@Unknown)
327327
// CHECK-NEXT: }
328328
// CHECK: ###For MemOp: store %7 to [assign] %6 : $*AnyObject
@@ -339,7 +339,7 @@ bb0(%0 : $Builtin.RawPointer):
339339
// CHECK-NEXT: Path: (@Unknown)
340340
// CHECK-NEXT: store %8 to [assign] %11 : $*AnyObject
341341
// CHECK-NEXT: Path: (@Unknown)
342-
// CHECK-NEXT: cond_br undef, bb2, bb3(%13 : $Builtin.RawPointer)
342+
// CHECK-NEXT: %17 = load [copy] %16 : $*AnyObject
343343
// CHECK-NEXT: Path: (@Unknown)
344344
// CHECK-NEXT: }
345345
// CHECK: ###For MemOp: store %8 to [assign] %11 : $*AnyObject
@@ -356,10 +356,10 @@ bb0(%0 : $Builtin.RawPointer):
356356
// CHECK-NEXT: Path: (@Unknown)
357357
// CHECK-NEXT: store %8 to [assign] %11 : $*AnyObject
358358
// CHECK-NEXT: Path: (@Unknown)
359-
// CHECK-NEXT: cond_br undef, bb2, bb3(%13 : $Builtin.RawPointer)
359+
// CHECK-NEXT: %17 = load [copy] %16 : $*AnyObject
360360
// CHECK-NEXT: Path: (@Unknown)
361361
// CHECK-NEXT: }
362-
// CHECK: ###For MemOp: %18 = load [copy] %17 : $*AnyObject
362+
// CHECK: ###For MemOp: %17 = load [copy] %16 : $*AnyObject
363363
// CHECK: Storage: Unidentified %1 = struct_extract %0 : $UnsafeMutablePointer<AnyObject>, #UnsafeMutablePointer._rawValue
364364
// CHECK: Path: (@Unknown)
365365
// CHECK: Exact Uses {
@@ -373,7 +373,7 @@ bb0(%0 : $Builtin.RawPointer):
373373
// CHECK-NEXT: Path: (@Unknown)
374374
// CHECK-NEXT: store %8 to [assign] %11 : $*AnyObject
375375
// CHECK-NEXT: Path: (@Unknown)
376-
// CHECK-NEXT: cond_br undef, bb2, bb3(%13 : $Builtin.RawPointer)
376+
// CHECK-NEXT: %17 = load [copy] %16 : $*AnyObject
377377
// CHECK-NEXT: Path: (@Unknown)
378378
// CHECK-NEXT: }
379379
sil shared [ossa] @testIndexLoop : $@convention(thin) (UnsafeMutablePointer<AnyObject>) -> @owned AnyObject {
@@ -392,13 +392,13 @@ bb1(%5 : $Builtin.RawPointer):
392392
%11 = index_addr %6 : $*AnyObject, %10 : $Builtin.Word
393393
store %8 to [assign] %11 : $*AnyObject
394394
%13 = address_to_pointer %11 : $*AnyObject to $Builtin.RawPointer
395-
cond_br undef, bb2, bb3(%13 : $Builtin.RawPointer)
395+
cond_br undef, bb2, bb3
396396

397397
bb2:
398398
br bb1(%13 : $Builtin.RawPointer)
399399

400-
bb3(%16 : $Builtin.RawPointer):
401-
%17 = pointer_to_address %16 : $Builtin.RawPointer to [strict] $*AnyObject
400+
bb3:
401+
%17 = pointer_to_address %13 : $Builtin.RawPointer to [strict] $*AnyObject
402402
%18 = load [copy] %17 : $*AnyObject
403403
destroy_value %3 : $AnyObject
404404
return %18 : $AnyObject

‎test/SILOptimizer/dead_code_elimination_ossa.sil‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,31 +110,31 @@ bb4:
110110
%44 = struct_extract %43 : $Bool, #Bool._value
111111
// CHECK-NOT: cond_br
112112
// CHECK: br bb5
113-
cond_br %44, bb5, bb6(%28 : $Int32)
113+
cond_br %44, bb5, bb6
114114

115115
bb5:
116116
%46 = integer_literal $Builtin.Int32, 0
117117
%47 = struct $Int32 (%46 : $Builtin.Int32)
118118
br bb7(%47 : $Int32)
119119

120-
bb6(%28a : $Int32):
121-
br bb7(%28a : $Int32)
120+
bb6:
121+
br bb7(%28 : $Int32)
122122

123123
bb7(%49 : $Int32):
124124
%50 = integer_literal $Builtin.Int32, 10
125125
%52 = struct_extract %29 : $Int32, #Int32._value
126126
%53 = builtin "cmp_eq_Int32"(%50 : $Builtin.Int32, %52 : $Builtin.Int32) : $Builtin.Int1
127127
%54 = struct $Bool (%53 : $Builtin.Int1)
128128
%55 = struct_extract %54 : $Bool, #Bool._value
129-
cond_br %55, bb8, bb9(%29 : $Int32)
129+
cond_br %55, bb8, bb9
130130

131131
bb8:
132132
%57 = integer_literal $Builtin.Int32, 0
133133
%58 = struct $Int32 (%57 : $Builtin.Int32)
134134
br bb10(%58 : $Int32)
135135

136-
bb9(%29a : $Int32):
137-
br bb10(%29a : $Int32)
136+
bb9:
137+
br bb10(%29 : $Int32)
138138

139139
// CHECK: bb5([[BBARG:%.*]]):
140140
bb10(%60 : $Int32):

‎test/SILOptimizer/loop_unroll_ossa.sil‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -299,12 +299,15 @@ bb4(%58 : $Builtin.Int64, %59 : $Builtin.Int64):
299299
%64 = builtin "smul_with_overflow_Int64"(%59 : $Builtin.Int64, %28 : $Builtin.Int64, %56 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
300300
%65 = tuple_extract %64 : $(Builtin.Int64, Builtin.Int1), 0
301301
%70 = builtin "cmp_slt_Int64"(%61 : $Builtin.Int64, %28 : $Builtin.Int64) : $Builtin.Int1
302-
cond_br %70, bb5, bb6(%61 : $Builtin.Int64)
302+
cond_br %70, bb5, bb6
303303

304304
bb5:
305305
br bb4(%61 : $Builtin.Int64, %65 : $Builtin.Int64)
306306

307-
bb6(%72 : $Builtin.Int64):
307+
bb6:
308+
br bb7(%61 : $Builtin.Int64)
309+
310+
bb7(%72 : $Builtin.Int64):
308311
%401 = tuple ()
309312
return %401 : $()
310313
}
@@ -337,12 +340,15 @@ bb4(%58 : $Builtin.Int64, %59 : $Builtin.Int64):
337340
%64 = builtin "smul_with_overflow_Int64"(%59 : $Builtin.Int64, %28 : $Builtin.Int64, %56 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
338341
%65 = tuple_extract %64 : $(Builtin.Int64, Builtin.Int1), 0
339342
%70 = builtin "cmp_slt_Int64"(%61 : $Builtin.Int64, %28 : $Builtin.Int64) : $Builtin.Int1
340-
cond_br %70, bb5, bb6(%61 : $Builtin.Int64)
343+
cond_br %70, bb5, bb6
341344

342345
bb5:
343346
br bb4(%61 : $Builtin.Int64, %65 : $Builtin.Int64)
344347

345-
bb6(%72 : $Builtin.Int64):
348+
bb6:
349+
br bb7(%61 : $Builtin.Int64)
350+
351+
bb7(%72 : $Builtin.Int64):
346352
%401 = tuple ()
347353
return %401 : $()
348354
}

‎test/SILOptimizer/looprotate_ossa.sil‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ bb0(%0 : $Int32):
178178
bb1(%4 : $Builtin.Int32, %5 : $Builtin.Int32):
179179
%6 = struct $Int32 (%5 : $Builtin.Int32)
180180
%8 = builtin "cmp_eq_Word"(%5 : $Builtin.Int32, %1 : $Builtin.Int32) : $Builtin.Int1
181-
cond_br %8, bb3(%4 : $Builtin.Int32), bb2
181+
cond_br %8, bb3, bb2
182182

183183
bb2:
184184
%10 = integer_literal $Builtin.Int32, 1
@@ -193,8 +193,8 @@ bb2:
193193
%21 = tuple_extract %20 : $(Builtin.Int32, Builtin.Int1), 0
194194
br bb1(%21 : $Builtin.Int32, %14 : $Builtin.Int32)
195195

196-
bb3 (%23 : $Builtin.Int32) :
197-
%24 = struct $Int32 (%23 : $Builtin.Int32)
196+
bb3:
197+
%24 = struct $Int32 (%4 : $Builtin.Int32)
198198
return %24 : $Int32
199199
}
200200

‎test/SILOptimizer/redundant_load_elim_ossa.sil‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,13 +1129,13 @@ bb2(%20 : $Builtin.Int32):
11291129
%39 = load [trivial] %12 : $*Builtin.Int32
11301130
%40 = load [trivial] %13 : $*Builtin.Int32
11311131
%41 = builtin "cmp_eq_Int32"(%39 : $Builtin.Int32, %40 : $Builtin.Int32) : $Builtin.Int1
1132-
cond_br %41, bb2a, bb2b(%39 : $Builtin.Int32)
1132+
cond_br %41, bb2a, bb2b
11331133

11341134
bb2a:
11351135
br bb3
11361136

1137-
bb2b(%39a : $Builtin.Int32):
1138-
br bb2(%39a : $Builtin.Int32)
1137+
bb2b:
1138+
br bb2(%39 : $Builtin.Int32)
11391139

11401140
// bb1 is after bb2 to make sure the first predecessor of bb2 is not bb2 to
11411141
// expose the bug.

0 commit comments

Comments
 (0)