[PATCH] D131592: [SampleProfile] Fix non-determinism in promoteMergeNotInlinedContextSamples()

Wenlei He via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 12 08:23:33 PDT 2022


wenlei added a comment.

In D131592#3716756 <https://reviews.llvm.org/D131592#3716756>, @rnk wrote:

> I would argue that one doesn't need a test case for this. It is essentially dangerous to iterate a DenseMap and do transforms.
>
> Is it possible to construct a test that fails in reverse iteration mode <https://lists.llvm.org/pipermail/llvm-dev/2017-July/115025.html>? Or, does an existing test fail in reverse iteration mode? If so, you can use that for test coverage instead.

Right, fixing non-determinism doesn't really need a test case. And the change itself is good.

The ask for example is more for us to understand whether there's other bug exposed by the non-determinism. Remarks printing aside, the use in the iteration should not be order sensitive, so it's not obvious how different iteration order would change branch weights and there could be another bug somewhere..


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131592/new/

https://reviews.llvm.org/D131592



More information about the llvm-commits mailing list