[PATCH] D54175: [PGO] context sensitive PGO

Rong Xu via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 8 11:55:40 PST 2018


On Wed, Nov 7, 2018 at 10:10 AM David Li via Phabricator <
reviews at reviews.llvm.org> wrote:

> davidxl added inline comments.
>
>
> ================
> Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:618
> +  // Reserve bit 60-63 for other information purpose.
> +  FunctionHash &= 0x0FFFFFFFFFFFFFFF;
> +  if (IsCS)
> ----------------
> xur wrote:
> > davidxl wrote:
> > > This won't work well -- the non-CS FunctionHash may collide with CS
> hash. How do you differentiate?
> > >
> > > Also doing this for nonCS profiling also breaks the backward
> compatibility -- old profile with high bits set in hash won't be found
> anymore. It may also create more hash conflicts.
> > That is the reason I bump the version number.  I use the version number
> to tell if this is old Hash format or new Hash format.
> Say there exists an old version profile with the 60-63 bits set for some
> functions. When this profile is used, the new compiler won't be able to
> find those functions.
>
> For this we will use one VARIANT bit in header to distinguish.
AnnotateAllFunction will check if the VARIANT flag is set when reading CS
profiles.
For old index profile, this won't be a problem as this bit will not be set.

This could be a problem for old raw profile. As of now, I did not change
instrumentation run-time. So I could only use FuncHash to tell if this
regular count or CS count. Setting of VARIANT bit in index profile relies
on the new FuncHash scheme.

I can add change to profile runtime and set the VARIANT for raw profile.
Then setting the VARIANT bit for index profile will not rely on check of
FuncHash. This will fully backward compatible.

The new FuncHash scheme will be guarded by a check of VARIANT bit. Maybe we
don't need bump the profile version.

Also where is the code that checks the version number?
>

 Right now, It's only in IndexedInstrProfReader::readSummary().


> https://reviews.llvm.org/D54175
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181108/1bb4f1b3/attachment.html>


More information about the llvm-commits mailing list