[llvm] [Utils] Identity map module-level debug info on first use in CloneFunction* (PR #118627)

Felipe de Azevedo Piovezan via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 22 08:34:11 PST 2025


================
@@ -136,6 +138,18 @@ inline RemapFlags operator|(RemapFlags LHS, RemapFlags RHS) {
 /// alternate \a ValueToValueMapTy and \a ValueMaterializer and returns a ID to
 /// pass into the schedule*() functions.
 ///
+/// NOTE: \c IdentityMD is used by CloneFunction* to directly specify metadata
+/// that should be identity mapped (and hence not cloned). The metadata will be
+/// identity mapped in \c VM on first use. There are several reasons for doing
+/// it this way rather than eagerly identity mapping metadata nodes in \c VM:
+/// 1. Mapping metadata is not cheap, particularly because of tracking.
+/// 2. When cloning a Function we identity map lots of global module-level
+///    metadata to avoid cloning it, while only a fraction of it is actually
+///    used by the function. Mapping on first use is a lot faster for modules
+///    with meaningful amount of debug info.
+/// 3. Eagerly identity mapping metadata makes it harder to cache module-level
+///    data (e.g. a set of metadata nodes in a \a DICompileUnit).
+///
----------------
felipepiovezan wrote:

I would not place a comment about Cloning functions inside of the value mapper header; this comment is mostly relevant to "Cloning" users of this abstraction. We would better express this by moving this comment to `Cloning.h`. 

In ValueMapper.h, we should add a brief comment talking about when _users_ of this class should provide an IdentityMD and what it does. In other words, here we should provide guidance for people looking to use ValueMapper. Something short and descriptive like:

> If an IdentityMD set is optionally provided, all Metadata inside this set will be mapped onto itself.

https://github.com/llvm/llvm-project/pull/118627


More information about the llvm-commits mailing list