[PATCH] D106448: [llvm][Inline] Add a module level inliner

[Kazu Hirata via Phabricator via llvm-commits]

kazu added inline comments.


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:113
 #include "llvm/Transforms/IPO/MergeFunctions.h"
+#include "llvm/Transforms/IPO/ModuleInliner.h"
 #include "llvm/Transforms/IPO/OpenMPOpt.h"
----------------
You should be able to remove this line if you are not otherwise changing `PassBuilder.cpp` at all.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:683-684
 
+  if (EnableModuleInliner)
+    IP.EnableDeferral = false;
+
----------------
Why do you need to do this?  Performance reasons?  Correctness?


================
Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:1024-1029
+  if (UseModuleInliner) {
+    if (MandatoryFirst)
+      MPM.addPass(ModuleInlinerPass(/*OnlyMandatory*/ true));
+    MPM.addPass(ModuleInlinerPass());
+    return;
+  }
----------------
This is a strange place to add your module inliner.  `ModuleInlinerWrapperPass` performs:

```
for each SCC in the bottom-up order:
  Call the inliner on the SCC
  Run cleanup passes on the SCC
```

Now, if `UseModuleInliner` is true, we don't add InlinerPass (see a few lines below), so the loop above essentially becomes:

```
for each SCC in the bottom-up order:
  Run cleanup passes on the SCC
```

If we are not inlining functions in the "for each" loop, we don't need to visit each SCC in the bottom-up order.

You might consider adding your module inliner just before:

```
  MPM.addPass(buildInlinerPipeline(Level, Phase));
```

at `PassBuilderPipelines.cpp:891` with an appropriate guard like `if (UseModuleInliner)`.

This way, you don't need to touch the existing inliner at all.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106448/new/

https://reviews.llvm.org/D106448