[PATCH] D130807: [InstrProf] Add the omitprofile attribute

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

Do you expect the difference between `noprofile` and `omitprofile` to be confusing to users? I can certainly see how users could be confused by what the difference is between `noprofile` and `omitprofile` ...

Since what you want to communicate is "never profile"(which `noprofile` can probably communicate as is) and "never profile this, but allow inlining of profiled functions" (which I'm not sure `omitprofile` communicates), then maybe there is a more obvious way we could communicate that? Maybe `neverprofile` and `nodirectprofile` are more descriptive names? I don't love the idea of changing an existing attribute name, but we can transparently upgrade existing uses for backwards compatibility if we have to.  What do you think?

I also think this will need to need to be supported in LLVM's passes, right? So the instrumentation passes in `PGOInstrumentation.cpp` (I can't remember if 'InstrProfiling.cpp` will need this too), and probably the Inlining passes too, right?  I assume those will be follow up patches, but I don't see them in the current stack. Is that an accurate assumption?

BTW I like the direction of these patches, I think adding the ability to differentiate these cases will add a lot of value to the profiling runtimes + coverage. :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130807/new/

https://reviews.llvm.org/D130807