[PATCH] D23299: [PM] Port the always inliner to the new pass manager in a much more minimal and boring form than the old pass manager's version.

[Philip Reames via llvm-commits]

On 08/09/2016 02:03 AM, Chandler Carruth via llvm-commits wrote:
> chandlerc created this revision.
> chandlerc added a subscriber: llvm-commits.
> Herald added a reviewer: tstellarAMD.
> Herald added subscribers: mcrosier, arsenm.
>
> This pass does the very minimal amount of work necessary to inline
> functions declared as always-inline. It doesn't support a wide array of
> things that the legacy pass manager did support, but is alse ... about
> 20 lines of code. So it has that going for it. Notably things this
> doesn't support:
>
> - Array alloca merging
>    - To support the above, bottom-up inlining with careful history
>      tracking and call graph updates
> - DCE of the functions that become dead after this inlining.
> - Inlining through call instructions with the always_inline attribute.
>    Instead, it focuses on inlining functions with that attribute.
>
> The first I've omitted because I'm hoping to just turn it off for the
> primary pass manager. If that doesn't pan out, I can add it here but it
> will be reasonably expensive to do so.
>
> The second should really be handled by running global-dce after the
> inliner. I don't want to re-implement the non-trivial logic necessary to
> do comdat-correct DCE of functions. This means the -O0 pipeline will
> have to be at least 'always-inline,global-dce', but that seems
> reasonable to me. If others are seriously worried about this I'd like to
> heard and understand why. Again, this is all solveable by factoring that
> logic into a utility and calling it here, but I'd like to wait to do
> that until there is a clear reason why the existing pass-based factoring
> won't work.
>
> The final point is a serious one. I can fairly easily add support for
> this, but it seems both costly and a confusing construct for the use
> case of the always inliner running at O0. This attribute can of course
> still impact the normal inliner easily (although I find that
> a questionable re-use of the same attribute). I've started a discussion
> to sort out what semantics we want here and based on that can figure out
> if it makes sense ta have this complexity at O0 or not.
Where is this discussion?  I may be missing the obvious, but I don't see 
a thread on llvm-dev.

This bit is definitely problematic for me.  I understand the desire, but 
the ability to mark call sites (separately from functions) as 
always_inline seems like a really major feature to just drop.  I will 
argue strongly against it.
>
> One other advantage of this design is that it should be quite a bit
> faster due to checking for whether the function is a viable candidate
> for inlining exactly once per function instead of doing it for each call
> site.
>
> Anyways, hopefully a reasonable starting point for this pass.
>
> https://reviews.llvm.org/D23299
>
> Files:
>    include/llvm/InitializePasses.h
>    include/llvm/LinkAllPasses.h
>    include/llvm/Transforms/IPO.h
>    include/llvm/Transforms/IPO/AlwaysInliner.h
>    lib/Passes/PassBuilder.cpp
>    lib/Passes/PassRegistry.def
>    lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
>    lib/Transforms/IPO/AlwaysInliner.cpp
>    lib/Transforms/IPO/CMakeLists.txt
>    lib/Transforms/IPO/IPO.cpp
>    lib/Transforms/IPO/InlineAlways.cpp
>    test/Transforms/Inline/always-inline.ll
>    tools/bugpoint/bugpoint.cpp
>    tools/opt/opt.cpp
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160820/1055a080/attachment.html>