Issue with the function hash used as PGO index

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Issue with the function hash used as PGO index

Fangrui Song via cfe-dev
Hi Folks,

This review

    https://reviews.llvm.org/D79961 

fixes a relatively serious bug in the computation of the function hash used to
compute index in profile files in clang. Basically the bug was allowing trivial
hash collision of two function that only differ by their last statement(s). This
could lead to segfaukts like this one [1].

I'm happy with the patch (if you want to review it, please jump in) but I'm also
worried that it changes the hash of *most* functions, thus invalidating all
profile files generated with previous clang revision.

So if it lands, people will find themselves with a lot of obsolete profile data
that need to be regenerated... I think I'd at least state that clearly on the
mlist, due to the potential large impact.

- Serge


[1] https://bugzilla.redhat.com/show_bug.cgi?id=1827282

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Issue with the function hash used as PGO index

Fangrui Song via cfe-dev
FYI, there is also IR PGO that computes function hash using CFG. To turn it on, use -fprofile-generate and -fprofile-use options.

David

On Thu, May 14, 2020 at 11:59 PM Serge Guelton via llvm-dev <[hidden email]> wrote:
Hi Folks,

This review

    https://reviews.llvm.org/D79961

fixes a relatively serious bug in the computation of the function hash used to
compute index in profile files in clang. Basically the bug was allowing trivial
hash collision of two function that only differ by their last statement(s). This
could lead to segfaukts like this one [1].

I'm happy with the patch (if you want to review it, please jump in) but I'm also
worried that it changes the hash of *most* functions, thus invalidating all
profile files generated with previous clang revision.

So if it lands, people will find themselves with a lot of obsolete profile data
that need to be regenerated... I think I'd at least state that clearly on the
mlist, due to the potential large impact.

- Serge


[1] https://bugzilla.redhat.com/show_bug.cgi?id=1827282

_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev