Skip to content

Commit c407917

Browse files
committed
Trivial fix for an LICM assertion in projectLoadValue
This seems to compile correctly in release builds. But it does go through an llvm_unreachable path, so really isn't safe to leave unfixed. When the accessPath has an offset, propagate it through the recursive calls. This may have happened when the offset was moved outside of the sequence of path indices. The code rebuilds a path from the indices without adding back the offset. LICM asserts during projectLoadValue when it needs to rematerialize a loaded value within the loop using projections and the loop-invariant address is an index_addr. Basically: %a = index_addr %4 : $*Wrapper, %1 : $Builtin.Word store %_ to %a : $*Wrapper br loop: loop: %f = struct_element_addr %a %v = load %f : $Value %s = struct $Wrapper (%v : $Value) store %s to %a : $*Wrapper Where the store inside the loop is deleted. And where the load is hoisted out of the loop, but now loads $Wrapper instead of $Value. Fixes rdar://92191909 (LICM assertion: isSubObjectProjection(), MemAccessUtils.h, line 1069)
1 parent 2eac3db commit c407917

File tree

2 files changed

+60
-2
lines changed

2 files changed

+60
-2
lines changed

lib/SILOptimizer/LoopTransforms/LICM.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,7 +1173,8 @@ static SILValue projectLoadValue(SILValue addr, AccessPath accessPath,
11731173
assert(ProjectionIndex(SEI).Index == elementIdx);
11741174
SILValue val = projectLoadValue(
11751175
SEI->getOperand(),
1176-
AccessPath(accessPath.getStorage(), pathNode.getParent(), 0),
1176+
AccessPath(accessPath.getStorage(), pathNode.getParent(),
1177+
accessPath.getOffset()),
11771178
rootVal, rootAccessPath, beforeInst);
11781179
SILBuilder B(beforeInst);
11791180
return B.createStructExtract(beforeInst->getLoc(), val, SEI->getField(),
@@ -1183,7 +1184,8 @@ static SILValue projectLoadValue(SILValue addr, AccessPath accessPath,
11831184
assert(ProjectionIndex(TEI).Index == elementIdx);
11841185
SILValue val = projectLoadValue(
11851186
TEI->getOperand(),
1186-
AccessPath(accessPath.getStorage(), pathNode.getParent(), 0),
1187+
AccessPath(accessPath.getStorage(), pathNode.getParent(),
1188+
accessPath.getOffset()),
11871189
rootVal, rootAccessPath, beforeInst);
11881190
SILBuilder B(beforeInst);
11891191
return B.createTupleExtract(beforeInst->getLoc(), val, TEI->getFieldIndex(),

test/SILOptimizer/licm.sil

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ sil_stage canonical
88
import Builtin
99
import Swift
1010

11+
class Storage {
12+
init()
13+
}
14+
15+
// globalArray
16+
sil_global @globalArray : $Storage
17+
1118
// CHECK-LABEL: @memset
1219

1320
// CHECK: bb0
@@ -1397,3 +1404,52 @@ bb6:
13971404
return %15 : $()
13981405
}
13991406

1407+
struct UInt64 {
1408+
@_hasStorage var _value: Builtin.Int64 { get set }
1409+
init(_value: Builtin.Int64)
1410+
}
1411+
1412+
public struct UInt64Wrapper {
1413+
@_hasStorage public var rawValue: UInt64 { get set }
1414+
private init(_ rawValue: UInt64)
1415+
public init()
1416+
}
1417+
1418+
// rdar://92191909 (LICM assertion: isSubObjectProjection(), MemAccessUtils.h, line 1069)
1419+
//
1420+
// projectLoadValue needs to rematerialize a loaded value within the
1421+
// loop using projections and the loop-invariant address is an
1422+
// index_addr.
1423+
//
1424+
// The store inside the loop is deleted, and the load is hoisted such
1425+
// that it now loads the UInt64Wrapper instead of Builtin.Int64
1426+
sil @testTailProjection : $@convention(thin) () -> () {
1427+
bb0:
1428+
%0 = integer_literal $Builtin.Int64, 0
1429+
%1 = integer_literal $Builtin.Word, 1
1430+
%2 = integer_literal $Builtin.Word, 2
1431+
%3 = alloc_ref [tail_elems $UInt64Wrapper * %2 : $Builtin.Word] $Storage
1432+
%4 = ref_tail_addr %3 : $Storage, $UInt64Wrapper
1433+
%5 = index_addr %4 : $*UInt64Wrapper, %1 : $Builtin.Word
1434+
%6 = struct $UInt64 (%0 : $Builtin.Int64)
1435+
%7 = struct $UInt64Wrapper (%6 : $UInt64)
1436+
store %7 to %5 : $*UInt64Wrapper
1437+
%9 = load %5 : $*UInt64Wrapper
1438+
br bb1(%0 : $Builtin.Int64, %9 : $UInt64Wrapper)
1439+
1440+
bb1(%11 : $Builtin.Int64, %12 : $UInt64Wrapper):
1441+
cond_br undef, bb3, bb2
1442+
1443+
bb2:
1444+
%14 = struct_element_addr %5 : $*UInt64Wrapper, #UInt64Wrapper.rawValue
1445+
%15 = struct_element_addr %14 : $*UInt64, #UInt64._value
1446+
%16 = load %15 : $*Builtin.Int64
1447+
%17 = struct $UInt64 (%16 : $Builtin.Int64)
1448+
%18 = struct $UInt64Wrapper (%17 : $UInt64)
1449+
store %18 to %5 : $*UInt64Wrapper
1450+
br bb1(%16 : $Builtin.Int64, %18 : $UInt64Wrapper)
1451+
1452+
bb3:
1453+
%21 = tuple ()
1454+
return %21 : $()
1455+
}

0 commit comments

Comments
 (0)