[CoroElide] Remove fallback for frame layout determination
authorNikita Popov <npopov@redhat.com>
Fri, 4 Mar 2022 12:07:58 +0000 (13:07 +0100)
committerNikita Popov <npopov@redhat.com>
Mon, 7 Mar 2022 10:23:02 +0000 (11:23 +0100)
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
llvm/test/Transforms/Coroutines/coro-elide-musttail.ll
llvm/test/Transforms/Coroutines/coro-elide-stat.ll
llvm/test/Transforms/Coroutines/coro-elide.ll
llvm/test/Transforms/Coroutines/coro-heap-elide.ll

index 84bebb7..1e37f8c 100644 (file)
@@ -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<uint64_t, Align> getFrameLayout(Function *Resume) {
-  // Prefer to pull information from the function attributes.
+static Optional<std::pair<uint64_t, Align>> 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<Function>(ResumeAddrConstant));
-    elideHeapAllocations(CoroId->getFunction(), FrameSizeAndAlign.first,
-                         FrameSizeAndAlign.second, AA);
-    coro::replaceCoroFree(CoroId, /*Elide=*/true);
-    NumOfCoroElided++;
+    if (auto FrameSizeAndAlign =
+            getFrameLayout(cast<Function>(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;
index 63b7aa1..a25f96b 100644 (file)
@@ -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.
index e92d484..b6e9bac 100644 (file)
@@ -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
 }
index 674996b..7937b85 100644 (file)
@@ -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
 }
index 9c6165a..2d8dcd6 100644 (file)
@@ -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*)