Skip to content

Fix MemProf imported alias handling after guid metadata changes#200430

Merged
teresajohnson merged 1 commit into
llvm:mainfrom
teresajohnson:memprof_guid_fix
May 29, 2026
Merged

Fix MemProf imported alias handling after guid metadata changes#200430
teresajohnson merged 1 commit into
llvm:mainfrom
teresajohnson:memprof_guid_fix

Conversation

@teresajohnson
Copy link
Copy Markdown
Contributor

After PR184065 was committed, memprof ThinLTO builds were failing on
imported aliases, which now have the original aliasee guid attached
as metadata (we import aliases as a copy of the aliasee body). In
distributed ThinLTO, unless also importing the aliasee symbol, we won't
have an entry in the summary for the aliasee guid. And we now don't have
a way to locate the alias summary, which caused some assumptions and
assertions to fail.

Work around this with a TODO to add a way to find the original alias
guid.

After PR184065 was committed, memprof ThinLTO builds were failing on
imported aliases, which now have the original aliasee guid attached
as metadata (we import aliases as a copy of the aliasee body). In
distributed ThinLTO, unless also importing the aliasee symbol, we won't
have an entry in the summary for the aliasee guid. And we now don't have
a way to locate the alias summary, which caused some assumptions and
assertions to fail.

Work around this with a TODO to add a way to find the original alias
guid.
@teresajohnson teresajohnson requested review from mtrofin and orodley May 29, 2026 15:26
@llvmorg-github-actions llvmorg-github-actions Bot added LTO Link time optimization (regular/full LTO or ThinLTO) llvm:transforms labels May 29, 2026
@llvmorg-github-actions
Copy link
Copy Markdown

llvmorg-github-actions Bot commented May 29, 2026

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-lto

Author: Teresa Johnson (teresajohnson)

Changes

After PR184065 was committed, memprof ThinLTO builds were failing on
imported aliases, which now have the original aliasee guid attached
as metadata (we import aliases as a copy of the aliasee body). In
distributed ThinLTO, unless also importing the aliasee symbol, we won't
have an entry in the summary for the aliasee guid. And we now don't have
a way to locate the alias summary, which caused some assumptions and
assertions to fail.

Work around this with a TODO to add a way to find the original alias
guid.


Full diff: https://github.com/llvm/llvm-project/pull/200430.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp (+12-1)
  • (added) llvm/test/ThinLTO/X86/memprof-imported-alias-assert.ll (+64)
diff --git a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
index 015b6bc5caf9f..3cc35877d2add 100644
--- a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
+++ b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
@@ -5899,9 +5899,20 @@ bool MemProfContextDisambiguation::applyImport(Module &M) {
           break;
         }
       }
-      assert(GVSummary && GVSummary->modulePath() == SrcModule);
+      // TODO: Put back the assert once we have metadata on imported copies of
+      // aliases linking them back to the original alias GUID, which would allow
+      // us to locate the alias summary here.
+      // assert(GVSummary && GVSummary->modulePath() == SrcModule);
     }
 
+    // GVSummary can be null if this is a function imported as a copy of an
+    // alias, and we don't have the aliasee's summary in our distributed index.
+    // TODO: Once we can locate the original GUID for imported aliases (e.g. via
+    // TBD additional metadata), we should find the alias summary instead, and
+    // we can remove this check and fall back to the original check below.
+    if (!GVSummary)
+      continue;
+
     // If this was an imported alias skip it as we won't have the function
     // summary, and it should be cloned in the original module.
     if (isa<AliasSummary>(GVSummary))
diff --git a/llvm/test/ThinLTO/X86/memprof-imported-alias-assert.ll b/llvm/test/ThinLTO/X86/memprof-imported-alias-assert.ll
new file mode 100644
index 0000000000000..964659af7dcce
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/memprof-imported-alias-assert.ll
@@ -0,0 +1,64 @@
+;; Test that we do not assert/crash in distributed ThinLTO when an alias is
+;; imported as a copy of the aliasee, but the aliasee summary is not in the
+;; distributed index.
+
+; REQUIRES: asserts
+
+; RUN: rm -rf %t && split-file %s %t && cd %t
+; RUN: opt -thinlto-bc src1.ll >src1.o
+; RUN: opt -thinlto-bc src2.ll >src2.o
+; RUN: llvm-lto2 run src1.o src2.o -enable-memprof-context-disambiguation \
+; RUN:  -supports-hot-cold-new \
+; RUN:  -thinlto-distributed-indexes \
+; RUN:  -r=src1.o,main,plx \
+; RUN:  -r=src1.o,alias, \
+; RUN:  -r=src2.o,alias,pl \
+; RUN:  -r=src2.o,aliasee,pl \
+; RUN:  -r=src2.o,_Znam, \
+; RUN:  -o %t.out
+
+;; ThinLTO backend for the importing module. Make sure this succeeds.
+; RUN: opt -import-all-index -passes=function-import,memprof-context-disambiguation \
+; RUN:  -summary-file=src1.o.thinlto.bc \
+; RUN:  -memprof-import-summary=src1.o.thinlto.bc \
+; RUN:  -enable-import-metadata \
+; RUN:  -stats -pass-remarks=memprof-context-disambiguation \
+; RUN:  src1.o -S 2>&1 | FileCheck %s
+
+; CHECK: define available_externally hidden ptr @alias(ptr %{{.*}})
+
+;--- src1.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @main() {
+entry:
+  %call = call ptr @alias(ptr null), !callsite !0
+  ret i32 0
+}
+
+declare ptr @alias(ptr)
+
+!0 = !{i64 8632435727821051414}
+
+;--- src2.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@alias = hidden alias ptr (ptr), ptr @aliasee
+
+define ptr @aliasee(ptr %0) !guid !1 {
+entry:
+  %call = call ptr @_Znam(i64 10), !memprof !2, !callsite !7
+  ret ptr null
+}
+
+declare ptr @_Znam(i64)
+
+!1 = !{i64 6174544574081827517}
+!2 = !{!3, !5}
+!3 = !{!4, !"notcold"}
+!4 = !{i64 9086428284934609951, i64 8632435727821051414}
+!5 = !{!6, !"cold"}
+!6 = !{i64 9086428284934609951, i64 -3421689549917153178}
+!7 = !{i64 9086428284934609951}

@teresajohnson teresajohnson merged commit 3330752 into llvm:main May 29, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:transforms LTO Link time optimization (regular/full LTO or ThinLTO)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants