modimo added a comment. Thanks for taking a look!
In D132186#3752150 <https://reviews.llvm.org/D132186#3752150>, @paulkirth wrote: > In D132186#3751985 <https://reviews.llvm.org/D132186#3751985>, @tejohnson > wrote: > >> I have seen a few cases where noinline was used for performance, in addition >> to other cases like avoiding too much stack growth. > > Well, I stand corrected. I'm curious about what these cases are, but in any > case if there are cases where its done, then I agree that a diagnostic would > be helpful. Same. The instances I've seen is an older codebase where compiler optimizations were not as powerful and/or purposefully written by engineers that didn't trust the compiler to do the right thing. >> It is a little different than misexpect though in that the expect hints are >> pretty much only for performance, so it is more useful to be able to issue a >> strong warning that can be turned into an error if they are wrong. And also >> there was no way to report the misuse of expects earlier, unlike inlining >> where we already had the remarks plumbing. >> >> I haven't looked through the patch in detail, but is it possible to use your >> changes to emit a better missed opt remark from the inliner for these cases >> (I assume we will already emit a -Rpass-missed=inline for the noinline >> attribute case, just not highlighting that it is hot and would have been >> inlined for performance reasons otherwise)? I suppose one main reason for >> adding a warning is that the missed inline remarks can be really noisy and >> not really useful to the user vs a compiler optimization engineer doing >> inliner/compiler tuning, and therefore a warning would make it easier to >> turn on more widely as user feedback that can/should be addressed in user >> code. > > Yeah, I was thinking we could emit a new remark type for this to > differentiate, but it seems simpler more user friendly to emit some clar > diagnostic directly. > > I think we’re starting to accumulate a few of these diagnostics now that are > trying to diagnose potential performance deficiencies based on profiling > information. Originally we had prototyped a tool for misexpect based on > libtooling that ran over the build based on the compile commands DB and > reported everything it found. I wonder if reviving that would be useful in > these cases when you want to look for performance issues like this, > misexpect, and other cases? Making ORE diagnostic output queryable through a > tool may also be a good option, but I'm not too familiar with what already > exists in that area. Currently a new ORE (`-pass-remarks=misnoinline`) is getting generated, which misnoexcept also does. Agreed a warning is more familiar and friendlier for users so I lean towards that approach. For additional tooling, I think the first step will be to trial this on more real programs to see what cases are interesting. @iamarchit123 just finished his internship with us so I'll be evaluating these changes on HHVM to see if they can swing the performance needle. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132186/new/ https://reviews.llvm.org/D132186 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
