Reland "[PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. " #75860
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… use semicolon as delimiter for local-linkage varibles." (llvm#75835) This reverts commit 3aa5d71.
llvmbot
added
compiler-rt
PGO
|
@llvm/pr-subscribers-lto @llvm/pr-subscribers-pgo Author: Mingming Liu (minglotus-6) ChangesFixed build-bot failures caught by post-submit tests
Original commit message This patch changes To elaborate on the scenario how things break without this PR
Patch is 44.32 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/75860.diff 18 Files Affected:
diff --git a/compiler-rt/test/profile/CMakeLists.txt b/compiler-rt/test/profile/CMakeLists.txt
index 975e4c42f4b640..eebe0469efebe0 100644
--- a/compiler-rt/test/profile/CMakeLists.txt
+++ b/compiler-rt/test/profile/CMakeLists.txt
@@ -6,7 +6,7 @@ set(PROFILE_TESTSUITES)
set(PROFILE_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS} compiler-rt-headers)
list(APPEND PROFILE_TEST_DEPS profile)
if(NOT COMPILER_RT_STANDALONE_BUILD)
- list(APPEND PROFILE_TEST_DEPS llvm-profdata llvm-cov)
+ list(APPEND PROFILE_TEST_DEPS llvm-cov llvm-dis llvm-lto llvm-profdata opt)
if(NOT APPLE AND COMPILER_RT_HAS_LLD AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
list(APPEND PROFILE_TEST_DEPS lld)
endif()
diff --git a/compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp b/compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp
new file mode 100644
index 00000000000000..82ca1cd7d0a564
--- /dev/null
+++ b/compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp
@@ -0,0 +1,115 @@
+// This is a regression test for ThinLTO indirect-call-promotion when candidate
+// callees need to be imported from another IR module. In the C++ test case,
+// `main` calls `global_func` which is defined in another module. `global_func`
+// has two indirect callees, one has external linkage and one has local linkage.
+// All three functions should be imported into the IR module of main.
+
+// What the test does:
+// - Generate raw profiles from executables and convert it to indexed profiles.
+// During the conversion, a profiled callee address in raw profiles will be
+// converted to function hash in indexed profiles.
+// - Run IRPGO profile use and ThinTLO prelink pipeline and get LLVM bitcodes
+// for both cpp files in the C++ test case.
+// - Generate ThinLTO summary file with LLVM bitcodes, and run `function-import` pass.
+// - Run `pgo-icall-prom` pass for the IR module which needs to import callees.
+
+// Use lld as linker for more robust test. We need to REQUIRE LLVMgold.so for
+// LTO if default linker is GNU ld or gold anyway.
+// REQUIRES: lld-available
+
+// Test should fail where linkage-name and mangled-name diverges, see issue https://github.com/llvm/llvm-project/issues/74565).
+// Currently, this name divergence happens on Mach-O object file format, or on
+// many (but not all) 32-bit Windows systems.
+//
+// XFAIL: system-darwin
+//
+// Mark 32-bit Windows as UNSUPPORTED for now as opposed to XFAIL. This test
+// should fail on many (but not all) 32-bit Windows systems and succeed on the
+// rest. The flexibility in triple string parsing makes it tricky to capture
+// both sets accurately. i[3-9]86 specifies arch as Triple::ArchType::x86, (win32|windows)
+// specifies OS as Triple::OS::Win32
+//
+// UNSUPPORTED: target={{i.86.*windows.*}}
+
+// RUN: rm -rf %t && split-file %s %t && cd %t
+
+// Do setup work for all below tests.
+// Generate raw profiles from real programs and convert it into indexed profiles.
+// Use clangxx_pgogen for IR level instrumentation for C++.
+// RUN: %clangxx_pgogen -fuse-ld=lld -O2 lib.cpp main.cpp -o main
+// RUN: env LLVM_PROFILE_FILE=main.profraw %run ./main
+// RUN: llvm-profdata merge main.profraw -o main.profdata
+
+// Use profile on lib and get bitcode, test that local function callee0 has
+// expected !PGOFuncName metadata and external function callee1 doesn't have
+// !PGOFuncName metadata. Explicitly skip ICP pass to test ICP happens as
+// expected in the IR module that imports functions from lib.
+// RUN: %clang -mllvm -disable-icp -fprofile-use=main.profdata -flto=thin -O2 -c lib.cpp -o lib.bc
+// RUN: llvm-dis lib.bc -o - | FileCheck %s --check-prefix=PGOName
+
+// Use profile on main and get bitcode.
+// RUN: %clang -fprofile-use=main.profdata -flto=thin -O2 -c main.cpp -o main.bc
+
+// Run llvm-lto to get summary file.
+// RUN: llvm-lto -thinlto -o summary main.bc lib.bc
+
+// Test the imports of functions. Default import thresholds would work but do
+// explicit override to be more futureproof. Note all functions have one basic
+// block with a function-entry-count of one, so they are actually hot functions
+// per default profile summary hotness cutoff.
+// RUN: opt -passes=function-import -import-instr-limit=100 -import-cold-multiplier=1 -summary-file summary.thinlto.bc main.bc -o main.import.bc -print-imports 2>&1 | FileCheck %s --check-prefix=IMPORTS
+// Test that '_Z11global_funcv' has indirect calls annotated with value profiles.
+// RUN: llvm-dis main.import.bc -o - | FileCheck %s --check-prefix=IR
+
+// Test that both candidates are ICP'ed and there is no `!VP` in the IR.
+// RUN: opt main.import.bc -icp-lto -passes=pgo-icall-prom -S -pass-remarks=pgo-icall-prom 2>&1 | FileCheck %s --check-prefixes=ICP-IR,ICP-REMARK --implicit-check-not="!VP"
+
+// IMPORTS: main.cpp: Import _Z7callee1v
+// IMPORTS: main.cpp: Import _ZL7callee0v.llvm.[[#]]
+// IMPORTS: main.cpp: Import _Z11global_funcv
+
+// PGOName: define {{(dso_local )?}}void @_Z7callee1v() #[[#]] !prof ![[#]] {
+// PGOName: define internal void @_ZL7callee0v() #[[#]] !prof ![[#]] !PGOFuncName ![[#MD:]] {
+// PGOName: ![[#MD]] = !{!"{{.*}}lib.cpp;_ZL7callee0v"}
+
+// IR-LABEL: define available_externally {{.*}} void @_Z11global_funcv() {{.*}} !prof ![[#]] {
+// IR-NEXT: entry:
+// IR-NEXT: %0 = load ptr, ptr @calleeAddrs
+// IR-NEXT: tail call void %0(), !prof ![[#PROF1:]]
+// IR-NEXT: %1 = load ptr, ptr getelementptr inbounds ([2 x ptr], ptr @calleeAddrs,
+// IR-NEXT: tail call void %1(), !prof ![[#PROF2:]]
+
+// The GUID of indirect callee is the MD5 hash of `/path/to/lib.cpp;_ZL7callee0v`
+// that depends on the directory. Use [[#]] for its MD5 hash.
+// Use {{.*}} for integer types so the test works on 32-bit and 64-bit systems.
+// IR: ![[#PROF1]] = !{!"VP", i32 0, {{.*}} 1, {{.*}} [[#]], {{.*}} 1}
+// IR: ![[#PROF2]] = !{!"VP", i32 0, {{.*}} 1, {{.*}} -3993653843325621743, {{.*}} 1}
+
+// ICP-REMARK: Promote indirect call to _ZL7callee0v.llvm.[[#]] with count 1 out of 1
+// ICP-REMARK: Promote indirect call to _Z7callee1v with count 1 out of 1
+
+// ICP-IR: br i1 %[[#]], label %if.true.direct_targ, label %if.false.orig_indirect, !prof ![[#BRANCH_WEIGHT1:]]
+// ICP-IR: br i1 %[[#]], label %if.true.direct_targ1, label %if.false.orig_indirect2, !prof ![[#BRANCH_WEIGHT1]]
+// ICP-IR: ![[#BRANCH_WEIGHT1]] = !{!"branch_weights", i32 1, i32 0}
+
+//--- lib.h
+void global_func();
+
+//--- lib.cpp
+#include "lib.h"
+static void callee0() {}
+void callee1() {}
+typedef void (*FPT)();
+FPT calleeAddrs[] = {callee0, callee1};
+// `global_func`` might call one of two indirect callees. callee0 has internal
+// linkage and callee1 has external linkage.
+void global_func() {
+ FPT fp = calleeAddrs[0];
+ fp();
+ fp = calleeAddrs[1];
+ fp();
+}
+
+//--- main.cpp
+#include "lib.h"
+int main() { global_func(); }
diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index d1891c157099d4..e97a7f2b963606 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -41,6 +41,10 @@ namespace Intrinsic {
typedef unsigned ID;
} // end namespace Intrinsic
+// Choose ';' as the delimiter. ':' was used once but it doesn't work well for
+// Objective-C functions which commonly have :'s in their names.
+inline constexpr char kGlobalIdentifierDelimiter = ';';
+
class GlobalValue : public Constant {
public:
/// An enumeration for the kinds of linkage for global values.
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index 288dc71d756aee..36be2e7d869e7b 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -171,6 +171,8 @@ inline StringRef getInstrProfCounterBiasVarName() {
/// Return the marker used to separate PGO names during serialization.
inline StringRef getInstrProfNameSeparator() { return "\01"; }
+/// Please use getIRPGOFuncName for LLVM IR instrumentation. This function is
+/// for front-end (Clang, etc) instrumentation.
/// Return the modified name for function \c F suitable to be
/// used the key for profile lookup. Variable \c InLTO indicates if this
/// is called in LTO optimization passes.
@@ -196,20 +198,22 @@ std::string getIRPGOFuncName(const Function &F, bool InLTO = false);
std::pair<StringRef, StringRef> getParsedIRPGOFuncName(StringRef IRPGOFuncName);
/// Return the name of the global variable used to store a function
-/// name in PGO instrumentation. \c FuncName is the name of the function
-/// returned by the \c getPGOFuncName call.
+/// name in PGO instrumentation. \c FuncName is the IRPGO function name
+/// (returned by \c getIRPGOFuncName) for LLVM IR instrumentation and PGO
+/// function name (returned by \c getPGOFuncName) for front-end instrumentation.
std::string getPGOFuncNameVarName(StringRef FuncName,
GlobalValue::LinkageTypes Linkage);
/// Create and return the global variable for function name used in PGO
-/// instrumentation. \c FuncName is the name of the function returned
-/// by \c getPGOFuncName call.
+/// instrumentation. \c FuncName is the IRPGO function name (returned by
+/// \c getIRPGOFuncName) for LLVM IR instrumentation and PGO function name
+/// (returned by \c getPGOFuncName) for front-end instrumentation.
GlobalVariable *createPGOFuncNameVar(Function &F, StringRef PGOFuncName);
/// Create and return the global variable for function name used in PGO
-/// instrumentation. /// \c FuncName is the name of the function
-/// returned by \c getPGOFuncName call, \c M is the owning module,
-/// and \c Linkage is the linkage of the instrumented function.
+/// instrumentation. \c FuncName is the IRPGO function name (returned by
+/// \c getIRPGOFuncName) for LLVM IR instrumentation and PGO function name
+/// (returned by \c getPGOFuncName) for front-end instrumentation.
GlobalVariable *createPGOFuncNameVar(Module &M,
GlobalValue::LinkageTypes Linkage,
StringRef PGOFuncName);
@@ -417,11 +421,11 @@ uint64_t ComputeHash(StringRef K);
} // end namespace IndexedInstrProf
-/// A symbol table used for function PGO name look-up with keys
+/// A symbol table used for function [IR]PGO name look-up with keys
/// (such as pointers, md5hash values) to the function. A function's
-/// PGO name or name's md5hash are used in retrieving the profile
-/// data of the function. See \c getPGOFuncName() method for details
-/// on how PGO name is formed.
+/// [IR]PGO name or name's md5hash are used in retrieving the profile
+/// data of the function. See \c getIRPGOFuncName() and \c getPGOFuncName
+/// methods for details how [IR]PGO name is formed.
class InstrProfSymtab {
public:
using AddrHashMap = std::vector<std::pair<uint64_t, uint64_t>>;
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 51bdbeb0abf2c4..239acd2181e854 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -144,25 +144,27 @@ void GlobalObject::copyAttributesFrom(const GlobalObject *Src) {
std::string GlobalValue::getGlobalIdentifier(StringRef Name,
GlobalValue::LinkageTypes Linkage,
StringRef FileName) {
-
// Value names may be prefixed with a binary '1' to indicate
// that the backend should not modify the symbols due to any platform
// naming convention. Do not include that '1' in the PGO profile name.
if (Name[0] == '\1')
Name = Name.substr(1);
- std::string NewName = std::string(Name);
+ std::string GlobalName;
if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
// For local symbols, prepend the main file name to distinguish them.
// Do not include the full path in the file name since there's no guarantee
// that it will stay the same, e.g., if the files are checked out from
// version control in different locations.
if (FileName.empty())
- NewName = NewName.insert(0, "<unknown>:");
+ GlobalName += "<unknown>";
else
- NewName = NewName.insert(0, FileName.str() + ":");
+ GlobalName += FileName;
+
+ GlobalName += kGlobalIdentifierDelimiter;
}
- return NewName;
+ GlobalName += Name;
+ return GlobalName;
}
std::string GlobalValue::getGlobalIdentifier() const {
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 649d814cfd9de0..134a400e639c4b 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -246,11 +246,27 @@ std::string InstrProfError::message() const {
char InstrProfError::ID = 0;
-std::string getPGOFuncName(StringRef RawFuncName,
- GlobalValue::LinkageTypes Linkage,
+std::string getPGOFuncName(StringRef Name, GlobalValue::LinkageTypes Linkage,
StringRef FileName,
uint64_t Version LLVM_ATTRIBUTE_UNUSED) {
- return GlobalValue::getGlobalIdentifier(RawFuncName, Linkage, FileName);
+ // Value names may be prefixed with a binary '1' to indicate
+ // that the backend should not modify the symbols due to any platform
+ // naming convention. Do not include that '1' in the PGO profile name.
+ if (Name[0] == '\1')
+ Name = Name.substr(1);
+
+ std::string NewName = std::string(Name);
+ if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
+ // For local symbols, prepend the main file name to distinguish them.
+ // Do not include the full path in the file name since there's no guarantee
+ // that it will stay the same, e.g., if the files are checked out from
+ // version control in different locations.
+ if (FileName.empty())
+ NewName = NewName.insert(0, "<unknown>:");
+ else
+ NewName = NewName.insert(0, FileName.str() + ":");
+ }
+ return NewName;
}
// Strip NumPrefix level of directory name from PathNameStr. If the number of
@@ -300,12 +316,10 @@ getIRPGONameForGlobalObject(const GlobalObject &GO,
GlobalValue::LinkageTypes Linkage,
StringRef FileName) {
SmallString<64> Name;
- if (llvm::GlobalValue::isLocalLinkage(Linkage)) {
- Name.append(FileName.empty() ? "<unknown>" : FileName);
- Name.append(";");
- }
+ // FIXME: Mangler's handling is kept outside of `getGlobalIdentifier` for now.
+ // For more details please check issue #74565.
Mangler().getNameWithPrefix(Name, &GO, /*CannotUsePrivateLabel=*/true);
- return Name.str().str();
+ return GlobalValue::getGlobalIdentifier(Name, Linkage, FileName);
}
static std::optional<std::string> lookupPGONameFromMetadata(MDNode *MD) {
@@ -352,6 +366,9 @@ std::string getIRPGOFuncName(const Function &F, bool InLTO) {
return getIRPGOObjectName(F, InLTO, getPGOFuncNameMetadata(F));
}
+// Please use getIRPGOFuncName for LLVM IR instrumentation. This function is
+// for front-end (Clang, etc) instrumentation.
+// The implementation is kept for profile matching from older profiles.
// This is similar to `getIRPGOFuncName` except that this function calls
// 'getPGOFuncName' to get a name and `getIRPGOFuncName` calls
// 'getIRPGONameForGlobalObject'. See the difference between two callees in the
@@ -384,7 +401,8 @@ getParsedIRPGOFuncName(StringRef IRPGOFuncName) {
StringRef getFuncNameWithoutPrefix(StringRef PGOFuncName, StringRef FileName) {
if (FileName.empty())
return PGOFuncName;
- // Drop the file name including ':'. See also getPGOFuncName.
+ // Drop the file name including ':' or ';'. See getIRPGONameForGlobalObject as
+ // well.
if (PGOFuncName.starts_with(FileName))
PGOFuncName = PGOFuncName.drop_front(FileName.size() + 1);
return PGOFuncName;
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 068922d421f8b9..8f62df79d5b7e8 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -1008,12 +1008,13 @@ class llvm::InstrProfReaderItaniumRemapper
/// Extract the original function name from a PGO function name.
static StringRef extractName(StringRef Name) {
- // We can have multiple :-separated pieces; there can be pieces both
- // before and after the mangled name. Find the first part that starts
- // with '_Z'; we'll assume that's the mangled name we want.
+ // We can have multiple pieces separated by kGlobalIdentifierDelimiter (
+ // semicolon now and colon in older profiles); there can be pieces both
+ // before and after the mangled name. Find the first part that starts with
+ // '_Z'; we'll assume that's the mangled name we want.
std::pair<StringRef, StringRef> Parts = {StringRef(), Name};
while (true) {
- Parts = Parts.second.split(':');
+ Parts = Parts.second.split(kGlobalIdentifierDelimiter);
if (Parts.first.starts_with("_Z"))
return Parts.first;
if (Parts.second.empty())
diff --git a/llvm/test/Bitcode/thinlto-function-summary-originalnames.ll b/llvm/test/Bitcode/thinlto-function-summary-originalnames.ll
index 7cc9654c8c7b12..0139f00b4aa3f4 100644
--- a/llvm/test/Bitcode/thinlto-function-summary-originalnames.ll
+++ b/llvm/test/Bitcode/thinlto-function-summary-originalnames.ll
@@ -6,13 +6,13 @@
; COMBINED: <GLOBALVAL_SUMMARY_BLOCK
; COMBINED-NEXT: <VERSION
; COMBINED-NEXT: <FLAGS
-; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=4947176790635855146/>
-; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=-6591587165810580810/>
-; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=-4377693495213223786/>
+; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=686735765308251824/>
+; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=4507502870619175775/>
+; COMBINED-NEXT: <VALUE_GUID {{.*}} op1=-8118561185538785069/>
; COMBINED-DAG: <COMBINED_PROFILE{{ }}
-; COMBINED-DAG: <COMBINED_ORIGINAL_NAME op0=6699318081062747564/>
-; COMBINED-DAG: <COMBINED_GLOBALVAR_INIT_REFS
; COMBINED-DAG: <COMBINED_ORIGINAL_NAME op0=-2012135647395072713/>
+; COMBINED-DAG: <COMBINED_GLOBALVAR_INIT_REFS
+; COMBINED-DAG: <COMBINED_ORIGINAL_NAME op0=6699318081062747564/>
; COMBINED-DAG: <COMBINED_ALIAS
; COMBINED-DAG: <COMBINED_ORIGINAL_NAME op0=-4170563161550796836/>
; COMBINED-NEXT: </GLOBALVAL_SUMMARY_BLOCK>
diff --git a/llvm/test/ThinLTO/X86/memprof-basic.ll b/llvm/test/ThinLTO/X86/memprof-basic.ll
index 0d466830ba57d6..54e01e5fcdf955 100644
--- a/llvm/test/ThinLTO/X86/memprof-basic.ll
+++ b/llvm/test/ThinLTO/X86/memprof-basic.ll
@@ -148,7 +148,7 @@ attributes #0 = { noinline optnone }
; DUMP: Edge from Callee [[BAR]] to Caller: [[BAZ:0x[a-z0-9]+]] AllocTypes: NotColdCold ContextIds: 1 2
; DUMP: Node [[BAZ]]
-; DUMP: Callee: 9832687305761716512 (_Z3barv) Clones: 0 StackIds: 2 (clone 0)
+; DUMP: Callee: 11481133863268513686 (_Z3barv) Clones: 0 StackIds: 2 (clone 0)
; DUMP: AllocTypes: NotColdCold
; DUMP: ContextIds: 1 2
; DUMP: CalleeEdges:
@@ -157,7 +157,7 @@ attributes #0 = { noinline optnone }
; DUMP: Edge from Callee [[BAZ]] to Caller: [[FOO:0x[a-z0-9]+]] AllocTypes: NotColdCold ContextIds: 1 2
; DUMP: Node [[FOO]]
-; DUMP: Callee: 5878270615442837395 (_Z3bazv) Clones: 0 StackIds: 3 (clone 0)
+; DUMP: Callee: 1807954217441101578 (_Z3bazv) Clones: 0 StackIds: 3 (clone 0)
; DUMP: AllocTypes: NotColdCold
; DUMP: ContextIds: 1 2
; DUMP: CalleeEdges:
@@ -167,7 +167,7 @@ attributes #0 = { noinline optnone }
; DUMP: Edge from Callee [[FOO]] to Caller: [[MAIN2:0x[a-z0-9]+]] AllocTypes: Cold ContextIds: 2
; DUMP: Node [[MAIN1]]
-; DUMP: Callee: 6731117468105397038 (_Z3foov) Clones: 0 StackIds: 0 (clone 0)
+; DUMP: Callee: 8107868197919466657 (_Z3foov) Clones: 0 StackIds: 0 (clone 0)
; DUMP: AllocTypes: NotCold
; DUMP: ContextIds: 1
; DUMP: CalleeEdges:
@@ -175,7 +175,7 @@ attributes #0 = { noinline optnone }
; DUMP: CallerEdges:
; DUMP: Node [[MAIN2]]
-; DUMP: Callee: 6731117468105397038 (_Z3foov) Clones: 0 StackIds: 1 (clone 0)
+; DUMP: Callee: 8107868197919466657 (_Z3foov) Clones: 0 StackIds: 1 (clone 0)
; DUMP: AllocTypes: Cold
; DUMP: ContextIds: 2
; DUMP: CalleeEdges:
@@ -197,7 +197,7 @@ attributes #0 = { noinline optnone }
; DUMP: Clones: [[BAR2:0x[a-z0-9]+]]
; DUMP: Node [[BAZ]]
-; DUMP: Callee: 9832687305761716512 (_Z3barv) Clones: 0 StackIds: 2 (clone 0)
+; DUMP: Callee: 11481133863268513686 (_Z3barv) Clones: 0 St...
[truncated]
|
This echoes my complaints :)
I am playing with https://getcord.github.io/spr/index.html to use Phabricator workflow without affecting reviewers' experience. |
MaskRay
approved these changes
Dec 18, 2023
Yeah, in this case there is no merge conflict for the pull request, and the error only manifest after merge happens. Plus, the presubmit build-bot (for example the currently pending one for this pull request) takes hours to complete, during which window new changes could in theory come in..
This is helpful to know! Are there known blockers to use |
|
The test failed on Windows due to mismatched names (https://lab.llvm.org/buildbot/#/builders/127/builds/59907/steps/8/logs/stdio) I propose to mark the test as 'unsupported' for now before fixing all name matchers (#75886) |
|
I'll revert this one and fix the names offline. |
mingmingl-llvm
added a commit
that referenced
this pull request
Dec 19, 2023
…tifier, use semicolon as delimiter for local-linkage varibles. "" (#75888) Reverts #75860 - Mangled name mismatch on Windows (https://lab.llvm.org/buildbot/#/builders/127/builds/59907/steps/8/logs/stdio)
mingmingl-llvm
added a commit
that referenced
this pull request
Dec 19, 2023
…dentifier, use semicolon as delimiter for local-linkage varibles. " (#75954) Simplify the compiler-rt test to make it more general for different platforms, and use `*DAG` matchers for lines that may be emitted out-of-order. - The compiler-rt test passed on a Windows machine. Previously name matchers don't work for MSVC mangling (https://lab.llvm.org/buildbot/#/builders/127/builds/59907) - `*DAG` matchers fixed the error in https://lab.llvm.org/buildbot/#/builders/94/builds/17924 This is the second reland and fixed errors caught in first reland (#75860) **Original commit message** Commit fe05193 (phab D156569), IRPGO names uses format `[<filepath>;]<linkage-name>` while prior format is `[<filepath>:<mangled-name>`. The format change would break the use case demonstrated in (updated) `llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll` and `compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp` This patch changes `GlobalValues::getGlobalIdentifer` to use the semicolon. To elaborate on the scenario how things break without this PR 1. IRPGO raw profiles stores (compressed) IRPGO names of functions in one section, and per-function profile data in another section. The [NameRef](https://github.com/llvm/llvm-project/blob/fc715e4cd942612a091097339841733757b53824/compiler-rt/include/profile/InstrProfData.inc#L72) field in per-function profile data is the MD5 hash of IRPGO names. 2. When raw profiles are converted to indexed format profiles, the profiled address is [mapped](https://github.com/llvm/llvm-project/blob/fc715e4cd942612a091097339841733757b53824/llvm/lib/ProfileData/InstrProf.cpp#L876-L885) to the MD5 hash of the callee. 3. In `pgo-instr-use` thin-lto prelink pipeline, MD5 hash of IRPGO names will be [annotated](https://github.com/llvm/llvm-project/blob/fc715e4cd942612a091097339841733757b53824/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#L1707) as value profiles, and used to import indirect-call-prom candidates. If the annotated MD5 hash is computed from the new format while import uses the prior format, the callee cannot be imported. * `compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp` is added to have an end-to-end test. * `llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll` is updated to have better test coverage from another aspect (as runtime tests are more sensitive to the environment and may be skipped by some contributors)
qihangkong
pushed a commit
to rvgpu/rvgpu-llvm
that referenced
this pull request
Apr 23, 2024
…tifier, use semicolon as delimiter for local-linkage varibles. "" (#75888) Reverts llvm/llvm-project#75860 - Mangled name mismatch on Windows (https://lab.llvm.org/buildbot/#/builders/127/builds/59907/steps/8/logs/stdio)
qihangkong
pushed a commit
to rvgpu/rvgpu-llvm
that referenced
this pull request
Apr 23, 2024
…dentifier, use semicolon as delimiter for local-linkage varibles. " (#75954) Simplify the compiler-rt test to make it more general for different platforms, and use `*DAG` matchers for lines that may be emitted out-of-order. - The compiler-rt test passed on a Windows machine. Previously name matchers don't work for MSVC mangling (https://lab.llvm.org/buildbot/#/builders/127/builds/59907) - `*DAG` matchers fixed the error in https://lab.llvm.org/buildbot/#/builders/94/builds/17924 This is the second reland and fixed errors caught in first reland (llvm/llvm-project#75860) **Original commit message** Commit fe05193 (phab D156569), IRPGO names uses format `[<filepath>;]<linkage-name>` while prior format is `[<filepath>:<mangled-name>`. The format change would break the use case demonstrated in (updated) `llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll` and `compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp` This patch changes `GlobalValues::getGlobalIdentifer` to use the semicolon. To elaborate on the scenario how things break without this PR 1. IRPGO raw profiles stores (compressed) IRPGO names of functions in one section, and per-function profile data in another section. The [NameRef](https://github.com/llvm/llvm-project/blob/fc715e4cd942612a091097339841733757b53824/compiler-rt/include/profile/InstrProfData.inc#L72) field in per-function profile data is the MD5 hash of IRPGO names. 2. When raw profiles are converted to indexed format profiles, the profiled address is [mapped](https://github.com/llvm/llvm-project/blob/fc715e4cd942612a091097339841733757b53824/llvm/lib/ProfileData/InstrProf.cpp#L876-L885) to the MD5 hash of the callee. 3. In `pgo-instr-use` thin-lto prelink pipeline, MD5 hash of IRPGO names will be [annotated](https://github.com/llvm/llvm-project/blob/fc715e4cd942612a091097339841733757b53824/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#L1707) as value profiles, and used to import indirect-call-prom candidates. If the annotated MD5 hash is computed from the new format while import uses the prior format, the callee cannot be imported. * `compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cpp` is added to have an end-to-end test. * `llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll` is updated to have better test coverage from another aspect (as runtime tests are more sensitive to the environment and may be skipped by some contributors)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixed build-bot failures caught by post-submit tests (currently this pull request contains a re-apply commit and a fix commit, to make the reland diff clearer)
<command-line-tool> not founderror in https://lab.llvm.org/buildbot/#/builders/127/builds/59884ninja check-profile, add missing dependencies one by one allows me to see errors due to missed dependencies and fixed all of them.starts_withto replace deprecatedstartswith.error: 'startswith' is deprecated: Use starts_with instead [-Werror,-Wdeprecated-declarations]in https://lab.llvm.org/buildbot/#/builders/36/builds/40893Original commit message
Commit fe05193 (phab D156569), IRPGO names uses format
[<filepath>;]<linkage-name>while prior format is[<filepath>:<mangled-name>. The format change would break the use case demonstrated in (updated)llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.llandcompiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cppThis patch changes
GlobalValues::getGlobalIdentiferto use the semicolon.To elaborate on the scenario how things break without this PR
pgo-instr-usethin-lto prelink pipeline, MD5 hash of IRPGO names will be annotated as value profiles, and used to import indirect-call-prom candidates. If the annotated MD5 hash is computed from the new format while import uses the prior format, the callee cannot be imported.compiler-rt/test/profile/instrprof-thinlto-indirect-call-promotion.cppis added to have an end-to-end test.llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.llis updated to have better test coverage from another aspect (as runtime tests are more sensitive to the environment and may be skipped by some contributors)