[PATCH] D54175: [PGO] context sensitive PGO

[Rong Xu via llvm-commits]

We have only 64 bit in the hash. There could be some collisions. We were
thinking they are only the corners cases.

I discussed with David Li about the potentially increased chance of
collisions for this change. We were thinking to expand the hash, but that
would need some non-trivial change.
The hash needs to be expanded anyway because currently we only encode
indirect-call counters in the hash and leave out other value-profile
counters.

For this case, I think we need to improve the code to handle the
already happened collision. We could print an warning and result instead of
assert.

I will send a patch to this.


On Mon, Apr 1, 2019 at 9:00 PM Peter Collingbourne via Phabricator <
reviews at reviews.llvm.org> wrote:

> pcc added inline comments.
>
>
> ================
> Comment at:
> llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:673
> +  // Reserve bit 60-63 for other information purpose.
> +  FunctionHash &= 0x0FFFFFFFFFFFFFFF;
> +  if (IsCS)
> ----------------
> Hi Rong, I discovered a problem with this line of code. If the number of
> select instructions when the profile was collected differs by a multiple of
> 16 from the number when the profile is used, we get a hash collision which
> results in us hitting an assertion here:
>
> http://llvm-cs.pcc.me.uk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#1042
>
> Granted, the same problem seems to have already existed for differences
> that are multiples of 256, but this seems to make the problem more likely
> to occur.
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D54175/new/
>
> https://reviews.llvm.org/D54175
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190402/63c985fb/attachment.html>