Skip to content

Commit eb60156

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) (cherry picked from commit c407917)
1 parent 88c0a9e commit eb60156

File tree

2 files changed

+79
-2
lines changed

2 files changed

+79
-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: 75 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,71 @@ 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+
// CHECK-LABEL: sil @testTailProjection : $@convention(thin) () -> () {
1427+
// CHECK: bb0:
1428+
// CHECK: [[A:%.*]] = index_addr %4 : $*UInt64Wrapper, %1 : $Builtin.Word
1429+
// CHECK: store %{{.*}} to [[A]] : $*UInt64Wrapper
1430+
// CHECK: load %5 : $*UInt64Wrapper
1431+
// CHECK: br bb1
1432+
// CHECK: bb1(%{{.*}} : $Builtin.Int64, %{{.*}} : $UInt64Wrapper, [[PHI:%.*]] : $UInt64Wrapper):
1433+
// CHECK: cond_br undef, bb3, bb2
1434+
// CHECK: bb2:
1435+
// CHECK-NOT: (load|store)
1436+
// CHECK: struct_extract [[PHI]] : $UInt64Wrapper, #UInt64Wrapper.rawValue
1437+
// CHECK: struct_extract
1438+
// CHECK: struct $UInt64
1439+
// CHECK: struct $UInt64Wrapper
1440+
// CHECK-NOT: (load|store)
1441+
// CHECK: br bb1
1442+
// CHECK: bb3:
1443+
// CHECK: store [[PHI]] to [[A]] : $*UInt64Wrapper
1444+
// CHECK-LABEL: } // end sil function 'testTailProjection'
1445+
sil @testTailProjection : $@convention(thin) () -> () {
1446+
bb0:
1447+
%0 = integer_literal $Builtin.Int64, 0
1448+
%1 = integer_literal $Builtin.Word, 1
1449+
%2 = integer_literal $Builtin.Word, 2
1450+
%3 = alloc_ref [tail_elems $UInt64Wrapper * %2 : $Builtin.Word] $Storage
1451+
%4 = ref_tail_addr %3 : $Storage, $UInt64Wrapper
1452+
%5 = index_addr %4 : $*UInt64Wrapper, %1 : $Builtin.Word
1453+
%6 = struct $UInt64 (%0 : $Builtin.Int64)
1454+
%7 = struct $UInt64Wrapper (%6 : $UInt64)
1455+
store %7 to %5 : $*UInt64Wrapper
1456+
%9 = load %5 : $*UInt64Wrapper
1457+
br bb1(%0 : $Builtin.Int64, %9 : $UInt64Wrapper)
1458+
1459+
bb1(%11 : $Builtin.Int64, %12 : $UInt64Wrapper):
1460+
cond_br undef, bb3, bb2
1461+
1462+
bb2:
1463+
%14 = struct_element_addr %5 : $*UInt64Wrapper, #UInt64Wrapper.rawValue
1464+
%15 = struct_element_addr %14 : $*UInt64, #UInt64._value
1465+
%16 = load %15 : $*Builtin.Int64
1466+
%17 = struct $UInt64 (%16 : $Builtin.Int64)
1467+
%18 = struct $UInt64Wrapper (%17 : $UInt64)
1468+
store %18 to %5 : $*UInt64Wrapper
1469+
br bb1(%16 : $Builtin.Int64, %18 : $UInt64Wrapper)
1470+
1471+
bb3:
1472+
%21 = tuple ()
1473+
return %21 : $()
1474+
}

0 commit comments

Comments
 (0)