From 1bd33691cb1d10fb118ddf7f9fb305b112f67cbe Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 4 Mar 2022 13:07:58 +0100 Subject: [PATCH] [CoroElide] Remove fallback for frame layout determination Only determine the frame layout based on dereferenceable and align attributes, and remove the type-based fallback, which is incompatible with opaque pointers. The dereferenceable attribute is required, while the align attribute uses default alignment of 1 (commonly, align 1 attributes do not get placed, relying on default alignment). The CoroSplit pass producing the resume function adds the necessary attributes in https://github.com/llvm/llvm-project/blob/7daed359111f6d151fef447f520f85ef1dabedf6/llvm/lib/Transforms/Coroutines/CoroSplit.cpp#L840, and their presence is checked in coro-debug.ll at least. Differential Revision: https://reviews.llvm.org/D120988 --- llvm/lib/Transforms/Coroutines/CoroElide.cpp | 39 +++++++++------------- .../Transforms/Coroutines/coro-elide-musttail.ll | 2 +- llvm/test/Transforms/Coroutines/coro-elide-stat.ll | 2 +- llvm/test/Transforms/Coroutines/coro-elide.ll | 2 +- llvm/test/Transforms/Coroutines/coro-heap-elide.ll | 2 +- 5 files changed, 20 insertions(+), 27 deletions(-) diff --git a/llvm/lib/Transforms/Coroutines/CoroElide.cpp b/llvm/lib/Transforms/Coroutines/CoroElide.cpp index 84bebb7..1e37f8c 100644 --- a/llvm/lib/Transforms/Coroutines/CoroElide.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroElide.cpp @@ -103,21 +103,12 @@ static void removeTailCallAttribute(AllocaInst *Frame, AAResults &AA) { // Given a resume function @f.resume(%f.frame* %frame), returns the size // and expected alignment of %f.frame type. -static std::pair getFrameLayout(Function *Resume) { - // Prefer to pull information from the function attributes. +static Optional> getFrameLayout(Function *Resume) { + // Pull information from the function attributes. auto Size = Resume->getParamDereferenceableBytes(0); - auto Align = Resume->getParamAlign(0); - - // If those aren't given, extract them from the type. - if (Size == 0 || !Align) { - auto *FrameTy = Resume->arg_begin()->getType()->getPointerElementType(); - - const DataLayout &DL = Resume->getParent()->getDataLayout(); - if (!Size) Size = DL.getTypeAllocSize(FrameTy); - if (!Align) Align = DL.getABITypeAlign(FrameTy); - } - - return std::make_pair(Size, *Align); + if (!Size) + return None; + return std::make_pair(Size, Resume->getParamAlign(0).valueOrOne()); } // Finds first non alloca instruction in the entry block of a function. @@ -361,17 +352,19 @@ bool Lowerer::processCoroId(CoroIdInst *CoroId, AAResults &AA, replaceWithConstant(DestroyAddrConstant, It.second); if (ShouldElide) { - auto FrameSizeAndAlign = getFrameLayout(cast(ResumeAddrConstant)); - elideHeapAllocations(CoroId->getFunction(), FrameSizeAndAlign.first, - FrameSizeAndAlign.second, AA); - coro::replaceCoroFree(CoroId, /*Elide=*/true); - NumOfCoroElided++; + if (auto FrameSizeAndAlign = + getFrameLayout(cast(ResumeAddrConstant))) { + elideHeapAllocations(CoroId->getFunction(), FrameSizeAndAlign->first, + FrameSizeAndAlign->second, AA); + coro::replaceCoroFree(CoroId, /*Elide=*/true); + NumOfCoroElided++; #ifndef NDEBUG - if (!CoroElideInfoOutputFilename.empty()) - *getOrCreateLogFile() - << "Elide " << CoroId->getCoroutine()->getName() << " in " - << CoroId->getFunction()->getName() << "\n"; + if (!CoroElideInfoOutputFilename.empty()) + *getOrCreateLogFile() + << "Elide " << CoroId->getCoroutine()->getName() << " in " + << CoroId->getFunction()->getName() << "\n"; #endif + } } return true; diff --git a/llvm/test/Transforms/Coroutines/coro-elide-musttail.ll b/llvm/test/Transforms/Coroutines/coro-elide-musttail.ll index 63b7aa1..a25f96b 100644 --- a/llvm/test/Transforms/Coroutines/coro-elide-musttail.ll +++ b/llvm/test/Transforms/Coroutines/coro-elide-musttail.ll @@ -13,7 +13,7 @@ @"bar.resumers" = private constant [3 x void (%"bar.Frame"*)*] [void (%"bar.Frame"*)* @"bar.resume", void (%"bar.Frame"*)* undef, void (%"bar.Frame"*)* undef] declare dso_local void @"bar"() align 2 -declare dso_local fastcc void @"bar.resume"(%"bar.Frame"*) align 2 +declare dso_local fastcc void @"bar.resume"(%"bar.Frame"* align 8 dereferenceable(24)) align 2 ; There is a musttail call. ; With alias analysis, we can tell that the frame does not interfere with CALL34, and hence we can keep the tailcalls. diff --git a/llvm/test/Transforms/Coroutines/coro-elide-stat.ll b/llvm/test/Transforms/Coroutines/coro-elide-stat.ll index e92d484..b6e9bac 100644 --- a/llvm/test/Transforms/Coroutines/coro-elide-stat.ll +++ b/llvm/test/Transforms/Coroutines/coro-elide-stat.ll @@ -17,7 +17,7 @@ declare void @print(i32) nounwind ; resume part of the coroutine -define fastcc void @f.resume(i8*) { +define fastcc void @f.resume(i8* dereferenceable(1)) { tail call void @print(i32 0) ret void } diff --git a/llvm/test/Transforms/Coroutines/coro-elide.ll b/llvm/test/Transforms/Coroutines/coro-elide.ll index 674996b..7937b85 100644 --- a/llvm/test/Transforms/Coroutines/coro-elide.ll +++ b/llvm/test/Transforms/Coroutines/coro-elide.ll @@ -7,7 +7,7 @@ declare void @print(i32) nounwind ; resume part of the coroutine -define fastcc void @f.resume(i8*) { +define fastcc void @f.resume(i8* dereferenceable(1)) { tail call void @print(i32 0) ret void } diff --git a/llvm/test/Transforms/Coroutines/coro-heap-elide.ll b/llvm/test/Transforms/Coroutines/coro-heap-elide.ll index 9c6165a..2d8dcd6 100644 --- a/llvm/test/Transforms/Coroutines/coro-heap-elide.ll +++ b/llvm/test/Transforms/Coroutines/coro-heap-elide.ll @@ -11,7 +11,7 @@ declare void @print(i32) nounwind declare void @bar(i8*) -declare fastcc void @f.resume(%f.frame*) +declare fastcc void @f.resume(%f.frame* align 4 dereferenceable(4)) declare fastcc void @f.destroy(%f.frame*) declare fastcc void @f.cleanup(%f.frame*) -- 2.7.4