From 098e653a5bed2e5fb2af6ccaedf2268d7d3935fa Mon Sep 17 00:00:00 2001 From: DianQK Date: Thu, 28 Sep 2023 16:13:21 +0800 Subject: [PATCH] [MemCpyOpt] Merge alias metadatas when replacing arguments (#67539) 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 4e6e476329a8359cb381517864ed6f88a152e37b) --- .../lib/Transforms/Scalar/MemCpyOptimizer.cpp | 23 +++++++++++-------- llvm/test/Transforms/MemCpyOpt/memcpy.ll | 5 ++-- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp index 00937e0d734ab3..91dc5018d232eb 100644 --- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp +++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp @@ -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 @@ -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; @@ -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; diff --git a/llvm/test/Transforms/MemCpyOpt/memcpy.ll b/llvm/test/Transforms/MemCpyOpt/memcpy.ll index d7475e7f5ea591..f7c1c85517ffa3 100644 --- a/llvm/test/Transforms/MemCpyOpt/memcpy.ll +++ b/llvm/test/Transforms/MemCpyOpt/memcpy.ll @@ -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