Skip to content

Commit 1bd3369

Browse files
committed
[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
1 parent 2d26f16 commit 1bd3369

File tree

5 files changed

+20
-27
lines changed

5 files changed

+20
-27
lines changed

llvm/lib/Transforms/Coroutines/CoroElide.cpp

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -103,21 +103,12 @@ static void removeTailCallAttribute(AllocaInst *Frame, AAResults &AA) {
103103

104104
// Given a resume function @f.resume(%f.frame* %frame), returns the size
105105
// and expected alignment of %f.frame type.
106-
static std::pair<uint64_t, Align> getFrameLayout(Function *Resume) {
107-
// Prefer to pull information from the function attributes.
106+
static Optional<std::pair<uint64_t, Align>> getFrameLayout(Function *Resume) {
107+
// Pull information from the function attributes.
108108
auto Size = Resume->getParamDereferenceableBytes(0);
109-
auto Align = Resume->getParamAlign(0);
110-
111-
// If those aren't given, extract them from the type.
112-
if (Size == 0 || !Align) {
113-
auto *FrameTy = Resume->arg_begin()->getType()->getPointerElementType();
114-
115-
const DataLayout &DL = Resume->getParent()->getDataLayout();
116-
if (!Size) Size = DL.getTypeAllocSize(FrameTy);
117-
if (!Align) Align = DL.getABITypeAlign(FrameTy);
118-
}
119-
120-
return std::make_pair(Size, *Align);
109+
if (!Size)
110+
return None;
111+
return std::make_pair(Size, Resume->getParamAlign(0).valueOrOne());
121112
}
122113

123114
// Finds first non alloca instruction in the entry block of a function.
@@ -361,17 +352,19 @@ bool Lowerer::processCoroId(CoroIdInst *CoroId, AAResults &AA,
361352
replaceWithConstant(DestroyAddrConstant, It.second);
362353

363354
if (ShouldElide) {
364-
auto FrameSizeAndAlign = getFrameLayout(cast<Function>(ResumeAddrConstant));
365-
elideHeapAllocations(CoroId->getFunction(), FrameSizeAndAlign.first,
366-
FrameSizeAndAlign.second, AA);
367-
coro::replaceCoroFree(CoroId, /*Elide=*/true);
368-
NumOfCoroElided++;
355+
if (auto FrameSizeAndAlign =
356+
getFrameLayout(cast<Function>(ResumeAddrConstant))) {
357+
elideHeapAllocations(CoroId->getFunction(), FrameSizeAndAlign->first,
358+
FrameSizeAndAlign->second, AA);
359+
coro::replaceCoroFree(CoroId, /*Elide=*/true);
360+
NumOfCoroElided++;
369361
#ifndef NDEBUG
370-
if (!CoroElideInfoOutputFilename.empty())
371-
*getOrCreateLogFile()
372-
<< "Elide " << CoroId->getCoroutine()->getName() << " in "
373-
<< CoroId->getFunction()->getName() << "\n";
362+
if (!CoroElideInfoOutputFilename.empty())
363+
*getOrCreateLogFile()
364+
<< "Elide " << CoroId->getCoroutine()->getName() << " in "
365+
<< CoroId->getFunction()->getName() << "\n";
374366
#endif
367+
}
375368
}
376369

377370
return true;

llvm/test/Transforms/Coroutines/coro-elide-musttail.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
@"bar.resumers" = private constant [3 x void (%"bar.Frame"*)*] [void (%"bar.Frame"*)* @"bar.resume", void (%"bar.Frame"*)* undef, void (%"bar.Frame"*)* undef]
1414

1515
declare dso_local void @"bar"() align 2
16-
declare dso_local fastcc void @"bar.resume"(%"bar.Frame"*) align 2
16+
declare dso_local fastcc void @"bar.resume"(%"bar.Frame"* align 8 dereferenceable(24)) align 2
1717

1818
; There is a musttail call.
1919
; With alias analysis, we can tell that the frame does not interfere with CALL34, and hence we can keep the tailcalls.

llvm/test/Transforms/Coroutines/coro-elide-stat.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
declare void @print(i32) nounwind
1818

1919
; resume part of the coroutine
20-
define fastcc void @f.resume(i8*) {
20+
define fastcc void @f.resume(i8* dereferenceable(1)) {
2121
tail call void @print(i32 0)
2222
ret void
2323
}

llvm/test/Transforms/Coroutines/coro-elide.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
declare void @print(i32) nounwind
88

99
; resume part of the coroutine
10-
define fastcc void @f.resume(i8*) {
10+
define fastcc void @f.resume(i8* dereferenceable(1)) {
1111
tail call void @print(i32 0)
1212
ret void
1313
}

llvm/test/Transforms/Coroutines/coro-heap-elide.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ declare void @print(i32) nounwind
1111

1212
declare void @bar(i8*)
1313

14-
declare fastcc void @f.resume(%f.frame*)
14+
declare fastcc void @f.resume(%f.frame* align 4 dereferenceable(4))
1515
declare fastcc void @f.destroy(%f.frame*)
1616
declare fastcc void @f.cleanup(%f.frame*)
1717

0 commit comments

Comments
 (0)