[llvm] 44ee9d0 - Revert D85812 "[coroutine] should disable inline before calling coro split"

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 24 11:41:14 PDT 2020


Author: Fangrui Song
Date: 2020-08-24T11:41:05-07:00
New Revision: 44ee9d070adee1aed105f90d88b30b8802d90d35

URL: https://github.com/llvm/llvm-project/commit/44ee9d070adee1aed105f90d88b30b8802d90d35
DIFF: https://github.com/llvm/llvm-project/commit/44ee9d070adee1aed105f90d88b30b8802d90d35.diff

LOG: Revert D85812 "[coroutine] should disable inline before calling coro split"

This reverts commit 2e43acfed89b1903de473f682c65878bdebc395a.

LLVMCoroutines (the library which contains Coroutines.h) depends on LLVMipo (the
library which contains SampleProfile.cpp). It is inappropriate for
SampleProfile.cpp to depent on Coroutines.h (circular dependency).

The test inverted dependencies as well:
llvm/test/Transforms/Coroutines/coro-inline.ll uses -sample-profile.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Coroutines.h
    llvm/lib/Transforms/Coroutines/CoroInternal.h
    llvm/lib/Transforms/IPO/AlwaysInliner.cpp
    llvm/lib/Transforms/IPO/SampleProfile.cpp

Removed: 
    llvm/test/Transforms/Coroutines/Inputs/sample.text.prof
    llvm/test/Transforms/Coroutines/coro-inline.ll


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Coroutines.h b/llvm/include/llvm/Transforms/Coroutines.h
index df1eaf0e48ac..ef05f549fbc1 100644
--- a/llvm/include/llvm/Transforms/Coroutines.h
+++ b/llvm/include/llvm/Transforms/Coroutines.h
@@ -13,18 +13,6 @@
 
 namespace llvm {
 
-// CoroEarly pass marks every function that has coro.begin with a string
-// attribute "coroutine.presplit"="0". CoroSplit pass processes the coroutine
-// twice. First, it lets it go through complete IPO optimization pipeline as a
-// single function. It forces restart of the pipeline by inserting an indirect
-// call to an empty function "coro.devirt.trigger" which is devirtualized by
-// CoroElide pass that triggers a restart of the pipeline by CGPassManager.
-// When CoroSplit pass sees the same coroutine the second time, it splits it up,
-// adds coroutine subfunctions to the SCC to be processed by IPO pipeline.
-#define CORO_PRESPLIT_ATTR "coroutine.presplit"
-#define UNPREPARED_FOR_SPLIT "0"
-#define PREPARED_FOR_SPLIT "1"
-
 class Pass;
 class PassManagerBuilder;
 

diff  --git a/llvm/lib/Transforms/Coroutines/CoroInternal.h b/llvm/lib/Transforms/Coroutines/CoroInternal.h
index e503bd4bf4ff..bd76e93c9124 100644
--- a/llvm/lib/Transforms/Coroutines/CoroInternal.h
+++ b/llvm/lib/Transforms/Coroutines/CoroInternal.h
@@ -26,6 +26,19 @@ void initializeCoroSplitLegacyPass(PassRegistry &);
 void initializeCoroElideLegacyPass(PassRegistry &);
 void initializeCoroCleanupLegacyPass(PassRegistry &);
 
+// CoroEarly pass marks every function that has coro.begin with a string
+// attribute "coroutine.presplit"="0". CoroSplit pass processes the coroutine
+// twice. First, it lets it go through complete IPO optimization pipeline as a
+// single function. It forces restart of the pipeline by inserting an indirect
+// call to an empty function "coro.devirt.trigger" which is devirtualized by
+// CoroElide pass that triggers a restart of the pipeline by CGPassManager.
+// When CoroSplit pass sees the same coroutine the second time, it splits it up,
+// adds coroutine subfunctions to the SCC to be processed by IPO pipeline.
+
+#define CORO_PRESPLIT_ATTR "coroutine.presplit"
+#define UNPREPARED_FOR_SPLIT "0"
+#define PREPARED_FOR_SPLIT "1"
+
 #define CORO_DEVIRT_TRIGGER_FN "coro.devirt.trigger"
 
 namespace coro {

diff  --git a/llvm/lib/Transforms/IPO/AlwaysInliner.cpp b/llvm/lib/Transforms/IPO/AlwaysInliner.cpp
index 504c69ae72c4..53f9512f86f3 100644
--- a/llvm/lib/Transforms/IPO/AlwaysInliner.cpp
+++ b/llvm/lib/Transforms/IPO/AlwaysInliner.cpp
@@ -22,7 +22,6 @@
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/InitializePasses.h"
-#include "llvm/Transforms/Coroutines.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/IPO/Inliner.h"
 #include "llvm/Transforms/Utils/Cloning.h"
@@ -45,14 +44,7 @@ PreservedAnalyses AlwaysInlinerPass::run(Module &M,
   SmallSetVector<CallBase *, 16> Calls;
   bool Changed = false;
   SmallVector<Function *, 16> InlinedFunctions;
-  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.hasFnAttribute(CORO_PRESPLIT_ATTR))
-      continue;
-
+  for (Function &F : M)
     if (!F.isDeclaration() && F.hasFnAttribute(Attribute::AlwaysInline) &&
         isInlineViable(F).isSuccess()) {
       Calls.clear();
@@ -74,7 +66,6 @@ PreservedAnalyses AlwaysInlinerPass::run(Module &M,
       // invalidation issues while deleting functions.
       InlinedFunctions.push_back(&F);
     }
-  }
 
   // Remove any live functions.
   erase_if(InlinedFunctions, [&](Function *F) {
@@ -167,13 +158,6 @@ 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->hasFnAttribute(CORO_PRESPLIT_ATTR))
-    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/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index 864da9ee8aea..2c520a1b5b6b 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -75,7 +75,6 @@
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/GenericDomTree.h"
 #include "llvm/Support/raw_ostream.h"
-#include "llvm/Transforms/Coroutines.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Utils/CallPromotionUtils.h"
@@ -921,14 +920,6 @@ bool SampleProfileLoader::inlineCallInstruction(CallBase &CB) {
 
   Function *CalledFunction = CB.getCalledFunction();
   assert(CalledFunction);
-
-  // 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 (CalledFunction->hasFnAttribute(CORO_PRESPLIT_ATTR))
-    return false;
-
   DebugLoc DLoc = CB.getDebugLoc();
   BasicBlock *BB = CB.getParent();
   InlineParams Params = getInlineParams();

diff  --git a/llvm/test/Transforms/Coroutines/Inputs/sample.text.prof b/llvm/test/Transforms/Coroutines/Inputs/sample.text.prof
deleted file mode 100644
index 5bdcad77416b..000000000000
--- a/llvm/test/Transforms/Coroutines/Inputs/sample.text.prof
+++ /dev/null
@@ -1,5 +0,0 @@
-ff:152730084:141806
-  1: 123
-
-foo:152730084:141806
-  65492: ff:302659
\ No newline at end of file

diff  --git a/llvm/test/Transforms/Coroutines/coro-inline.ll b/llvm/test/Transforms/Coroutines/coro-inline.ll
deleted file mode 100644
index fc5c1d0d6b1f..000000000000
--- a/llvm/test/Transforms/Coroutines/coro-inline.ll
+++ /dev/null
@@ -1,43 +0,0 @@
-; 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