Skip to content

Commit baf031a

Browse files
authored
[MemCpyOpt] fix miscompile for non-dominated use of src alloca for stack-move optimization (llvm#66618)
Stack-move optimization, the optimization that merges src and dest alloca of the full-size copy, replaces all uses of the dest alloca with src alloca. For safety, we needed to check all uses of the dest alloca locations are dominated by src alloca, to be replaced. This PR adds the check for that. Fixes llvm#65225
1 parent 1a8b36b commit baf031a

File tree

2 files changed

+36
-2
lines changed

2 files changed

+36
-2
lines changed

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1486,6 +1486,15 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store,
14861486
Instruction *I = Worklist.back();
14871487
Worklist.pop_back();
14881488
for (const Use &U : I->uses()) {
1489+
auto *UI = cast<Instruction>(U.getUser());
1490+
// TODO: We can perform the transformation if we move src alloca to
1491+
// before the dominator of all uses. If any use that isn't dominated by
1492+
// SrcAlloca exists, non-dominating uses will be produced.
1493+
if (!DT->dominates(SrcAlloca, UI)) {
1494+
LLVM_DEBUG(dbgs() << "Stack Move: SrcAlloca doesn't dominate all "
1495+
"uses for the location, bailing\n");
1496+
return false;
1497+
}
14891498
if (Visited.size() >= MaxUsesToExplore) {
14901499
LLVM_DEBUG(
14911500
dbgs()
@@ -1499,10 +1508,9 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store,
14991508
return false;
15001509
case UseCaptureKind::PASSTHROUGH:
15011510
// Instructions cannot have non-instruction users.
1502-
Worklist.push_back(cast<Instruction>(U.getUser()));
1511+
Worklist.push_back(UI);
15031512
continue;
15041513
case UseCaptureKind::NO_CAPTURE: {
1505-
auto *UI = cast<Instruction>(U.getUser());
15061514
if (UI->isLifetimeStartOrEnd()) {
15071515
// We note the locations of these intrinsic calls so that we can
15081516
// delete them later if the optimization succeeds, this is safe

llvm/test/Transforms/MemCpyOpt/stack-move.ll

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,32 @@ bb2:
981981

982982
; Optimization failures follow:
983983

984+
; TODO: we can merge those alloca if we move src alloca to the start of the BB.
985+
; Tests that a the optimization isn't performed,
986+
; when any use that isn't dominated by SrcAlloca exists.
987+
define i32 @use_not_dominated_by_src_alloca() {
988+
; CHECK-LABEL: define i32 @use_not_dominated_by_src_alloca() {
989+
; CHECK-NEXT: [[DEST:%.*]] = alloca i1, align 1
990+
; CHECK-NEXT: [[DEST_GEP:%.*]] = getelementptr i64, ptr [[DEST]], i64 -1
991+
; CHECK-NEXT: [[DEST_USE:%.*]] = load i8, ptr [[DEST_GEP]], align 1
992+
; CHECK-NEXT: [[SRC:%.*]] = alloca i8, align 4
993+
; CHECK-NEXT: [[SRC_VAL:%.*]] = load i1, ptr [[SRC]], align 4
994+
; CHECK-NEXT: store i1 [[SRC_VAL]], ptr [[DEST]], align 1
995+
; CHECK-NEXT: ret i32 0
996+
;
997+
%dest = alloca i1, align 1
998+
; Replacing the use of dest with src causes no domination uses.
999+
%dest.gep = getelementptr i64, ptr %dest, i64 -1
1000+
%dest.use = load i8, ptr %dest.gep, align 1
1001+
%src = alloca i8, align 4
1002+
%src.val = load i1, ptr %src, align 4
1003+
1004+
store i1 %src.val, ptr %dest, align 1
1005+
1006+
ret i32 0
1007+
}
1008+
1009+
9841010
; Tests that a memcpy that doesn't completely overwrite a stack value is a use
9851011
; for the purposes of liveness analysis, not a definition.
9861012
define void @incomplete_memcpy() {

0 commit comments

Comments
 (0)