[PATCH] D39287: Add a flag to disable inlining calls to cygprofile functions generated by -finstrument-functions

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D39287#907578, @hans wrote:

> In https://reviews.llvm.org/D39287#906565, @hfinkel wrote:
>
> > If you only want these calls post-inlining (and otherwise wish to minimize the effect on the rest of the optimizer),
>
>
> As I mentioned in the description, the calls should generally be inserted before inlining and preserved. Doing them post-inlining should be an optional mode that I'm trying to add. This is sort of the crux of the problem, otherwise inserting the instrumentation later would be the obvious way to go.


I understood. I figured that you would add them later in some mode. Otherwise, you would leave the logic as-is. As you suggest, you could have CountingFunctionInserter run both pre and post inlining and then not have duplicate logic.

> 
> 
>> I'd recommend a different approach. What we did for mcount, which has similar requirements, was that we had the frontend add an attribute, and then a late pass actually added the function calls (lib/CodeGen/CountingFunctionInserter.cpp). I recommend extending CountingFunctionInserter to enable it to add these functions (based on some additional attribute), and then just add whatever attribute is necessary in the frontend based on some new flag.
> 
> Would it be OK to run CountingFunctionInserter both before and after inlining and inserting these calls depending on some flag?

I'm not sure the logic of inserting the calls is complex enough to worry about reducing the duplication of having both logic in the frontend and in CountingFunctionInserter, but if you feel that it is, then I think this makes sense.


https://reviews.llvm.org/D39287