diff --git a/llvm/include/llvm/Analysis/MustExecute.h b/llvm/include/llvm/Analysis/MustExecute.h --- a/llvm/include/llvm/Analysis/MustExecute.h +++ b/llvm/include/llvm/Analysis/MustExecute.h @@ -420,6 +420,30 @@ } ///} + /// Helper to look for \p I in the context of \p PP. + /// + /// The context is expanded until \p I was found or no more expansion is + /// possible. + /// + /// \returns True, iff \p I was found. + bool findInContextOf(const Instruction *I, const Instruction *PP) { + auto EIt = begin(PP), EEnd = end(PP); + return findInContextOf(I, EIt, EEnd); + } + + /// Helper to look for \p I in the context defined by \p EIt and \p EEnd. + /// + /// The context is expanded until \p I was found or no more expansion is + /// possible. + /// + /// \returns True, iff \p I was found. + bool findInContextOf(const Instruction *I, iterator &EIt, iterator &EEnd) { + bool Found = EIt.count(I); + while (!Found && EIt != EEnd) + Found = (++EIt).getCurrentInst() == I; + return Found; + } + /// Return the next instruction that is guaranteed to be executed after \p PP. /// /// \param It The iterator that is used to traverse the must be diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -728,12 +728,10 @@ SetVector NextUses; + auto EIt = Explorer.begin(CtxI), EEnd = Explorer.end(CtxI); for (const Use *U : Uses) { if (const Instruction *UserI = dyn_cast(U->getUser())) { - auto EIt = Explorer.begin(CtxI), EEnd = Explorer.end(CtxI); - bool Found = EIt.count(UserI); - while (!Found && ++EIt != EEnd) - Found = EIt.getCurrentInst() == UserI; + bool Found = Explorer.findInContextOf(UserI, EIt, EEnd); if (Found && Base::followUse(A, U, UserI)) for (const Use &Us : UserI->uses()) NextUses.insert(&Us); @@ -3634,7 +3632,21 @@ const Function *F = getAssociatedFunction(); const auto *TLI = A.getInfoCache().getTargetLibraryInfoForFunction(*F); + MustBeExecutedContextExplorer &Explorer = + A.getInfoCache().getMustBeExecutedContextExplorer(); + + auto FreeCheck = [&](Instruction &I) { + const auto &Frees = FreesForMalloc.lookup(&I); + if (Frees.size() != 1) + return false; + Instruction *UniqueFree = *Frees.begin(); + return Explorer.findInContextOf(UniqueFree, I.getNextNode()); + }; + auto UsesCheck = [&](Instruction &I) { + bool ValidUsesOnly = true; + bool MustUse = true; + SmallPtrSet Visited; SmallVector Worklist; @@ -3652,10 +3664,12 @@ continue; if (auto *SI = dyn_cast(UserI)) { if (SI->getValueOperand() == U->get()) { - LLVM_DEBUG(dbgs() << "[H2S] escaping store to memory: " << *UserI << "\n"); - return false; + LLVM_DEBUG(dbgs() + << "[H2S] escaping store to memory: " << *UserI << "\n"); + ValidUsesOnly = false; + } else { + // A store into the malloc'ed memory is fine. } - // A store into the malloc'ed memory is fine. continue; } @@ -3673,8 +3687,14 @@ // Record malloc. if (isFreeCall(UserI, TLI)) { - FreesForMalloc[&I].insert( - cast(const_cast(UserI))); + if (MustUse) { + FreesForMalloc[&I].insert( + cast(const_cast(UserI))); + } else { + LLVM_DEBUG(dbgs() << "[H2S] free potentially on different mallocs: " + << *UserI << "\n"); + ValidUsesOnly = false; + } continue; } @@ -3688,22 +3708,25 @@ if (!NoCaptureAA.isAssumedNoCapture() || !NoFreeAA.isAssumedNoFree()) { LLVM_DEBUG(dbgs() << "[H2S] Bad user: " << *UserI << "\n"); - return false; + ValidUsesOnly = false; } continue; } - if (isa(UserI) || isa(UserI)) { + if (isa(UserI) || isa(UserI) || + isa(UserI) || isa(UserI)) { + MustUse &= !(isa(UserI) || isa(UserI)); for (Use &U : UserI->uses()) Worklist.push_back(&U); continue; } - // Unknown user. + // Unknown user for which we can not track uses further (in a way that + // makes sense). LLVM_DEBUG(dbgs() << "[H2S] Unknown user: " << *UserI << "\n"); - return false; + ValidUsesOnly = false; } - return true; + return ValidUsesOnly; }; auto MallocCallocCheck = [&](Instruction &I) { @@ -3720,7 +3743,7 @@ if (IsMalloc) { if (auto *Size = dyn_cast(I.getOperand(0))) if (Size->getValue().sle(MaxHeapToStackSize)) - if (UsesCheck(I)) { + if (UsesCheck(I) || FreeCheck(I)) { MallocCalls.insert(&I); return true; } @@ -3730,7 +3753,7 @@ if (auto *Size = dyn_cast(I.getOperand(1))) if ((Size->getValue().umul_ov(Num->getValue(), Overflow)) .sle(MaxHeapToStackSize)) - if (!Overflow && UsesCheck(I)) { + if (!Overflow && (UsesCheck(I) || FreeCheck(I))) { MallocCalls.insert(&I); return true; } @@ -3756,8 +3779,10 @@ /// See AbstractAttribute::trackStatistics() void trackStatistics() const override { STATS_DECL(MallocCalls, Function, - "Number of MallocCalls converted to allocas"); - BUILD_STAT_NAME(MallocCalls, Function) += MallocCalls.size(); + "Number of malloc calls converted to allocas"); + for (auto *C : MallocCalls) + if (!BadMallocCalls.count(C)) + ++BUILD_STAT_NAME(MallocCalls, Function); } }; diff --git a/llvm/test/Transforms/FunctionAttrs/heap_to_stack.ll b/llvm/test/Transforms/FunctionAttrs/heap_to_stack.ll --- a/llvm/test/Transforms/FunctionAttrs/heap_to_stack.ll +++ b/llvm/test/Transforms/FunctionAttrs/heap_to_stack.ll @@ -8,7 +8,7 @@ declare void @sync_func(i8* %p) -declare void @sync_will_return(i8* %p) willreturn +declare void @sync_will_return(i8* %p) willreturn nounwind declare void @no_sync_func(i8* nocapture %p) nofree nosync willreturn @@ -202,11 +202,11 @@ } ; TEST 11 -; FIXME: should be ok define void @test11() { %1 = tail call noalias i8* @malloc(i64 4) - ; CHECK: @malloc(i64 4) + ; CHECK: test11 + ; CHECK-NEXT: alloc ; CHECK-NEXT: @sync_will_return(i8* %1) tail call void @sync_will_return(i8* %1) tail call void @free(i8* %1) @@ -330,9 +330,29 @@ %1 = tail call noalias i8* @malloc(i64 4) ; CHECK-NEXT: store i8* %1, i8** %P store i8* %1, i8** %P - ; CHECK-NEXT: @no_sync_func(i8* %1) + ; CHECK-NEXT: @no_sync_func(i8* nocapture %1) tail call void @no_sync_func(i8* %1) ; CHECK-NEXT: @free(i8* %1) tail call void @free(i8* %1) ret void } + +define void @test16c(i8 %v, i8** %P) { + ; CHECK: %1 = alloca + %1 = tail call noalias i8* @malloc(i64 4) + ; CHECK-NEXT: store i8* %1, i8** %P + store i8* %1, i8** %P + ; CHECK-NEXT: @no_sync_func(i8* nocapture %1) + tail call void @no_sync_func(i8* %1) nounwind + ; CHECK-NOT: @free + tail call void @free(i8* %1) + ret void +} + +define void @test16d(i8 %v, i8** %P) { + ; CHECK: %1 = tail call noalias i8* @malloc(i64 4) + %1 = tail call noalias i8* @malloc(i64 4) + ; CHECK-NEXT: store i8* %1, i8** %P + store i8* %1, i8** %P + ret void +} diff --git a/llvm/test/Transforms/FunctionAttrs/noalias_returned.ll b/llvm/test/Transforms/FunctionAttrs/noalias_returned.ll --- a/llvm/test/Transforms/FunctionAttrs/noalias_returned.ll +++ b/llvm/test/Transforms/FunctionAttrs/noalias_returned.ll @@ -204,7 +204,7 @@ ; CHECK-NEXT: tail call void @use_nocapture(i8* noalias nonnull align 4 dereferenceable(1) [[A]]) ; CHECK-NEXT: tail call void @use_nocapture(i8* noalias nonnull align 4 dereferenceable(1) [[A]]) ; CHECK-NEXT: tail call void @use_nocapture(i8* noalias nocapture [[B]]) -; CHECK-NEXT: tail call void @use_nocapture(i8* noalias [[B]]) +; CHECK-NEXT: tail call void @use_nocapture(i8* noalias nocapture [[B]]) ; CHECK-NEXT: ret void ; %A = alloca i8, align 4 @@ -221,10 +221,10 @@ ; CHECK-NEXT: [[A:%.*]] = tail call noalias i8* @malloc(i64 4) ; FIXME: This should be @use_nocapture(i8* noalias [[A]]) ; CHECK-NEXT: tail call void @use_nocapture(i8* nocapture [[A]]) -; FIXME: This should be @use_nocapture(i8* noalias [[A]]) -; CHECK-NEXT: tail call void @use_nocapture(i8* [[A]]) +; FIXME: This should be @use_nocapture(i8* noalias nocapture [[A]]) +; CHECK-NEXT: tail call void @use_nocapture(i8* nocapture [[A]]) ; CHECK-NEXT: tail call void @use(i8* [[A]]) -; CHECK-NEXT: tail call void @use_nocapture(i8* [[A]]) +; CHECK-NEXT: tail call void @use_nocapture(i8* nocapture [[A]]) ; CHECK-NEXT: ret void ; %A = tail call noalias i8* @malloc(i64 4) @@ -239,7 +239,7 @@ define void @test12_3(){ ; CHECK-LABEL: @test12_3( %A = tail call noalias i8* @malloc(i64 4) -; CHECK: tail call void @two_args(i8* nocapture %A, i8* %A) +; CHECK: tail call void @two_args(i8* nocapture %A, i8* nocapture %A) tail call void @two_args(i8* %A, i8* %A) ret void } @@ -252,17 +252,17 @@ %A_1 = getelementptr i8, i8* %A, i64 1 %B_0 = getelementptr i8, i8* %B, i64 0 -; CHECK: tail call void @two_args(i8* noalias %A, i8* noalias %B) +; CHECK: tail call void @two_args(i8* noalias nocapture %A, i8* noalias nocapture %B) tail call void @two_args(i8* %A, i8* %B) -; CHECK: tail call void @two_args(i8* %A, i8* nocapture %A_0) +; CHECK: tail call void @two_args(i8* nocapture %A, i8* nocapture %A_0) tail call void @two_args(i8* %A, i8* %A_0) -; CHECK: tail call void @two_args(i8* %A, i8* %A_1) +; CHECK: tail call void @two_args(i8* nocapture %A, i8* nocapture %A_1) tail call void @two_args(i8* %A, i8* %A_1) -; FIXME: This should be @two_args(i8* noalias %A_0, i8* noalias %B_0) -; CHECK: tail call void @two_args(i8* %A_0, i8* nocapture %B_0) +; FIXME: This should be @two_args(i8* noalias nocapture %A_0, i8* noalias nocapture %B_0) +; CHECK: tail call void @two_args(i8* nocapture %A_0, i8* nocapture %B_0) tail call void @two_args(i8* %A_0, i8* %B_0) ret void }