[PATCH] D31900: [AddDiscriminators] Assign discriminators to memset/memcpy/memmove intrinsic calls.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

In https://reviews.llvm.org/D31900#723288, @danielcdh wrote:

> I don't quite understand why you want to set a new discriminator for memcpy builtin. What optimization would it enable? For normal function calls, we need to have discriminator to distinguish callsites in the same BB so that we can annotate the inlined callee correctly. But for the memcpy case, looks like adding a new discriminator does not help down-stream optimizations like inlining?


Hi Dehao,

This patch is not about enabling more optimization (or more inlining). This is about improving the debug location of loads and stores introduced by SROA when rewriting memory intrinsics.

Basically, SROA knows how to rewrite a memcpy intrinsic as load/store pairs. Each new load/store would obtain the debug location from the original memcpy intrinsic.
However, AddDiscriminators doesn't add a discriminator to intrinsic calls, so the debug location of those loads/stores would not have a discriminator.

Normally, most of those loads and stores don't survive mem2reg. However, there are cases (see memcpy-discriminator.ll) where some loads survive mem2reg.

This is problematic for pass SampleProfile which uses the debug location of instructions to compute block weights.
Essentially, we risk to attribute the wrong block weight to those blocks which contain loads/stores expanded from memory intrinsics.

We have seen this manifest in a relatively hot function of a quite large internal codebase. It led to a suboptimal block placement.

-Andrea


https://reviews.llvm.org/D31900