Skip to content

[CGData] Fix assertions when skipping name #151570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 31, 2025
Merged

Conversation

kyulee-com
Copy link
Contributor

This fixes assertion failures when using -indexed-codegen-data-read-function-map-names=false

This fixes assertion failures when using `-indexed-codegen-data-read-function-map-names=false`
@llvmbot llvmbot added the LTO Link time optimization (regular/full LTO or ThinLTO) label Jul 31, 2025
@kyulee-com kyulee-com requested a review from ellishg July 31, 2025 18:04
@llvmbot
Copy link
Member

llvmbot commented Jul 31, 2025

@llvm/pr-subscribers-lto

Author: Kyungwoo Lee (kyulee-com)

Changes

This fixes assertion failures when using -indexed-codegen-data-read-function-map-names=false


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

2 Files Affected:

  • (modified) llvm/lib/CGData/StableFunctionMapRecord.cpp (+10-4)
  • (modified) llvm/test/ThinLTO/AArch64/cgdata-merge-read.ll (+16)
diff --git a/llvm/lib/CGData/StableFunctionMapRecord.cpp b/llvm/lib/CGData/StableFunctionMapRecord.cpp
index 4e4fceffb52b5..0c060bdbf27bc 100644
--- a/llvm/lib/CGData/StableFunctionMapRecord.cpp
+++ b/llvm/lib/CGData/StableFunctionMapRecord.cpp
@@ -162,12 +162,18 @@ void StableFunctionMapRecord::deserialize(const unsigned char *&Ptr,
         endian::readNext<stable_hash, endianness::little, unaligned>(Ptr);
     auto FunctionNameId =
         endian::readNext<uint32_t, endianness::little, unaligned>(Ptr);
-    assert(FunctionMap->getNameForId(FunctionNameId) &&
-           "FunctionNameId out of range");
+    (void)FunctionNameId;
     auto ModuleNameId =
         endian::readNext<uint32_t, endianness::little, unaligned>(Ptr);
-    assert(FunctionMap->getNameForId(ModuleNameId) &&
-           "ModuleNameId out of range");
+    (void)ModuleNameId;
+    // Only validate IDs if we've read the names
+    if (ReadStableFunctionMapNames) {
+      assert(FunctionMap->getNameForId(FunctionNameId) &&
+             "FunctionNameId out of range");
+      assert(FunctionMap->getNameForId(ModuleNameId) &&
+             "ModuleNameId out of range");
+    }
+
     auto InstCount =
         endian::readNext<uint32_t, endianness::little, unaligned>(Ptr);
 
diff --git a/llvm/test/ThinLTO/AArch64/cgdata-merge-read.ll b/llvm/test/ThinLTO/AArch64/cgdata-merge-read.ll
index da756e7f15e68..37113028240e8 100644
--- a/llvm/test/ThinLTO/AArch64/cgdata-merge-read.ll
+++ b/llvm/test/ThinLTO/AArch64/cgdata-merge-read.ll
@@ -1,3 +1,5 @@
+; REQUIRES: asserts
+
 ; This test demonstrates how similar functions are handled during global outlining.
 ; Currently, we do not attempt to share an merged function for identical sequences.
 ; Instead, each merging instance is created uniquely.
@@ -30,6 +32,20 @@
 ; RUN: llvm-objdump -d %tout-read.1 | FileCheck %s --check-prefix=THUNK1
 ; RUN: llvm-objdump -d %tout-read.2 | FileCheck %s --check-prefix=THUNK2
 
+; It runs the same if we use -indexed-codegen-data-read-function-map-names=false.
+; RUN: llvm-lto2 run -enable-global-merge-func=true \
+; RUN:    -indexed-codegen-data-read-function-map-names=false \
+; RUN:    -codegen-data-use-path=%tout.cgdata \
+; RUN:    %t-foo.bc %t-goo.bc -o %tout-read \
+; RUN:    -r %t-foo.bc,_f1,px \
+; RUN:    -r %t-goo.bc,_f2,px \
+; RUN:    -r %t-foo.bc,_g,l -r %t-foo.bc,_g1,l -r %t-foo.bc,_g2,l \
+; RUN:    -r %t-goo.bc,_g,l -r %t-goo.bc,_g1,l -r %t-goo.bc,_g2,l
+; RUN: llvm-nm %tout-read.1 | FileCheck %s --check-prefix=READ1
+; RUN: llvm-nm %tout-read.2 | FileCheck %s --check-prefix=READ2
+; RUN: llvm-objdump -d %tout-read.1 | FileCheck %s --check-prefix=THUNK1
+; RUN: llvm-objdump -d %tout-read.2 | FileCheck %s --check-prefix=THUNK2
+
 ; READ1: _f1.Tgm
 ; READ2: _f2.Tgm
 

@kyulee-com
Copy link
Contributor Author

@ellishg Thanks for comments which I addressed!

@kyulee-com kyulee-com merged commit d4e8619 into llvm:main Jul 31, 2025
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTO Link time optimization (regular/full LTO or ThinLTO)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants