Skip to content

Commit

Permalink
[MemCpyOpt] Merge alias metadatas when replacing arguments (llvm#67539)
Browse files Browse the repository at this point in the history
Alias metadata may no longer be valid after replacing the call argument.
Fix this by merging it with the memcpy alias metadata.

This fixes a miscompilation encountered in
https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Failing.20tests.20when.20rustc.20is.20compiled.20with.201.20CGU.

(cherry picked from commit 4e6e476)
  • Loading branch information
DianQK authored and tru committed Sep 29, 2023
1 parent 78d201e commit 098e653
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 11 deletions.
23 changes: 14 additions & 9 deletions llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,17 @@ static bool writtenBetween(MemorySSA *MSSA, BatchAAResults &AA,
return !MSSA->dominates(Clobber, Start);
}

// Update AA metadata
static void combineAAMetadata(Instruction *ReplInst, Instruction *I) {
// FIXME: MD_tbaa_struct and MD_mem_parallel_loop_access should also be
// handled here, but combineMetadata doesn't support them yet
unsigned KnownIDs[] = {LLVMContext::MD_tbaa, LLVMContext::MD_alias_scope,
LLVMContext::MD_noalias,
LLVMContext::MD_invariant_group,
LLVMContext::MD_access_group};
combineMetadata(ReplInst, I, KnownIDs, true);
}

/// When scanning forward over instructions, we look for some other patterns to
/// fold away. In particular, this looks for stores to neighboring locations of
/// memory. If it sees enough consecutive ones, it attempts to merge them
Expand Down Expand Up @@ -1086,16 +1097,9 @@ bool MemCpyOptPass::performCallSlotOptzn(Instruction *cpyLoad,
MSSA->getMemoryAccess(C));
}

// Update AA metadata
// FIXME: MD_tbaa_struct and MD_mem_parallel_loop_access should also be
// handled here, but combineMetadata doesn't support them yet
unsigned KnownIDs[] = {LLVMContext::MD_tbaa, LLVMContext::MD_alias_scope,
LLVMContext::MD_noalias,
LLVMContext::MD_invariant_group,
LLVMContext::MD_access_group};
combineMetadata(C, cpyLoad, KnownIDs, true);
combineAAMetadata(C, cpyLoad);
if (cpyLoad != cpyStore)
combineMetadata(C, cpyStore, KnownIDs, true);
combineAAMetadata(C, cpyStore);

++NumCallSlot;
return true;
Expand Down Expand Up @@ -1694,6 +1698,7 @@ bool MemCpyOptPass::processImmutArgument(CallBase &CB, unsigned ArgNo) {
<< " " << CB << "\n");

// Otherwise we're good! Update the immut argument.
combineAAMetadata(&CB, MDep);
CB.setArgOperand(ArgNo, MDep->getSource());
++NumMemCpyInstr;
return true;
Expand Down
5 changes: 3 additions & 2 deletions llvm/test/Transforms/MemCpyOpt/memcpy.ll
Original file line number Diff line number Diff line change
Expand Up @@ -695,10 +695,11 @@ define void @immut_valid_align_branched(i1 %c, ptr noalias align 4 %val) {
ret void
}

; FIXME: This is a miscompile.
; Merge/drop noalias metadata when replacing parameter.
define void @immut_param_noalias_metadata(ptr align 4 byval(i32) %ptr) {
; CHECK-LABEL: @immut_param_noalias_metadata(
; CHECK-NEXT: call void @f(ptr noalias nocapture readonly [[PTR:%.*]]), !alias.scope !0
; CHECK-NEXT: store i32 1, ptr [[PTR:%.*]], align 4, !noalias !0
; CHECK-NEXT: call void @f(ptr noalias nocapture readonly [[PTR]])
; CHECK-NEXT: ret void
;
%tmp = alloca i32, align 4
Expand Down

0 comments on commit 098e653

Please sign in to comment.