diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp --- a/llvm/lib/Transforms/Utils/InlineFunction.cpp +++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp @@ -782,7 +782,8 @@ /// When inlining a call site that has !llvm.mem.parallel_loop_access, /// !llvm.access.group, !alias.scope or !noalias metadata, that metadata should /// be propagated to all memory-accessing cloned instructions. -static void PropagateCallSiteMetadata(CallBase &CB, ValueToValueMapTy &VMap) { +static void PropagateCallSiteMetadata(CallBase &CB, Function::iterator FStart, + Function::iterator FEnd) { MDNode *MemParallelLoopAccess = CB.getMetadata(LLVMContext::MD_mem_parallel_loop_access); MDNode *AccessGroup = CB.getMetadata(LLVMContext::MD_access_group); @@ -791,41 +792,33 @@ if (!MemParallelLoopAccess && !AccessGroup && !AliasScope && !NoAlias) return; - for (ValueToValueMapTy::iterator VMI = VMap.begin(), VMIE = VMap.end(); - VMI != VMIE; ++VMI) { - // Check that key is an instruction, to skip the Argument mapping, which - // points to an instruction in the original function, not the inlined one. - if (!VMI->second || !isa(VMI->first)) - continue; - - Instruction *NI = dyn_cast(VMI->second); - if (!NI) - continue; - - // This metadata is only relevant for instructions that access memory. - if (!NI->mayReadOrWriteMemory()) - continue; + for (BasicBlock &BB : make_range(FStart, FEnd)) { + for (Instruction &I : BB) { + // This metadata is only relevant for instructions that access memory. + if (!I.mayReadOrWriteMemory()) + continue; - if (MemParallelLoopAccess) { - // TODO: This probably should not overwrite MemParalleLoopAccess. - MemParallelLoopAccess = MDNode::concatenate( - NI->getMetadata(LLVMContext::MD_mem_parallel_loop_access), - MemParallelLoopAccess); - NI->setMetadata(LLVMContext::MD_mem_parallel_loop_access, + if (MemParallelLoopAccess) { + // TODO: This probably should not overwrite MemParalleLoopAccess. + MemParallelLoopAccess = MDNode::concatenate( + I.getMetadata(LLVMContext::MD_mem_parallel_loop_access), + MemParallelLoopAccess); + I.setMetadata(LLVMContext::MD_mem_parallel_loop_access, MemParallelLoopAccess); - } + } - if (AccessGroup) - NI->setMetadata(LLVMContext::MD_access_group, uniteAccessGroups( - NI->getMetadata(LLVMContext::MD_access_group), AccessGroup)); + if (AccessGroup) + I.setMetadata(LLVMContext::MD_access_group, uniteAccessGroups( + I.getMetadata(LLVMContext::MD_access_group), AccessGroup)); - if (AliasScope) - NI->setMetadata(LLVMContext::MD_alias_scope, MDNode::concatenate( - NI->getMetadata(LLVMContext::MD_alias_scope), AliasScope)); + if (AliasScope) + I.setMetadata(LLVMContext::MD_alias_scope, MDNode::concatenate( + I.getMetadata(LLVMContext::MD_alias_scope), AliasScope)); - if (NoAlias) - NI->setMetadata(LLVMContext::MD_noalias, MDNode::concatenate( - NI->getMetadata(LLVMContext::MD_noalias), NoAlias)); + if (NoAlias) + I.setMetadata(LLVMContext::MD_noalias, MDNode::concatenate( + I.getMetadata(LLVMContext::MD_noalias), NoAlias)); + } } } @@ -846,9 +839,9 @@ /// subsequent remap() calls. void clone(); - /// Remap instructions in the given VMap from the original to the cloned + /// Remap instructions in the given range from the original to the cloned /// metadata. - void remap(ValueToValueMapTy &VMap); + void remap(Function::iterator FStart, Function::iterator FEnd); }; ScopedAliasMetadataDeepCloner::ScopedAliasMetadataDeepCloner( @@ -909,34 +902,27 @@ } } -void ScopedAliasMetadataDeepCloner::remap(ValueToValueMapTy &VMap) { +void ScopedAliasMetadataDeepCloner::remap(Function::iterator FStart, + Function::iterator FEnd) { if (MDMap.empty()) return; // Nothing to do. - for (auto Entry : VMap) { - // Check that key is an instruction, to skip the Argument mapping, which - // points to an instruction in the original function, not the inlined one. - if (!Entry->second || !isa(Entry->first)) - continue; - - Instruction *I = dyn_cast(Entry->second); - if (!I) - continue; - - // Only update scopes when we find them in the map. If they are not, it is - // because we already handled that instruction before. This is faster than - // tracking which instructions we already updated. - if (MDNode *M = I->getMetadata(LLVMContext::MD_alias_scope)) - if (MDNode *MNew = MDMap.lookup(M)) - I->setMetadata(LLVMContext::MD_alias_scope, MNew); - - if (MDNode *M = I->getMetadata(LLVMContext::MD_noalias)) - if (MDNode *MNew = MDMap.lookup(M)) - I->setMetadata(LLVMContext::MD_noalias, MNew); - - if (auto *Decl = dyn_cast(I)) - if (MDNode *MNew = MDMap.lookup(Decl->getScopeList())) - Decl->setScopeList(MNew); + for (BasicBlock &BB : make_range(FStart, FEnd)) { + for (Instruction &I : BB) { + // TODO: The null checks for the MDMap.lookup() results should no longer + // be necessary. + if (MDNode *M = I.getMetadata(LLVMContext::MD_alias_scope)) + if (MDNode *MNew = MDMap.lookup(M)) + I.setMetadata(LLVMContext::MD_alias_scope, MNew); + + if (MDNode *M = I.getMetadata(LLVMContext::MD_noalias)) + if (MDNode *MNew = MDMap.lookup(M)) + I.setMetadata(LLVMContext::MD_noalias, MNew); + + if (auto *Decl = dyn_cast(&I)) + if (MDNode *MNew = MDMap.lookup(Decl->getScopeList())) + Decl->setScopeList(MNew); + } } } @@ -2038,7 +2024,7 @@ // Now clone the inlined noalias scope metadata. SAMetadataCloner.clone(); - SAMetadataCloner.remap(VMap); + SAMetadataCloner.remap(FirstNewBlock, Caller->end()); // Add noalias metadata if necessary. AddAliasScopeMetadata(CB, VMap, DL, CalleeAAR); @@ -2048,7 +2034,7 @@ AddReturnAttributes(CB, VMap); // Propagate metadata on the callsite if necessary. - PropagateCallSiteMetadata(CB, VMap); + PropagateCallSiteMetadata(CB, FirstNewBlock, Caller->end()); // Register any cloned assumptions. if (IFI.GetAssumptionCache) diff --git a/llvm/test/Transforms/Inline/pr50270.ll b/llvm/test/Transforms/Inline/pr50270.ll new file mode 100644 --- /dev/null +++ b/llvm/test/Transforms/Inline/pr50270.ll @@ -0,0 +1,71 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; RUN: opt -S -inline < %s | FileCheck %s + +; This tests cases where instructions in the callee are simplified to +; instructions in the caller, thus making VMap contain instructions from +; the caller. We should not be assigning incorrect noalias metadata in +; that case. + +declare { i64* } @opaque_callee() + +define { i64* } @callee(i64* %x) { +; CHECK-LABEL: @callee( +; CHECK-NEXT: [[RES:%.*]] = insertvalue { i64* } undef, i64* [[X:%.*]], 0 +; CHECK-NEXT: ret { i64* } [[RES]] +; + %res = insertvalue { i64* } undef, i64* %x, 0 + ret { i64* } %res +} + +; @opaque_callee() should not receive noalias metadata here. +define void @caller() { +; CHECK-LABEL: @caller( +; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata !0) +; CHECK-NEXT: [[S:%.*]] = call { i64* } @opaque_callee() +; CHECK-NEXT: [[X:%.*]] = extractvalue { i64* } [[S]], 0 +; CHECK-NEXT: ret void +; + call void @llvm.experimental.noalias.scope.decl(metadata !0) + %s = call { i64* } @opaque_callee() + %x = extractvalue { i64* } %s, 0 + call { i64* } @callee(i64* %x), !noalias !0 + ret void +} + +; @opaque_callee() should no the same noalias metadata as the load from the +; else branch, not as the load in the if branch. +define { i64* } @self_caller(i1 %c, i64* %a) { +; CHECK-LABEL: @self_caller( +; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata !0) +; CHECK-NEXT: br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]] +; CHECK: if: +; CHECK-NEXT: [[S:%.*]] = call { i64* } @opaque_callee(), !noalias !0 +; CHECK-NEXT: [[X:%.*]] = extractvalue { i64* } [[S]], 0 +; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata !3) +; CHECK-NEXT: [[TMP1:%.*]] = load volatile i64, i64* [[X]], align 4, !alias.scope !3 +; CHECK-NEXT: ret { i64* } [[S]] +; CHECK: else: +; CHECK-NEXT: [[R2:%.*]] = insertvalue { i64* } undef, i64* [[A:%.*]], 0 +; CHECK-NEXT: [[TMP2:%.*]] = load volatile i64, i64* [[A]], align 4, !alias.scope !0 +; CHECK-NEXT: ret { i64* } [[R2]] +; + call void @llvm.experimental.noalias.scope.decl(metadata !0) + br i1 %c, label %if, label %else + +if: + %s = call { i64* } @opaque_callee(), !noalias !0 + %x = extractvalue { i64* } %s, 0 + %r = call { i64* } @self_caller(i1 false, i64* %x) + ret { i64* } %r + +else: + %r2 = insertvalue { i64* } undef, i64* %a, 0 + load volatile i64, i64* %a, !alias.scope !0 + ret { i64* } %r2 +} + +declare void @llvm.experimental.noalias.scope.decl(metadata) + +!0 = !{!1} +!1 = !{!1, !2, !"scope"} +!2 = !{!2, !"domain"}