[PATCH] D39287: Use CountingFunctionInserter both mcount and cygprofile calls, before and after inlining

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

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

> Rebasing and addressing comments.
>
> I've also added the pass in PassManagerBuilder::populateFunctionPassManager() because I realized that's how Clang sets up the passes.
>
> One thing that worries me is that this set-up is pretty fragile against how the passes are run. I think it will currently work, but if the pass were to somehow run more than once, the instrumentation could get inserted multiple times. One way to avoid that would be to remove the attribute (or probably faster, give it an empty value?) when the pass "consumes" the attribute. Does that sound reasonable?


What do other instrumentation passes do to defend against this? I guess they just don't have this problem because they don't appear twice in the pipeline?

I'm in favor of removing the function attribute when we instrument. AttributeLists are immutable, so nuking the whole attribute should be just as fast as setting the value.

> I also looked into consolidating this with the FEntryInserter pass which inserts calls to __fentry__. That turned out to be tricky however, because the calls are supposed to be inserted before the function prologue, which means the pass operates on MachineFunctions and inserts MachineInstrs. So I've left that out.

Oh, wow, that's pretty special functionality. Seems best to leave it alone.

> I've updated the Clang-side patch here: https://reviews.llvm.org/D39331

I think this is pretty uncontroversial, so let's remove the attribute on instrumentation, land this, and we can address any feedback from Hal or others in follow-ups.


https://reviews.llvm.org/D39287