[llvm] 31e60b9 - [coroutine] should disable inline before calling coro split
Xun Li via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 8 08:54:54 PST 2020
Author: Xun Li
Date: 2020-12-08T08:53:08-08:00
New Revision: 31e60b9133596c185e6daac7f1761dc17e464826
URL: https://github.com/llvm/llvm-project/commit/31e60b9133596c185e6daac7f1761dc17e464826
DIFF: https://github.com/llvm/llvm-project/commit/31e60b9133596c185e6daac7f1761dc17e464826.diff
LOG: [coroutine] should disable inline before calling coro split
This is a rework of D85812, which didn't land.
When callee coroutine function is inlined into caller coroutine function before coro-split pass, llvm will emits "coroutine should have exactly one defining @llvm.coro.begin". It seems that coro-early pass can not handle this quiet well.
So we believe that unsplited coroutine function should not be inlined.
This patch fix such issue by not inlining function if it has attribute "coroutine.presplit" (it means the function has not been splited) to fix this issue
test plan: check-llvm, check-clang
In D85812, there was suggestions on moving the macros to Attributes.td to avoid circular header dependency issue.
I believe it's not worth doing just to be able to use one constant string in one place.
Today, there are already 3 possible attribute values for "coroutine.presplit": https://github.com/llvm/llvm-project/blob/c6543cc6b8f107b58e7205d8fc64865a508bacba/llvm/lib/Transforms/Coroutines/CoroInternal.h#L40-L42
If we move them into Attributes.td, we would be adding 3 new attributes to EnumAttr, just to support this, which I think is an overkill.
Instead, I think the best way to do this is to add an API in Function class that checks whether this function is a coroutine, by checking the attribute by name directly.
Differential Revision: https://reviews.llvm.org/D92706
Added:
llvm/test/Transforms/Coroutines/Inputs/sample.text.prof
llvm/test/Transforms/Coroutines/coro-inline.ll
Modified:
llvm/include/llvm/IR/Function.h
llvm/lib/Analysis/InlineCost.cpp
llvm/lib/Transforms/IPO/AlwaysInliner.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Function.h b/llvm/include/llvm/IR/Function.h
index cf61db1d3347..019e3a98a1af 100644
--- a/llvm/include/llvm/IR/Function.h
+++ b/llvm/include/llvm/IR/Function.h
@@ -268,6 +268,12 @@ class Function : public GlobalObject, public ilist_node<Function> {
getContext(), AttributeList::FunctionIndex, Kind));
}
+ /// A function will have the "coroutine.presplit" attribute if it's
+ /// a coroutine and has not gone through full CoroSplit pass.
+ bool isPresplitCoroutine() const {
+ return hasFnAttribute("coroutine.presplit");
+ }
+
enum ProfileCountType { PCT_Invalid, PCT_Real, PCT_Synthetic };
/// Class to represent profile counts.
diff --git a/llvm/lib/Analysis/InlineCost.cpp b/llvm/lib/Analysis/InlineCost.cpp
index 077b05edcd13..5ee12818e44c 100644
--- a/llvm/lib/Analysis/InlineCost.cpp
+++ b/llvm/lib/Analysis/InlineCost.cpp
@@ -2332,6 +2332,13 @@ Optional<InlineResult> llvm::getAttributeBasedInliningDecision(
if (!Callee)
return InlineResult::failure("indirect call");
+ // When callee coroutine function is inlined into caller coroutine function
+ // before coro-split pass,
+ // coro-early pass can not handle this quiet well.
+ // So we won't inline the coroutine function if it have not been unsplited
+ if (Callee->isPresplitCoroutine())
+ return InlineResult::failure("unsplited coroutine call");
+
// Never inline calls with byval arguments that does not have the alloca
// address space. Since byval arguments can be replaced with a copy to an
// alloca, the inlined code would need to be adjusted to handle that the
diff --git a/llvm/lib/Transforms/IPO/AlwaysInliner.cpp b/llvm/lib/Transforms/IPO/AlwaysInliner.cpp
index cf810dbc3952..532599b42e0d 100644
--- a/llvm/lib/Transforms/IPO/AlwaysInliner.cpp
+++ b/llvm/lib/Transforms/IPO/AlwaysInliner.cpp
@@ -46,7 +46,14 @@ PreservedAnalyses AlwaysInlinerPass::run(Module &M,
SmallSetVector<CallBase *, 16> Calls;
bool Changed = false;
SmallVector<Function *, 16> InlinedFunctions;
- for (Function &F : M)
+ for (Function &F : M) {
+ // When callee coroutine function is inlined into caller coroutine function
+ // before coro-split pass,
+ // coro-early pass can not handle this quiet well.
+ // So we won't inline the coroutine function if it have not been unsplited
+ if (F.isPresplitCoroutine())
+ continue;
+
if (!F.isDeclaration() && F.hasFnAttribute(Attribute::AlwaysInline) &&
isInlineViable(F).isSuccess()) {
Calls.clear();
@@ -90,6 +97,7 @@ PreservedAnalyses AlwaysInlinerPass::run(Module &M,
// invalidation issues while deleting functions.
InlinedFunctions.push_back(&F);
}
+ }
// Remove any live functions.
erase_if(InlinedFunctions, [&](Function *F) {
@@ -182,6 +190,13 @@ InlineCost AlwaysInlinerLegacyPass::getInlineCost(CallBase &CB) {
if (!Callee)
return InlineCost::getNever("indirect call");
+ // When callee coroutine function is inlined into caller coroutine function
+ // before coro-split pass,
+ // coro-early pass can not handle this quiet well.
+ // So we won't inline the coroutine function if it have not been unsplited
+ if (Callee->isPresplitCoroutine())
+ return InlineCost::getNever("unsplited coroutine call");
+
// FIXME: We shouldn't even get here for declarations.
if (Callee->isDeclaration())
return InlineCost::getNever("no definition");
diff --git a/llvm/test/Transforms/Coroutines/Inputs/sample.text.prof b/llvm/test/Transforms/Coroutines/Inputs/sample.text.prof
new file mode 100644
index 000000000000..f2586e5cc976
--- /dev/null
+++ b/llvm/test/Transforms/Coroutines/Inputs/sample.text.prof
@@ -0,0 +1,5 @@
+ff:152730084:141806
+ 1: 123
+
+foo:152730084:141806
+ 65492: ff:302659
diff --git a/llvm/test/Transforms/Coroutines/coro-inline.ll b/llvm/test/Transforms/Coroutines/coro-inline.ll
new file mode 100644
index 000000000000..fc5c1d0d6b1f
--- /dev/null
+++ b/llvm/test/Transforms/Coroutines/coro-inline.ll
@@ -0,0 +1,43 @@
+; RUN: opt < %s -always-inline -barrier -coro-early -barrier -coro-split -S | FileCheck %s
+; RUN: opt < %s -enable-new-pm -always-inline -coro-early -coro-split -S | FileCheck %s
+; RUN: opt < %s -sample-profile-file=%S/Inputs/sample.text.prof -pgo-kind=pgo-sample-use-pipeline -coro-early -barrier -sample-profile -barrier -coro-split -disable-inlining=true -S | FileCheck %s
+; RUN: opt < %s -enable-new-pm -sample-profile-file=%S/Inputs/sample.text.prof -pgo-kind=pgo-sample-use-pipeline -coro-early -sample-profile -coro-split -disable-inlining=true -S | FileCheck %s
+
+; Function Attrs: alwaysinline ssp uwtable
+define void @ff() #0 !dbg !12 {
+entry:
+ %id = call token @llvm.coro.id(i32 16, i8* null, i8* null, i8* null)
+ %begin = call i8* @llvm.coro.begin(token %id, i8* null)
+ ret void
+}
+
+; CHECK: call void @ff()
+; Function Attrs: alwaysinline ssp uwtable
+define void @foo() #0 !dbg !8 {
+entry:
+ %id1 = call token @llvm.coro.id(i32 16, i8* null, i8* null, i8* null)
+ %begin = call i8* @llvm.coro.begin(token %id1, i8* null)
+ call void @ff(), !dbg !11
+ ret void
+}
+
+declare token @llvm.coro.id(i32, i8* readnone, i8* nocapture readonly, i8*)
+declare i8* @llvm.coro.begin(token, i8* writeonly)
+
+attributes #0 = { alwaysinline ssp uwtable "coroutine.presplit"="1" "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="penryn" "target-features"="+cx16,+cx8,+fxsr,+mmx,+sahf,+sse,+sse2,+sse3,+sse4.1,+ssse3,+x87" "unsafe-fp-math"="false" "use-sample-profile" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "inline_O2.cpp", directory: "")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!8 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 46, type: !9, scopeLine: 46, flags: DIFlagPrototyped, unit: !0, retainedNodes: !2)
+!9 = !DISubroutineType(types: !10)
+!10 = !{null}
+!11 = !DILocation(line: 2, column: 0, scope: !8)
+!12 = distinct !DISubprogram(name: "ff", linkageName: "ff", scope: !1, file: !1, line: 46, type: !9, scopeLine: 46, flags: DIFlagPrototyped, unit: !0, retainedNodes: !2)
More information about the llvm-commits
mailing list