[all-commits] [llvm/llvm-project] 594e11: [MemProf] Avoid incorrect ICP symtab canonicalizat...

[Teresa Johnson via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 594e11ce4247feb3197dc3cf0da331e96f9a098b
      https://github.com/llvm/llvm-project/commit/594e11ce4247feb3197dc3cf0da331e96f9a098b
  Author: Teresa Johnson <tejohnson at google.com>
  Date:   2024-11-07 (Thu, 07 Nov 2024)

  Changed paths:
    M llvm/include/llvm/ProfileData/InstrProf.h
    M llvm/lib/ProfileData/InstrProf.cpp
    M llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
    M llvm/test/ThinLTO/X86/memprof-icp.ll

  Log Message:
  -----------
  [MemProf] Avoid incorrect ICP symtab canonicalization (#115419)

ICP builds a symtab from the symbols in the module allowing mapping from
the VP metadata GUIDs to the Function. MemProf uses this same symtab
handling for its ICP during cloning. When symbols are added to the
symtab, the handling adds both a GUID computed from the function name,
or from the attached PGOFuncName metadata for locals, as well as a GUID
computed from the "canonicalized" name, which strips all "." suffixes
other than ".__uniq". This was originally meant to remove the ".llvm.*"
suffix added to promoted locals (done earlier in the ThinLTO backend).
In theory, it should no longer be needed as locals should have
PGOFuncName metadata.

However, this was causing a linker unsat, in code that used coroutines.
For an original coroutine function, there were several additional
functions created that had the same name, but different "." suffixes.
Therefore the canonical name for these additional functions had the same
GUID as that of the original function, leading to extra entries in the
symtab, and to selecting the wrong function for promotion. For regular
ICP this can happen, but is just a performance issue. However, for
memprof the promoted direct call calls a memprof clone, and because we
called the wrong function, in this case it didn't have a memprof clone
and we got a linker unsat.

We may be able to remove the canonical name handling for ICP in general,
but for now disable it for MemProf. At worst this could lead to not
finding a GUID in the symtab and not performing an ICP, so should be
conservatively correct.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications