Skip to content

Commit

Permalink
[LoopIdiom] Fix bailout for aliasing in memcpy transform.
Browse files Browse the repository at this point in the history
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

(cherry picked from commit abdf0da)
  • Loading branch information
efriedma-quic authored and tstellar committed Jun 8, 2022
1 parent 2e857fe commit b75bf75
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 17 deletions.
27 changes: 10 additions & 17 deletions llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
Expand Up @@ -1420,26 +1420,19 @@ bool LoopIdiomRecognize::processLoopStoreOfLoopLoad(

// If the store is a memcpy instruction, we must check if it will write to
// the load memory locations. So remove it from the ignored stores.
if (IsMemCpy)
IgnoredInsts.erase(TheStore);
MemmoveVerifier Verifier(*LoadBasePtr, *StoreBasePtr, *DL);
if (IsMemCpy && !Verifier.IsSameObject)
IgnoredInsts.erase(TheStore);
if (mayLoopAccessLocation(LoadBasePtr, ModRefInfo::Mod, CurLoop, BECount,
StoreSizeSCEV, *AA, IgnoredInsts)) {
if (!IsMemCpy) {
ORE.emit([&]() {
return OptimizationRemarkMissed(DEBUG_TYPE, "LoopMayAccessLoad",
TheLoad)
<< ore::NV("Inst", InstRemark) << " in "
<< ore::NV("Function", TheStore->getFunction())
<< " function will not be hoisted: "
<< ore::NV("Reason", "The loop may access load location");
});
return Changed;
}
// At this point loop may access load only for memcpy in same underlying
// object. If that's not the case bail out.
if (!Verifier.IsSameObject)
return Changed;
ORE.emit([&]() {
return OptimizationRemarkMissed(DEBUG_TYPE, "LoopMayAccessLoad", TheLoad)
<< ore::NV("Inst", InstRemark) << " in "
<< ore::NV("Function", TheStore->getFunction())
<< " function will not be hoisted: "
<< ore::NV("Reason", "The loop may access load location");
});
return Changed;
}

bool UseMemMove = IsMemCpy ? Verifier.IsSameObject : LoopAccessStore;
Expand Down
43 changes: 43 additions & 0 deletions llvm/test/Transforms/LoopIdiom/basic.ll
Expand Up @@ -1530,6 +1530,49 @@ for.body: ; preds = %entry, %for.body
br i1 %cmp, label %for.body, label %for.cond.cleanup
}

; Do not form memmove when there's an aliasing operation, even
; if the memcpy source and destination are in the same object.
define void @do_not_form_memmove8(i64* %p) {
; CHECK-LABEL: @do_not_form_memmove8(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[P2:%.*]] = getelementptr inbounds i64, i64* [[P:%.*]], i64 1000
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: exit:
; CHECK-NEXT: ret void
; CHECK: loop:
; CHECK-NEXT: [[X4:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[X13:%.*]], [[LOOP]] ]
; CHECK-NEXT: [[X5:%.*]] = zext i32 [[X4]] to i64
; CHECK-NEXT: [[X7:%.*]] = getelementptr inbounds i64, i64* [[P2]], i64 [[X5]]
; CHECK-NEXT: [[X8:%.*]] = bitcast i64* [[X7]] to i8*
; CHECK-NEXT: store i64 1, i64* [[X7]], align 4
; CHECK-NEXT: [[X11:%.*]] = getelementptr inbounds i64, i64* [[P]], i64 [[X5]]
; CHECK-NEXT: [[X12:%.*]] = bitcast i64* [[X11]] to i8*
; CHECK-NEXT: tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* [[X12]], i8* [[X8]], i64 8, i1 false)
; CHECK-NEXT: [[X13]] = add i32 [[X4]], 1
; CHECK-NEXT: [[X14:%.*]] = icmp eq i32 [[X13]], 44
; CHECK-NEXT: br i1 [[X14]], label [[EXIT:%.*]], label [[LOOP]]
;
entry:
%p2 = getelementptr inbounds i64, i64* %p, i64 1000
br label %loop

exit:
ret void

loop:
%x4 = phi i32 [ 0, %entry ], [ %x13, %loop ]
%x5 = zext i32 %x4 to i64
%x7 = getelementptr inbounds i64, i64* %p2, i64 %x5
%x8 = bitcast i64* %x7 to i8*
store i64 1, i64* %x7, align 4
%x11 = getelementptr inbounds i64, i64* %p, i64 %x5
%x12 = bitcast i64* %x11 to i8*
tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %x12, i8* %x8, i64 8, i1 false)
%x13 = add i32 %x4, 1
%x14 = icmp eq i32 %x13, 44
br i1 %x14, label %exit, label %loop
}

;; Memcpy formation is still preferred over memmove.
define void @prefer_memcpy_over_memmove(i8* noalias %Src, i8* noalias %Dest, i64 %Size) {
; CHECK-LABEL: @prefer_memcpy_over_memmove(
Expand Down

0 comments on commit b75bf75

Please sign in to comment.