Skip to content

Commit abdf0da

Browse files
committed
[LoopIdiom] Fix bailout for aliasing in memcpy transform.
Commit dd5991c modified the aliasing checks here to allow transforming a memcpy where the source and destination point into the same object. However, the change accidentally made the code skip the alias check for other operations in the loop. Instead of completely skipping the alias check, just skip the check for whether the memcpy aliases itself. Differential Revision: https://reviews.llvm.org/D126486
1 parent d92f7f7 commit abdf0da

File tree

2 files changed

+53
-17
lines changed

2 files changed

+53
-17
lines changed

llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,26 +1422,19 @@ bool LoopIdiomRecognize::processLoopStoreOfLoopLoad(
14221422

14231423
// If the store is a memcpy instruction, we must check if it will write to
14241424
// the load memory locations. So remove it from the ignored stores.
1425-
if (IsMemCpy)
1426-
IgnoredInsts.erase(TheStore);
14271425
MemmoveVerifier Verifier(*LoadBasePtr, *StoreBasePtr, *DL);
1426+
if (IsMemCpy && !Verifier.IsSameObject)
1427+
IgnoredInsts.erase(TheStore);
14281428
if (mayLoopAccessLocation(LoadBasePtr, ModRefInfo::Mod, CurLoop, BECount,
14291429
StoreSizeSCEV, *AA, IgnoredInsts)) {
1430-
if (!IsMemCpy) {
1431-
ORE.emit([&]() {
1432-
return OptimizationRemarkMissed(DEBUG_TYPE, "LoopMayAccessLoad",
1433-
TheLoad)
1434-
<< ore::NV("Inst", InstRemark) << " in "
1435-
<< ore::NV("Function", TheStore->getFunction())
1436-
<< " function will not be hoisted: "
1437-
<< ore::NV("Reason", "The loop may access load location");
1438-
});
1439-
return Changed;
1440-
}
1441-
// At this point loop may access load only for memcpy in same underlying
1442-
// object. If that's not the case bail out.
1443-
if (!Verifier.IsSameObject)
1444-
return Changed;
1430+
ORE.emit([&]() {
1431+
return OptimizationRemarkMissed(DEBUG_TYPE, "LoopMayAccessLoad", TheLoad)
1432+
<< ore::NV("Inst", InstRemark) << " in "
1433+
<< ore::NV("Function", TheStore->getFunction())
1434+
<< " function will not be hoisted: "
1435+
<< ore::NV("Reason", "The loop may access load location");
1436+
});
1437+
return Changed;
14451438
}
14461439

14471440
bool UseMemMove = IsMemCpy ? Verifier.IsSameObject : LoopAccessStore;

llvm/test/Transforms/LoopIdiom/basic.ll

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,6 +1536,49 @@ for.body: ; preds = %entry, %for.body
15361536
br i1 %cmp, label %for.body, label %for.cond.cleanup
15371537
}
15381538

1539+
; Do not form memmove when there's an aliasing operation, even
1540+
; if the memcpy source and destination are in the same object.
1541+
define void @do_not_form_memmove8(i64* %p) {
1542+
; CHECK-LABEL: @do_not_form_memmove8(
1543+
; CHECK-NEXT: entry:
1544+
; CHECK-NEXT: [[P2:%.*]] = getelementptr inbounds i64, i64* [[P:%.*]], i64 1000
1545+
; CHECK-NEXT: br label [[LOOP:%.*]]
1546+
; CHECK: exit:
1547+
; CHECK-NEXT: ret void
1548+
; CHECK: loop:
1549+
; CHECK-NEXT: [[X4:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[X13:%.*]], [[LOOP]] ]
1550+
; CHECK-NEXT: [[X5:%.*]] = zext i32 [[X4]] to i64
1551+
; CHECK-NEXT: [[X7:%.*]] = getelementptr inbounds i64, i64* [[P2]], i64 [[X5]]
1552+
; CHECK-NEXT: [[X8:%.*]] = bitcast i64* [[X7]] to i8*
1553+
; CHECK-NEXT: store i64 1, i64* [[X7]], align 4
1554+
; CHECK-NEXT: [[X11:%.*]] = getelementptr inbounds i64, i64* [[P]], i64 [[X5]]
1555+
; CHECK-NEXT: [[X12:%.*]] = bitcast i64* [[X11]] to i8*
1556+
; CHECK-NEXT: tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* [[X12]], i8* [[X8]], i64 8, i1 false)
1557+
; CHECK-NEXT: [[X13]] = add i32 [[X4]], 1
1558+
; CHECK-NEXT: [[X14:%.*]] = icmp eq i32 [[X13]], 44
1559+
; CHECK-NEXT: br i1 [[X14]], label [[EXIT:%.*]], label [[LOOP]]
1560+
;
1561+
entry:
1562+
%p2 = getelementptr inbounds i64, i64* %p, i64 1000
1563+
br label %loop
1564+
1565+
exit:
1566+
ret void
1567+
1568+
loop:
1569+
%x4 = phi i32 [ 0, %entry ], [ %x13, %loop ]
1570+
%x5 = zext i32 %x4 to i64
1571+
%x7 = getelementptr inbounds i64, i64* %p2, i64 %x5
1572+
%x8 = bitcast i64* %x7 to i8*
1573+
store i64 1, i64* %x7, align 4
1574+
%x11 = getelementptr inbounds i64, i64* %p, i64 %x5
1575+
%x12 = bitcast i64* %x11 to i8*
1576+
tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %x12, i8* %x8, i64 8, i1 false)
1577+
%x13 = add i32 %x4, 1
1578+
%x14 = icmp eq i32 %x13, 44
1579+
br i1 %x14, label %exit, label %loop
1580+
}
1581+
15391582
;; Memcpy formation is still preferred over memmove.
15401583
define void @prefer_memcpy_over_memmove(i8* noalias %Src, i8* noalias %Dest, i64 %Size) {
15411584
; CHECK-LABEL: @prefer_memcpy_over_memmove(

0 commit comments

Comments
 (0)