teresajohnson wrote:

> I realized one problem during testing IRPGO (thanks again for the suggestion 
> @minglotus-6 !).
> 
> A function's control flow may change between `-fprofile-generate` and 
> `-fprofile-use` when we make use of definitions in the new header. For 
> example, one may have the following code:
> 
> ```c
> #include "profile/instr_prof_interface.h"
> 
> void main() {
>    ...
>   if (__llvm_profile_dump())
>     return error;
>   
>   cleanup();
> }
> ```
> 
> During `-fprofile-generate`, `__llvm_profile_dump` is a declared name and 
> main's control flow includes a branch to `return error;`. During 
> `-fprofile-use`, `__llvm_profile_dump()` is replaced by `(0)` and the 
> frontend eliminates the `if` statement and the branch to `return error`. Such 
> control flow change can lead to PGO warnings (hash mismatch).
> 
> I think it may be OK to keep the PR this way because the new macros can 
> potentially cause control flow changes directly as well. The documentation is 
> updated 
> (https://github.com/llvm/llvm-project/pull/76471/files#diff-7389be311daf0b9b476c876bef04245fa3c0ad9337ce865682174bd77d53b648R2908)
>  to advise against using these APIs in a program's hot regions and warn about 
> possible impact on control flow.
> 
> Do you all think this is reasonable?

That's probably ok, as it doesn't make sense to do dumping etc in a hot region 
of code anyway. An alternate suggestion is to make these functions real 
functions that simply return 0 instead of #defined to 0. But that may not avoid 
the issue described above because early inlining will likely inline and 
simplify the code before IR PGO matching anyway.


https://github.com/llvm/llvm-project/pull/76471
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to