From 8eac42b1cfc7c139cefa4eca95efd0ce1c31f380 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Sun, 18 Jul 2021 18:33:32 +0200 Subject: [PATCH] [Inline] Fix noalias addition on simplified instructions (PR50589) When adding noalias/alias.scope metadata, we analyze the instructions of the original callee, and then place metadata on the corresponding inlined instructions in the caller as provided by VMap. However, this assumes that this actually a clone of the instruction, rather than the result of simplification. If simplification occurred, the instruction that VMap points to may not have any relationship as far as ModRef behavior is concerned. Fix this by tracking simplified instructions during cloning and then only processing instructions that have not been simplified. This is done with an additional map form original to cloned instruction, into which we only insert if no simplification is performed. The mapping in VMap can then be compared to this map. If they're the same, the instruction hasn't been simplified. (I originally wanted to only track a set of simplified instructions, but that wouldn't work if the instruction only gets simplified afterwards, e.g. based on rewritten phis.) Fixes https://bugs.llvm.org/show_bug.cgi?id=50589. Differential Revision: https://reviews.llvm.org/D106242 --- include/llvm/Transforms/Utils/Cloning.h | 9 +++++++++ lib/Transforms/Utils/CloneFunction.cpp | 8 ++++++-- lib/Transforms/Utils/InlineFunction.cpp | 7 ++++--- test/Transforms/Inline/pr50589.ll | 8 ++++---- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/include/llvm/Transforms/Utils/Cloning.h b/include/llvm/Transforms/Utils/Cloning.h index 3e5f3d5313b..f4fb265c25e 100644 --- a/include/llvm/Transforms/Utils/Cloning.h +++ b/include/llvm/Transforms/Utils/Cloning.h @@ -75,7 +75,16 @@ struct ClonedCodeInfo { /// originally inserted callsites were DCE'ed after they were cloned. std::vector OperandBundleCallSites; + /// Like VMap, but maps only unsimplified instructions. Values in the map + /// may be dangling, it is only intended to be used via isSimplified(), to + /// check whether the main VMap mapping involves simplification or not. + DenseMap OrigVMap; + ClonedCodeInfo() = default; + + bool isSimplified(const Value *From, const Value *To) const { + return OrigVMap.lookup(From) != To; + } }; /// Return a copy of the specified basic block, but without diff --git a/lib/Transforms/Utils/CloneFunction.cpp b/lib/Transforms/Utils/CloneFunction.cpp index 08b370aeefe..0ac9a5aaa42 100644 --- a/lib/Transforms/Utils/CloneFunction.cpp +++ b/lib/Transforms/Utils/CloneFunction.cpp @@ -412,10 +412,12 @@ void PruningFunctionCloner::CloneBlock( NewBB->getInstList().push_back(NewInst); hasCalls |= (isa(II) && !isa(II)); - if (CodeInfo) + if (CodeInfo) { + CodeInfo->OrigVMap[&*II] = NewInst; if (auto *CB = dyn_cast(&*II)) if (CB->hasOperandBundles()) CodeInfo->OperandBundleCallSites.push_back(NewInst); + } if (const AllocaInst *AI = dyn_cast(II)) { if (isa(AI->getArraySize())) @@ -469,10 +471,12 @@ void PruningFunctionCloner::CloneBlock( NewBB->getInstList().push_back(NewInst); VMap[OldTI] = NewInst; // Add instruction map to value. - if (CodeInfo) + if (CodeInfo) { + CodeInfo->OrigVMap[OldTI] = NewInst; if (auto *CB = dyn_cast(OldTI)) if (CB->hasOperandBundles()) CodeInfo->OperandBundleCallSites.push_back(NewInst); + } // Recursively clone any reachable successor blocks. append_range(ToClone, successors(BB->getTerminator())); diff --git a/lib/Transforms/Utils/InlineFunction.cpp b/lib/Transforms/Utils/InlineFunction.cpp index 1272042eb1c..9363e0dc9b2 100644 --- a/lib/Transforms/Utils/InlineFunction.cpp +++ b/lib/Transforms/Utils/InlineFunction.cpp @@ -939,7 +939,8 @@ void ScopedAliasMetadataDeepCloner::remap(Function::iterator FStart, /// parameters with noalias metadata specifying the new scope, and tag all /// non-derived loads, stores and memory intrinsics with the new alias scopes. static void AddAliasScopeMetadata(CallBase &CB, ValueToValueMapTy &VMap, - const DataLayout &DL, AAResults *CalleeAAR) { + const DataLayout &DL, AAResults *CalleeAAR, + ClonedCodeInfo &InlinedFunctionInfo) { if (!EnableNoAliasConversion) return; @@ -1009,7 +1010,7 @@ static void AddAliasScopeMetadata(CallBase &CB, ValueToValueMapTy &VMap, continue; Instruction *NI = dyn_cast(VMI->second); - if (!NI) + if (!NI || InlinedFunctionInfo.isSimplified(I, NI)) continue; bool IsArgMemOnlyCall = false, IsFuncCall = false; @@ -2037,7 +2038,7 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI, SAMetadataCloner.remap(FirstNewBlock, Caller->end()); // Add noalias metadata if necessary. - AddAliasScopeMetadata(CB, VMap, DL, CalleeAAR); + AddAliasScopeMetadata(CB, VMap, DL, CalleeAAR, InlinedFunctionInfo); // Clone return attributes on the callsite into the calls within the inlined // function which feed into its return value. diff --git a/test/Transforms/Inline/pr50589.ll b/test/Transforms/Inline/pr50589.ll index 7431c1d37f8..ba7ed236b12 100644 --- a/test/Transforms/Inline/pr50589.ll +++ b/test/Transforms/Inline/pr50589.ll @@ -15,10 +15,10 @@ define <2 x i8> @callee1(<2 x i8>* %ptr1, <2 x i8>* noalias %ptr2, <2 x i1> %mas ret <2 x i8> %ret } -; TODO: The load should not have !noalias. +; The load should not have !noalias. define void @caller1(<2 x i8>* %ptr1, <2 x i8>* %ptr2) { ; CHECK-LABEL: @caller1( -; CHECK-NEXT: [[PASSTHRU:%.*]] = load <2 x i8>, <2 x i8>* [[PTR2:%.*]], align 2, !noalias !0 +; CHECK-NEXT: [[PASSTHRU:%.*]] = load <2 x i8>, <2 x i8>* [[PTR2:%.*]], align 2{{$}} ; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META0:![0-9]+]]) ; CHECK-NEXT: store <2 x i8> zeroinitializer, <2 x i8>* [[PTR2]], align 2, !alias.scope !0 ; CHECK-NEXT: ret void @@ -41,11 +41,11 @@ define <2 x i8> @callee2(<2 x i8>* %ptr1, <2 x i8>* noalias %ptr2, <2 x i1> %mas ret <2 x i8> %ret } -; TODO: The load should not have !noalias. +; The load should not have !noalias. define void @caller2(<2 x i8>* %ptr1, <2 x i8>* %ptr2) { ; CHECK-LABEL: @caller2( ; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META3:![0-9]+]]) -; CHECK-NEXT: [[PASSTHRU_I:%.*]] = load <2 x i8>, <2 x i8>* [[PTR2:%.*]], align 2, !alias.scope !3, !noalias !3 +; CHECK-NEXT: [[PASSTHRU_I:%.*]] = load <2 x i8>, <2 x i8>* [[PTR2:%.*]], align 2, !alias.scope !3{{$}} ; CHECK-NEXT: store <2 x i8> zeroinitializer, <2 x i8>* [[PTR2]], align 2, !alias.scope !3 ; CHECK-NEXT: ret void ;