Skip to content

Commit aad321c

Browse files
meg-guptameg-gupta
committed
Improve pattern matching range check in ArrayBoundsCheck optimizations
Current pattern matching misses when range checks are of the form: %cond = %end < %start cond_fail %cond, "Range requires lowerBound <= upperBound" instead of %cond = (%start <= %end) ^ 1 cond_fail %cond, "Range requires lowerBound <= upperBound"
1 parent e4566c8 commit aad321c

File tree

2 files changed

+146
-16
lines changed

2 files changed

+146
-16
lines changed

‎lib/SILOptimizer/LoopTransforms/ArrayBoundsCheckOpts.cpp‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,11 @@ static bool isSignedLessEqual(SILValue Start, SILValue End, SILBasicBlock &BB) {
357357
m_Specific(End)),
358358
m_One())))
359359
return true;
360+
// Try to match a cond_fail on "SLT End, Start".
361+
if (match(CF->getOperand(),
362+
m_ApplyInst(BuiltinValueKind::ICMP_SLT, m_Specific(End),
363+
m_Specific(Start))))
364+
return true;
360365
// Inclusive ranges will have a check on the upper value (before adding
361366
// one).
362367
if (PreInclusiveEnd) {
@@ -367,6 +372,11 @@ static bool isSignedLessEqual(SILValue Start, SILValue End, SILBasicBlock &BB) {
367372
m_Specific(PreInclusiveEnd)),
368373
m_One())))
369374
IsPreInclusiveEndLEQ = true;
375+
if (match(CF->getOperand(),
376+
m_ApplyInst(BuiltinValueKind::ICMP_SLT,
377+
m_Specific(PreInclusiveEnd),
378+
m_Specific(Start))))
379+
IsPreInclusiveEndLEQ = true;
370380
if (match(CF->getOperand(),
371381
m_ApplyInst(BuiltinValueKind::Xor,
372382
m_ApplyInst(BuiltinValueKind::ICMP_SGT,
@@ -497,10 +507,7 @@ static bool isRangeChecked(SILValue Start, SILValue End,
497507
if (!PreheaderPred)
498508
return false;
499509
auto *CondBr = dyn_cast<CondBranchInst>(PreheaderPred->getTerminator());
500-
if (!CondBr)
501-
return false;
502-
503-
if (isLessThanCheck(Start, End, CondBr, Preheader))
510+
if (CondBr && isLessThanCheck(Start, End, CondBr, Preheader))
504511
return true;
505512

506513
// Walk up the dominator tree looking for a range check ("SLE Start, End").
@@ -1197,7 +1204,6 @@ bool ABCOpt::processLoop(SILLoop *Loop) {
11971204

11981205
LLVM_DEBUG(llvm::dbgs() << "Attempting to remove redundant checks in "
11991206
<< *Loop);
1200-
LLVM_DEBUG(Header->getParent()->dump());
12011207

12021208
// Collect safe arrays. Arrays are safe if there is no function call that
12031209
// could mutate their size in the loop.
@@ -1243,8 +1249,6 @@ bool ABCOpt::processLoop(SILLoop *Loop) {
12431249
} else return Changed;
12441250
}
12451251

1246-
LLVM_DEBUG(Preheader->getParent()->dump());
1247-
12481252
// Find canonical induction variables.
12491253
InductionAnalysis IndVars(DT, *IVs, Preheader, Header, ExitingBlk, ExitBlk);
12501254
bool IVarsFound = IndVars.analyze();
@@ -1315,8 +1319,6 @@ bool ABCOpt::processLoop(SILLoop *Loop) {
13151319
}
13161320
}
13171321

1318-
LLVM_DEBUG(Preheader->getParent()->dump());
1319-
13201322
// Hoist bounds checks.
13211323
Changed |= hoistChecksInLoop(DT->getNode(Header), ABC, IndVars, Preheader,
13221324
Header, SingleExitingBlk, /*recursionDepth*/ 0);
@@ -1429,7 +1431,6 @@ bool ABCOpt::hoistChecksInLoop(DominanceInfoNode *DTNode, ABCAnalysis &ABC,
14291431
Changed = true;
14301432
}
14311433

1432-
LLVM_DEBUG(Preheader->getParent()->dump());
14331434
// Traverse the children in the dominator tree.
14341435
for (auto Child : *DTNode)
14351436
Changed |= hoistChecksInLoop(Child, ABC, IndVars, Preheader, Header,

‎test/SILOptimizer/abcopts_ossa_guaranteed.sil‎

Lines changed: 135 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ bb2:
742742
return %23 : $Int32
743743
}
744744

745-
// CHECK-LABEL: sil [ossa] @hoist_rangechecked :
745+
// CHECK-LABEL: sil [ossa] @hoist_rangechecked1 :
746746
// CHECK: bb0
747747
// CHECK: cond_br {{.*}}, bb2, bb1
748748
// CHECK: bb1:
@@ -759,8 +759,8 @@ bb2:
759759
// CHECK: br bb6
760760
// CHECK: bb6{{.*}}:
761761
// CHECK: return
762-
// CHECK-LABEL: } // end sil function 'hoist_rangechecked'
763-
sil [ossa] @hoist_rangechecked : $@convention(thin) (Int32, @inout ArrayInt) -> Int32 {
762+
// CHECK-LABEL: } // end sil function 'hoist_rangechecked1'
763+
sil [ossa] @hoist_rangechecked1 : $@convention(thin) (Int32, @inout ArrayInt) -> Int32 {
764764
bb0(%0 : $Int32, %1 : $*ArrayInt):
765765
%2 = integer_literal $Builtin.Int1, -1
766766
%3 = struct $Bool (%2 : $Builtin.Int1)
@@ -808,6 +808,70 @@ bb6(%36 : $Builtin.Int32):
808808
return %37 : $Int32
809809
}
810810

811+
// CHECK-LABEL: sil [ossa] @hoist_rangechecked2 :
812+
// CHECK: bb0
813+
// CHECK: cond_br {{.*}}, bb2, bb1
814+
// CHECK: bb1:
815+
// CHECK: [[CB:%[0-9]+]] = function_ref @checkbounds
816+
// CHECK: apply [[CB]]
817+
// CHECK: br bb3{{.*}}
818+
// CHECK: bb3{{.*}}:
819+
// CHECK-NOT: function_ref @checkbounds
820+
// CHECK-NOT: apply [[CB]]
821+
// CHECK: cond_br {{.*}}, bb5, bb4
822+
// CHECK: bb4
823+
// CHECK: br bb3
824+
// CHECK: bb5
825+
// CHECK: br bb6
826+
// CHECK: bb6{{.*}}:
827+
// CHECK: return
828+
// CHECK-LABEL: } // end sil function 'hoist_rangechecked2'
829+
sil [ossa] @hoist_rangechecked2 : $@convention(thin) (Int32, @inout ArrayInt) -> Int32 {
830+
bb0(%0 : $Int32, %1 : $*ArrayInt):
831+
%2 = integer_literal $Builtin.Int1, -1
832+
%3 = struct $Bool (%2 : $Builtin.Int1)
833+
%4 = struct_extract %0 : $Int32, #Int32._value
834+
%5 = integer_literal $Builtin.Int32, 0
835+
%6 = integer_literal $Builtin.Int1, -1
836+
%7 = builtin "cmp_slt_Int32"(%4 : $Builtin.Int32, %5 : $Builtin.Int32) : $Builtin.Int1
837+
cond_fail %7 : $Builtin.Int1, ""
838+
%10 = builtin "cmp_eq_Int32"(%5 : $Builtin.Int32, %4 : $Builtin.Int32) : $Builtin.Int1
839+
%17 = load [copy] %1 : $*ArrayInt
840+
cond_br %10, bb2, bb1
841+
842+
bb1:
843+
br bb3(%5 : $Builtin.Int32)
844+
845+
bb2:
846+
br bb6(%5 : $Builtin.Int32)
847+
848+
bb3(%14 : $Builtin.Int32):
849+
%15 = struct $Int32 (%14 : $Builtin.Int32)
850+
%16 = function_ref @checkbounds : $@convention(method) (Int32, Bool, @guaranteed ArrayInt) -> _DependenceToken
851+
%21 = apply %16(%15, %3, %17) : $@convention(method) (Int32, Bool, @guaranteed ArrayInt) -> _DependenceToken
852+
%22 = integer_literal $Builtin.Int32, 1
853+
%23 = integer_literal $Builtin.Int1, -1
854+
%24 = builtin "sadd_with_overflow_Int32"(%14 : $Builtin.Int32, %22 : $Builtin.Int32, %23 : $Builtin.Int1) : $(Builtin.Int32, Builtin.Int1)
855+
%25 = tuple_extract %24 : $(Builtin.Int32, Builtin.Int1), 0
856+
%26 = function_ref @getElementAddr : $@convention(method) (Int32, @guaranteed ArrayInt) -> UnsafeMutablePointerInt
857+
%28 = apply %26(%15, %17) : $@convention(method) (Int32, @guaranteed ArrayInt) -> UnsafeMutablePointerInt
858+
%29 = struct_extract %28 : $UnsafeMutablePointerInt, #UnsafeMutablePointerInt._rawValue
859+
%30 = pointer_to_address %29 : $Builtin.RawPointer to [strict] $*Int32
860+
store %0 to [trivial] %30 : $*Int32
861+
%32 = builtin "cmp_eq_Int32"(%25 : $Builtin.Int32, %4 : $Builtin.Int32) : $Builtin.Int1
862+
cond_br %32, bb5, bb4
863+
864+
bb4:
865+
br bb3(%25 : $Builtin.Int32)
866+
867+
bb5:
868+
br bb6(%25 : $Builtin.Int32)
869+
870+
bb6(%36 : $Builtin.Int32):
871+
destroy_value %17 : $ArrayInt
872+
%37 = struct $Int32 (%36 : $Builtin.Int32)
873+
return %37 : $Int32
874+
}
811875
// CHECK-LABEL: sil [ossa] @hoist_rangechecked_ref_tail_addr :
812876
// CHECK: bb0
813877
// CHECK: cond_br {{.*}}, bb2, bb1
@@ -1049,7 +1113,7 @@ bb6(%38 : $Builtin.Int32):
10491113
return %39 : $Int32
10501114
}
10511115

1052-
// CHECK-LABEL: sil [ossa] @hoist_inclusive_rangechecked :
1116+
// CHECK-LABEL: sil [ossa] @hoist_inclusive_rangechecked1 :
10531117
// CHECK: bb0
10541118
// CHECK: cond_br {{.*}}, bb2, bb1
10551119
// CHECK: bb1:
@@ -1065,8 +1129,8 @@ bb6(%38 : $Builtin.Int32):
10651129
// CHECK: bb5:
10661130
// CHECK: br bb6
10671131
// CHECK: bb6{{.*}}:
1068-
// CHECK-LABEL: } // end sil function 'hoist_inclusive_rangechecked'
1069-
sil [ossa] @hoist_inclusive_rangechecked : $@convention(thin) (Int32, @inout ArrayInt) -> Int32 {
1132+
// CHECK-LABEL: } // end sil function 'hoist_inclusive_rangechecked1'
1133+
sil [ossa] @hoist_inclusive_rangechecked1 : $@convention(thin) (Int32, @inout ArrayInt) -> Int32 {
10701134
bb0(%0 : $Int32, %1 : $*ArrayInt):
10711135
%2 = integer_literal $Builtin.Int1, -1
10721136
%3 = struct $Bool (%2 : $Builtin.Int1)
@@ -1116,6 +1180,71 @@ bb6(%37 : $Builtin.Int32):
11161180
return %38 : $Int32
11171181
}
11181182

1183+
// CHECK-LABEL: sil [ossa] @hoist_inclusive_rangechecked2 :
1184+
// CHECK: bb0
1185+
// CHECK: cond_br {{.*}}, bb2, bb1
1186+
// CHECK: bb1:
1187+
// CHECK: [[CB:%[0-9]+]] = function_ref @checkbounds
1188+
// CHECK: apply [[CB]]
1189+
// CHECK: br bb3{{.*}}
1190+
// CHECK: bb3{{.*}}:
1191+
// CHECK-NOT: function_ref @checkbounds
1192+
// CHECK-NOT: apply [[CB]]
1193+
// CHECK: cond_br {{.*}}, bb5, bb4
1194+
// CHECK: bb4:
1195+
// CHECK: br bb3
1196+
// CHECK: bb5:
1197+
// CHECK: br bb6
1198+
// CHECK: bb6{{.*}}:
1199+
// CHECK-LABEL: } // end sil function 'hoist_inclusive_rangechecked2'
1200+
sil [ossa] @hoist_inclusive_rangechecked2 : $@convention(thin) (Int32, @inout ArrayInt) -> Int32 {
1201+
bb0(%0 : $Int32, %1 : $*ArrayInt):
1202+
%2 = integer_literal $Builtin.Int1, -1
1203+
%3 = struct $Bool (%2 : $Builtin.Int1)
1204+
%4 = struct_extract %0 : $Int32, #Int32._value
1205+
%5 = integer_literal $Builtin.Int32, 0
1206+
%6 = integer_literal $Builtin.Int1, -1
1207+
%7 = builtin "cmp_slt_Int32"(%4 : $Builtin.Int32, %5 : $Builtin.Int32) : $Builtin.Int1
1208+
cond_fail %7 : $Builtin.Int1, ""
1209+
%10 = integer_literal $Builtin.Int32, 1
1210+
%11 = integer_literal $Builtin.Int1, 0
1211+
%12 = builtin "sadd_with_overflow_Int32"(%4 : $Builtin.Int32, %10 : $Builtin.Int32, %11 : $Builtin.Int1) : $(Builtin.Int32, Builtin.Int1)
1212+
%13 = tuple_extract %12 : $(Builtin.Int32, Builtin.Int1), 0
1213+
%14 = builtin "cmp_sgt_Int32"(%13 : $Builtin.Int32, %4 : $Builtin.Int32) : $Builtin.Int1
1214+
%15 = builtin "xor_Int1"(%14 : $Builtin.Int1, %6 : $Builtin.Int1) : $Builtin.Int1
1215+
cond_fail %15 : $Builtin.Int1, ""
1216+
%17 = builtin "cmp_eq_Int32"(%5 : $Builtin.Int32, %13 : $Builtin.Int32) : $Builtin.Int1
1217+
%24 = load [copy] %1 : $*ArrayInt
1218+
cond_br %17, bb2, bb1
1219+
1220+
bb1:
1221+
br bb3(%5 : $Builtin.Int32)
1222+
1223+
bb2:
1224+
br bb6(%5 : $Builtin.Int32)
1225+
1226+
bb3(%21 : $Builtin.Int32):
1227+
%22 = struct $Int32 (%21 : $Builtin.Int32)
1228+
%23 = function_ref @checkbounds : $@convention(method) (Int32, Bool, @guaranteed ArrayInt) -> _DependenceToken
1229+
%28 = apply %23(%22, %3, %24) : $@convention(method) (Int32, Bool, @guaranteed ArrayInt) -> _DependenceToken
1230+
%29 = integer_literal $Builtin.Int32, 1
1231+
%30 = integer_literal $Builtin.Int1, -1
1232+
%31 = builtin "sadd_with_overflow_Int32"(%21 : $Builtin.Int32, %29 : $Builtin.Int32, %30 : $Builtin.Int1) : $(Builtin.Int32, Builtin.Int1)
1233+
%32 = tuple_extract %31 : $(Builtin.Int32, Builtin.Int1), 0
1234+
%33 = builtin "cmp_eq_Int32"(%32 : $Builtin.Int32, %13 : $Builtin.Int32) : $Builtin.Int1
1235+
cond_br %33, bb5, bb4
1236+
1237+
bb4:
1238+
br bb3(%32 : $Builtin.Int32)
1239+
1240+
bb5:
1241+
br bb6(%32 : $Builtin.Int32)
1242+
1243+
bb6(%37 : $Builtin.Int32):
1244+
destroy_value %24 : $ArrayInt
1245+
%38 = struct $Int32 (%37 : $Builtin.Int32)
1246+
return %38 : $Int32
1247+
}
11191248
// Don't hoist arrays that are variant.
11201249
// CHECK-LABEL: sil [ossa] @dont_hoist_variant_array :
11211250
// CHECK: bb0

0 commit comments

Comments
 (0)