[all-commits] [llvm/llvm-project] 35c7e4: [MemProf] Fix inline propagation of memprof metadata

[Teresa Johnson via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 35c7e457e8b00a4772b00b0100748f9a517ea0c2
      https://github.com/llvm/llvm-project/commit/35c7e457e8b00a4772b00b0100748f9a517ea0c2
  Author: Teresa Johnson <tejohnson at google.com>
  Date:   2022-12-30 (Fri, 30 Dec 2022)

  Changed paths:
    M llvm/lib/Transforms/Utils/InlineFunction.cpp
    M llvm/test/Transforms/Inline/memprof_inline.ll
    M llvm/test/Transforms/Inline/memprof_inline2.ll

  Log Message:
  -----------
  [MemProf] Fix inline propagation of memprof metadata

It isn't correct to always remove memprof metadata MIBs from the
original allocation call after inlining.

Let's say we have the following partial call graph:

C     D
 \   /
  v v
   B   E
   |  /
   v v
    A

where A contains an allocation call. If both contexts including B have
the same allocation behavior, the context in the memprof metadata on the
allocation will be pruned, and we will have 2 MIBs with contexts:
A,B and A,E.

Previously, if we inlined A into B we propagate the matching MIBs onto
the inlined allocation call in B' (A,B in this case), and remove it from
the original out of line allocation in A. This is correct if we have a
single round of bottom up inlining.

However, in the compiler we can have multiple invocations of the inliner
pass (e.g. LTO). We may also inline non-bottom up with an alternative
inliner such as the ModuleInliner. In that case, we could end up first
inlining B into C, without having inlined A into B. The call graph then
looks like:

    D
    |
    v
C'  B   E
 \  |  /
  v v v
    A

If we subsequently (perhaps on a later invocation of bottom up inlining)
inline A into B, the previous handling would propagate the memprof MIB
context A,B up into the inlined allocation in B', and remove it from the
original allocation in A. The propagation into B' is fine, however, by
removing it from A's allocation, we no longer reflect the context coming
from C'.

To fix this, simply prevent the removal of MIB from the original
allocation callsites.

Note that the memprof_inline.ll test has some changes to existing
checking to replace "noncold" with "notcold" in the metadata. The
corresponding CHECK was accidentally commented out in the old version
and thus this mistake was not previously detected.

Differential Revision: https://reviews.llvm.org/D140764