Skip to content

Commit bc6ae3e

Browse files
committed
Fix array bounds check optimization for ossa
In non-ossa, a dominance check is sufficient to check if a value is available at a use point. In ossa this isn't sufficient since the lifetime of the value may have ended. Ensure proper checks for ossa before hoisting an array value.
1 parent 252a57a commit bc6ae3e

File tree

2 files changed

+124
-2
lines changed

2 files changed

+124
-2
lines changed

lib/SILOptimizer/Analysis/ArraySemantic.cpp

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "swift/Basic/Assertions.h"
1515
#include "swift/SIL/DebugUtils.h"
1616
#include "swift/SIL/InstructionUtils.h"
17+
#include "swift/SIL/PrunedLiveness.h"
1718
#include "swift/SIL/SILArgument.h"
1819
#include "swift/SIL/SILBuilder.h"
1920
#include "swift/SIL/SILFunction.h"
@@ -282,6 +283,26 @@ std::optional<int64_t> swift::ArraySemanticsCall::getConstantIndex() const {
282283
return Val.getSExtValue();
283284
}
284285

286+
static bool isLiveAt(SILValue value, SILInstruction *newUser, DominanceInfo *DT) {
287+
auto *func = value->getFunction();
288+
if (!func->hasOwnership()) {
289+
return DT->dominates(value->getParentBlock(), newUser->getParent());
290+
}
291+
SSAPrunedLiveness liveness(func);
292+
liveness.initializeDef(value);
293+
auto summary = liveness.computeSimple();
294+
if (summary.addressUseKind != AddressUseKind::NonEscaping) {
295+
return false;
296+
}
297+
if (summary.innerBorrowKind != InnerBorrowKind::Contained) {
298+
return false;
299+
}
300+
if (!liveness.isWithinBoundary(newUser, /*deadEndBlocks=*/nullptr)) {
301+
return false;
302+
}
303+
return true;
304+
}
305+
285306
static bool canHoistArrayArgument(ApplyInst *SemanticsCall, SILValue Arr,
286307
SILInstruction *InsertBefore,
287308
DominanceInfo *DT) {
@@ -294,10 +315,11 @@ static bool canHoistArrayArgument(ApplyInst *SemanticsCall, SILValue Arr,
294315
return false;
295316

296317
ValueBase *SelfVal = Arr;
297-
auto *SelfBB = SelfVal->getParentBlock();
298-
if (DT->dominates(SelfBB, InsertBefore->getParent()))
318+
if (isLiveAt(SelfVal, InsertBefore, DT)) {
299319
return true;
320+
}
300321

322+
// OSSAFIXME: Handle begin_borrow and load_borrow
301323
if (auto *Copy = dyn_cast<CopyValueInst>(SelfVal)) {
302324
// look through one level
303325
SelfVal = Copy->getOperand();

test/SILOptimizer/abcopts_ossa_guaranteed.sil

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,3 +1458,103 @@ bb3:
14581458
%r = tuple ()
14591459
return %r : $()
14601460
}
1461+
1462+
// The bounds check isn't hoisted here since the array value for the isNativeTypeChecked is not available in the preheader
1463+
// Ensure we don't fail verification
1464+
sil [ossa] @isNativeUnavailable1 : $@convention(thin) (Int32, @owned ArrayInt) -> Int32 {
1465+
bb0(%0 : $Int32, %1 : @owned $ArrayInt):
1466+
%2 = integer_literal $Builtin.Int1, -1
1467+
%3 = struct $Bool (%2)
1468+
%4 = struct_extract %0, #Int32._value
1469+
%5 = integer_literal $Builtin.Int32, 0
1470+
%6 = builtin "cmp_eq_Int32"(%5, %4) : $Builtin.Int1
1471+
%7 = begin_borrow %1
1472+
%8 = copy_value %7
1473+
%9 = function_ref @arrayPropertyIsNative : $@convention(method) (@guaranteed ArrayInt) -> Bool
1474+
%10 = apply %9(%7) : $@convention(method) (@guaranteed ArrayInt) -> Bool
1475+
end_borrow %7
1476+
cond_br %6, bb2, bb1
1477+
1478+
bb1:
1479+
br bb3(%5)
1480+
1481+
bb2:
1482+
destroy_value %8
1483+
destroy_value %1
1484+
br bb6(%5)
1485+
1486+
bb3(%17 : $Builtin.Int32):
1487+
%18 = struct $Int32 (%17)
1488+
%19 = function_ref @checkbounds : $@convention(method) (Int32, Bool, @guaranteed ArrayInt) -> _DependenceToken
1489+
%20 = apply %19(%18, %10, %8) : $@convention(method) (Int32, Bool, @guaranteed ArrayInt) -> _DependenceToken
1490+
%21 = integer_literal $Builtin.Int32, 1
1491+
%22 = integer_literal $Builtin.Int1, -1
1492+
%23 = builtin "sadd_with_overflow_Int32"(%17, %21, %22) : $(Builtin.Int32, Builtin.Int1)
1493+
%24 = tuple_extract %23, 0
1494+
%25 = tuple_extract %23, 1
1495+
cond_fail %25, ""
1496+
%27 = builtin "cmp_eq_Int32"(%24, %4) : $Builtin.Int1
1497+
cond_br %27, bb5, bb4
1498+
1499+
bb4:
1500+
br bb3(%24)
1501+
1502+
bb5:
1503+
destroy_value %8
1504+
destroy_value %1
1505+
br bb6(%24)
1506+
1507+
bb6(%33 : $Builtin.Int32):
1508+
%34 = struct $Int32 (%33)
1509+
return %34
1510+
}
1511+
1512+
// The bounds check isn't hoisted here since the array value for the isNativeTypeChecked is not available in the preheader
1513+
// Ensure we don't fail verification
1514+
sil [ossa] @isNativeUnavailable2 : $@convention(thin) (Int32, @owned ArrayInt) -> Int32 {
1515+
bb0(%0 : $Int32, %1 : @owned $ArrayInt):
1516+
%2 = integer_literal $Builtin.Int1, -1
1517+
%3 = struct $Bool (%2)
1518+
%4 = struct_extract %0, #Int32._value
1519+
%5 = integer_literal $Builtin.Int32, 0
1520+
%6 = builtin "cmp_eq_Int32"(%5, %4) : $Builtin.Int1
1521+
%7 = copy_value %1
1522+
%8 = copy_value %7
1523+
%9 = function_ref @arrayPropertyIsNative : $@convention(method) (@guaranteed ArrayInt) -> Bool
1524+
%10 = apply %9(%7) : $@convention(method) (@guaranteed ArrayInt) -> Bool
1525+
destroy_value %7
1526+
cond_br %6, bb2, bb1
1527+
1528+
bb1:
1529+
br bb3(%5)
1530+
1531+
bb2:
1532+
destroy_value %8
1533+
destroy_value %1
1534+
br bb6(%5)
1535+
1536+
bb3(%17 : $Builtin.Int32):
1537+
%18 = struct $Int32 (%17)
1538+
%19 = function_ref @checkbounds : $@convention(method) (Int32, Bool, @guaranteed ArrayInt) -> _DependenceToken
1539+
%20 = apply %19(%18, %10, %8) : $@convention(method) (Int32, Bool, @guaranteed ArrayInt) -> _DependenceToken
1540+
%21 = integer_literal $Builtin.Int32, 1
1541+
%22 = integer_literal $Builtin.Int1, -1
1542+
%23 = builtin "sadd_with_overflow_Int32"(%17, %21, %22) : $(Builtin.Int32, Builtin.Int1)
1543+
%24 = tuple_extract %23, 0
1544+
%25 = tuple_extract %23, 1
1545+
cond_fail %25, ""
1546+
%27 = builtin "cmp_eq_Int32"(%24, %4) : $Builtin.Int1
1547+
cond_br %27, bb5, bb4
1548+
1549+
bb4:
1550+
br bb3(%24)
1551+
1552+
bb5:
1553+
destroy_value %8
1554+
destroy_value %1
1555+
br bb6(%24)
1556+
1557+
bb6(%33 : $Builtin.Int32):
1558+
%34 = struct $Int32 (%33)
1559+
return %34
1560+
}

0 commit comments

Comments
 (0)