[PATCH] D22666: Frontend: Fix mcount inlining bug

[Hal Finkel via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D22666#493706, @honggyu.kim wrote:

> In https://reviews.llvm.org/D22666#493109, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D22666#493026, @hfinkel wrote:
> >
> > > What's the actual desired behavior here? Should the inliner strip out calls to mcount() as it inlines?
> >
> >
> > I think they basically just want a late pass (as in immediately prior to codegen) to insert the mcount calls.
>
>
> You're right. I wanted to move mcount insertion phase myself but it seems not that simple as I expected because mcount call is inserted in bitcode anyway, then can be optimized using `opt` tool to do function inlining later on. In this case, function inliner must strip mcount calls at the entry of each function.
>
> Otherwise, we shouldn't generate mcount calls inside bitcode so that `opt` can simply do function inlining as it does, then code generator, `llc`, has to insert mcount just before code generation is done.  I think this is not that simple as well.
>
> To sum up, we have two approaches.
>
> 1. inliner must strip mcount calls when it does function inlining.
> 2. `llc` has to insert mcount calls just before code generation.
>
>   We have to choose one of these approaches. But I think this patch fixes the problem in a simpler way as of now.


But it will also severely hurt performance, and that's not good either.

Adding an early IR-level pass to llc to insert calls to mcount seems straightforward. We could, for example, add some function attribute in the frontend which causes the pass to actually insert the mcount calls. There is some frontend knowledge currently regarding exactly what the mcount function is named (lib/Basic/Targets.cpp sets the MCountName variable) that we'd either need to pass in the attribute or move to the backend.

Another option is to add a new function attribute, drop_when_inlining (modulo bikeshedding), add that to the mcount function in the frontend, and teach the inliner to drop functions with that attribute when inlining.


https://reviews.llvm.org/D22666