Skip to content

[RawProfReader]When constructing symbol table, read the MD5 of function name in the proper byte order #76312

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 1 commit into from
Jan 2, 2024

Conversation

mingmingl-llvm
Copy link
Contributor

@mingmingl-llvm mingmingl-llvm commented Dec 24, 2023

Before this patch, when the field NameRef is generated in little-endian systems and read back in big-endian systems, the information gets dropped.

  ; IMPORTS-DAG: Import _Z7callee1v
              ^
<stdin>:1:1: note: scanning from here
main.ll: Import _Z11global_funcv from lib.cc
^
<stdin>:1:10: note: possible intended match here
main.ll: Import _Z11global_funcv from lib.cc
        ^
Input file: <stdin>
Check file: /home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll
-dump-input=help explains the following input dump.
Input was:
<<<<<<
         1: main.ll: Import _Z11global_funcv from lib.cc 
dag:34'0     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
dag:34'1              ?                                    possible intended match

This commit (b399924#diff-b196b796c5a396c7cdf93b347fe47e2b29b72d0b7dd0e2b88abb964d376ee50e) gates the fix by flag and provide test data by creating big-endian profiles (rather than getting a real big-endian system).

  • This is a hexdump of little-endian profile data, and this is the big-endian version of it.
  • The README.md shows the result of llvm-profdata show -ic-targets before and after the fix when the profile is in big-endian.

@llvmbot
Copy link
Member

llvmbot commented Jan 2, 2024

@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-llvm-transforms

Author: Mingming Liu (minglotus-6)

Changes

Before this patch, when the field NameRef is generated in little-endian systems and read back in big-endian systems, the information gets dropped.

This commit (b399924#diff-b196b796c5a396c7cdf93b347fe47e2b29b72d0b7dd0e2b88abb964d376ee50e) gates the fix by flag and provide test data by creating big-endian profiles (rather than getting a real big-endian system).

  • This is a hexdump of little-endian profile data, and this is the big-endian version of it.
  • The README.md shows the result of llvm-profdata show -ic-targets before and after the fix when the profile is in big-endian.

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

2 Files Affected:

  • (modified) llvm/lib/ProfileData/InstrProfReader.cpp (+1-1)
  • (modified) llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll (-3)
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 8f62df79d5b7e8..b547cf7181b16a 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -539,7 +539,7 @@ Error RawInstrProfReader<IntPtrT>::createSymtab(InstrProfSymtab &Symtab) {
     const IntPtrT FPtr = swap(I->FunctionPointer);
     if (!FPtr)
       continue;
-    Symtab.mapAddress(FPtr, I->NameRef);
+    Symtab.mapAddress(FPtr, swap(I->NameRef));
   }
   return success();
 }
diff --git a/llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll b/llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll
index b24effed7024c2..d2f4696ccf41d7 100644
--- a/llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll
+++ b/llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll
@@ -9,9 +9,6 @@
 ; The raw profiles storesd compressed function names, so profile reader should
 ; be built with zlib support to decompress them.
 ; REQUIRES: zlib
-; REQUIRES: host-byteorder-little-endian
-; Raw profiles are generate on 64-bit systems.
-; REQUIRES: llvm-64-bits
 
 ; RUN: rm -rf %t && split-file %s %t && cd %t
 

@mingmingl-llvm
Copy link
Contributor Author

thanks for the quick review @teresajohnson ! As discussed I added a pointer to buildbot in the description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:transforms PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants