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

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

In https://reviews.llvm.org/D39287#923553, @rnk wrote:

> 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?


CountingFunctionInserter wouldn't appear in the pipeline twice becaues it runs so late, as part of the codegen passes (which is also why it lived under Codegen/). The sanitizer instrumentation passes avoid the problem by not being in the pipeline by default, but requiring e.g. Clang to insert them.

> 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.

Done.

>> 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.

Great, thanks. (I'll just wait until the Clang side is completely worked out)


https://reviews.llvm.org/D39287