[Profile] Refactor profile correlation. #69656
Closed
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
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Zequan Wu (ZequanWu) ChangesRefactor some code from #69493. Patch is 37.13 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/69656.diff 21 Files Affected:
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 70accce456d3c07..83b81a38a768523 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -42,6 +42,7 @@
#include "llvm/Passes/PassBuilder.h"
#include "llvm/Passes/PassPlugin.h"
#include "llvm/Passes/StandardInstrumentations.h"
+#include "llvm/ProfileData/InstrProfCorrelator.h"
#include "llvm/Support/BuryPointer.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/MemoryBuffer.h"
@@ -55,6 +56,7 @@
#include "llvm/Target/TargetOptions.h"
#include "llvm/TargetParser/SubtargetFeature.h"
#include "llvm/TargetParser/Triple.h"
+#include "llvm/Transforms/HipStdPar/HipStdPar.h"
#include "llvm/Transforms/IPO/EmbedBitcodePass.h"
#include "llvm/Transforms/IPO/LowerTypeTests.h"
#include "llvm/Transforms/IPO/ThinLTOBitcodeWriter.h"
@@ -78,7 +80,6 @@
#include "llvm/Transforms/Scalar/EarlyCSE.h"
#include "llvm/Transforms/Scalar/GVN.h"
#include "llvm/Transforms/Scalar/JumpThreading.h"
-#include "llvm/Transforms/HipStdPar/HipStdPar.h"
#include "llvm/Transforms/Utils/Debugify.h"
#include "llvm/Transforms/Utils/EntryExitInstrumenter.h"
#include "llvm/Transforms/Utils/ModuleUtils.h"
@@ -98,13 +99,18 @@ extern cl::opt<bool> PrintPipelinePasses;
static cl::opt<bool> ClSanitizeOnOptimizerEarlyEP(
"sanitizer-early-opt-ep", cl::Optional,
cl::desc("Insert sanitizers on OptimizerEarlyEP."), cl::init(false));
-}
+
+extern cl::opt<bool> DebugInfoCorrelate;
+extern cl::opt<InstrProfCorrelator::ProfCorrelatorKind> ProfileCorrelate;
+} // namespace llvm
namespace {
// Default filename used for profile generation.
std::string getDefaultProfileGenName() {
- return DebugInfoCorrelate ? "default_%m.proflite" : "default_%m.profraw";
+ return DebugInfoCorrelate || ProfileCorrelate != InstrProfCorrelator::NONE
+ ? "default_%m.proflite"
+ : "default_%m.profraw";
}
class EmitAssemblyHelper {
@@ -197,7 +203,7 @@ class EmitAssemblyHelper {
void EmitAssembly(BackendAction Action,
std::unique_ptr<raw_pwrite_stream> OS);
};
-}
+} // namespace
static SanitizerCoverageOptions
getSancovOptsFromCGOpts(const CodeGenOptions &CGOpts) {
diff --git a/compiler-rt/lib/profile/InstrProfiling.c b/compiler-rt/lib/profile/InstrProfiling.c
index 0dd5ff5ae6331cb..e3bb8329216363e 100644
--- a/compiler-rt/lib/profile/InstrProfiling.c
+++ b/compiler-rt/lib/profile/InstrProfiling.c
@@ -85,3 +85,7 @@ COMPILER_RT_VISIBILITY void __llvm_profile_reset_counters(void) {
}
lprofSetProfileDumped(0);
}
+
+inline int hasCorrelation() {
+ return (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) != 0ULL;
+}
diff --git a/compiler-rt/lib/profile/InstrProfiling.h b/compiler-rt/lib/profile/InstrProfiling.h
index 4433d7bd48871fc..c3203d51d5ca75c 100644
--- a/compiler-rt/lib/profile/InstrProfiling.h
+++ b/compiler-rt/lib/profile/InstrProfiling.h
@@ -259,6 +259,9 @@ uint64_t __llvm_profile_get_magic(void);
/*! \brief Get the version of the file format. */
uint64_t __llvm_profile_get_version(void);
+/*! \brief If the binary is compiled with profile correlation. */
+int hasCorrelation();
+
/*! \brief Get the number of entries in the profile data section. */
uint64_t __llvm_profile_get_num_data(const __llvm_profile_data *Begin,
const __llvm_profile_data *End);
@@ -276,6 +279,9 @@ uint64_t __llvm_profile_get_num_counters(const char *Begin, const char *End);
/*! \brief Get the size of the profile counters section in bytes. */
uint64_t __llvm_profile_get_counters_size(const char *Begin, const char *End);
+/*! \brief Get the size of the profile name section in bytes. */
+uint64_t __llvm_profile_get_name_size(const char *Begin, const char *End);
+
/* ! \brief Given the sizes of the data and counter information, return the
* number of padding bytes before and after the counters, and after the names,
* in the raw profile.
diff --git a/compiler-rt/lib/profile/InstrProfilingBuffer.c b/compiler-rt/lib/profile/InstrProfilingBuffer.c
index 61ac5d9c0285002..7f46e874f0b3248 100644
--- a/compiler-rt/lib/profile/InstrProfilingBuffer.c
+++ b/compiler-rt/lib/profile/InstrProfilingBuffer.c
@@ -53,6 +53,8 @@ uint64_t __llvm_profile_get_size_for_buffer(void) {
COMPILER_RT_VISIBILITY
uint64_t __llvm_profile_get_num_data(const __llvm_profile_data *Begin,
const __llvm_profile_data *End) {
+ if (hasCorrelation())
+ return 0;
intptr_t BeginI = (intptr_t)Begin, EndI = (intptr_t)End;
return ((EndI + sizeof(__llvm_profile_data) - 1) - BeginI) /
sizeof(__llvm_profile_data);
@@ -61,6 +63,8 @@ uint64_t __llvm_profile_get_num_data(const __llvm_profile_data *Begin,
COMPILER_RT_VISIBILITY
uint64_t __llvm_profile_get_data_size(const __llvm_profile_data *Begin,
const __llvm_profile_data *End) {
+ if (hasCorrelation())
+ return 0;
return __llvm_profile_get_num_data(Begin, End) * sizeof(__llvm_profile_data);
}
@@ -83,6 +87,13 @@ uint64_t __llvm_profile_get_counters_size(const char *Begin, const char *End) {
__llvm_profile_counter_entry_size();
}
+COMPILER_RT_VISIBILITY
+uint64_t __llvm_profile_get_name_size(const char *Begin, const char *End) {
+ if (hasCorrelation())
+ return 0;
+ return End - Begin;
+}
+
/// Calculate the number of padding bytes needed to add to \p Offset in order
/// for (\p Offset + Padding) to be page-aligned.
static uint64_t calculateBytesNeededToPageAlign(uint64_t Offset) {
diff --git a/compiler-rt/lib/profile/InstrProfilingMerge.c b/compiler-rt/lib/profile/InstrProfilingMerge.c
index 9cf12f251f7262d..e3a2e31df7c43ec 100644
--- a/compiler-rt/lib/profile/InstrProfilingMerge.c
+++ b/compiler-rt/lib/profile/InstrProfilingMerge.c
@@ -66,8 +66,9 @@ int __llvm_profile_check_compatibility(const char *ProfileData,
Header->NumCounters !=
__llvm_profile_get_num_counters(__llvm_profile_begin_counters(),
__llvm_profile_end_counters()) ||
- Header->NamesSize != (uint64_t)(__llvm_profile_end_names() -
- __llvm_profile_begin_names()) ||
+ Header->NamesSize !=
+ __llvm_profile_get_name_size(__llvm_profile_begin_names(),
+ __llvm_profile_end_names()) ||
Header->ValueKindLast != IPVK_Last)
return 1;
@@ -130,9 +131,9 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData,
if (SrcNameStart < SrcCountersStart)
return 1;
- // Merge counters by iterating the entire counter section when debug info
- // correlation is enabled.
- if (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) {
+ // Merge counters by iterating the entire counter section when correlation is
+ // enabled.
+ if (hasCorrelation()) {
for (SrcCounter = SrcCountersStart,
DstCounter = __llvm_profile_begin_counters();
SrcCounter < SrcCountersEnd;) {
diff --git a/compiler-rt/lib/profile/InstrProfilingWriter.c b/compiler-rt/lib/profile/InstrProfilingWriter.c
index 1e22398a4c0f64a..2a41600b65b9aac 100644
--- a/compiler-rt/lib/profile/InstrProfilingWriter.c
+++ b/compiler-rt/lib/profile/InstrProfilingWriter.c
@@ -259,19 +259,17 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const __llvm_profile_data *DataBegin,
const char *CountersBegin, const char *CountersEnd,
VPDataReaderType *VPDataReader, const char *NamesBegin,
const char *NamesEnd, int SkipNameDataWrite) {
- int DebugInfoCorrelate =
- (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) != 0ULL;
+ int ProfileCorrelation = hasCorrelation();
/* Calculate size of sections. */
const uint64_t DataSectionSize =
- DebugInfoCorrelate ? 0 : __llvm_profile_get_data_size(DataBegin, DataEnd);
- const uint64_t NumData =
- DebugInfoCorrelate ? 0 : __llvm_profile_get_num_data(DataBegin, DataEnd);
+ __llvm_profile_get_data_size(DataBegin, DataEnd);
+ const uint64_t NumData = __llvm_profile_get_num_data(DataBegin, DataEnd);
const uint64_t CountersSectionSize =
__llvm_profile_get_counters_size(CountersBegin, CountersEnd);
const uint64_t NumCounters =
__llvm_profile_get_num_counters(CountersBegin, CountersEnd);
- const uint64_t NamesSize = DebugInfoCorrelate ? 0 : NamesEnd - NamesBegin;
+ const uint64_t NamesSize = __llvm_profile_get_name_size(NamesBegin, NamesEnd);
/* Create the header. */
__llvm_profile_header Header;
@@ -298,7 +296,7 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const __llvm_profile_data *DataBegin,
#endif
/* The data and names sections are omitted in lightweight mode. */
- if (DebugInfoCorrelate) {
+ if (ProfileCorrelation) {
Header.CountersDelta = 0;
Header.NamesDelta = 0;
}
@@ -314,19 +312,20 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const __llvm_profile_data *DataBegin,
/* Write the profile data. */
ProfDataIOVec IOVecData[] = {
- {DebugInfoCorrelate ? NULL : DataBegin, sizeof(uint8_t), DataSectionSize,
+ {ProfileCorrelation ? NULL : DataBegin, sizeof(uint8_t), DataSectionSize,
0},
{NULL, sizeof(uint8_t), PaddingBytesBeforeCounters, 1},
{CountersBegin, sizeof(uint8_t), CountersSectionSize, 0},
{NULL, sizeof(uint8_t), PaddingBytesAfterCounters, 1},
- {(SkipNameDataWrite || DebugInfoCorrelate) ? NULL : NamesBegin,
+ {(SkipNameDataWrite || ProfileCorrelation) ? NULL : NamesBegin,
sizeof(uint8_t), NamesSize, 0},
{NULL, sizeof(uint8_t), PaddingBytesAfterNames, 1}};
if (Writer->Write(Writer, IOVecData, sizeof(IOVecData) / sizeof(*IOVecData)))
return -1;
- /* Value profiling is not yet supported in continuous mode. */
- if (__llvm_profile_is_continuous_mode_enabled())
+ /* Value profiling is not yet supported in continuous mode and profile
+ * correlation mode. */
+ if (__llvm_profile_is_continuous_mode_enabled() || ProfileCorrelation)
return 0;
return writeValueProfData(Writer, VPDataReader, DataBegin, DataEnd);
diff --git a/compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c b/compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
index f347d439e2e0671..46d25a4e386dc3a 100644
--- a/compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
+++ b/compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
@@ -1,5 +1,5 @@
// Value profiling is currently not supported in lightweight mode.
-// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: %clang_pgogen -o %t -g -mllvm --profile-correlate=debug-info -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
// RUN: env LLVM_PROFILE_FILE=%t.proflite %run %t
// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t.dSYM %t.proflite
@@ -9,7 +9,7 @@
// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata)
-// RUN: %clang_pgogen -o %t.cov -g -mllvm --debug-info-correlate -mllvm -pgo-function-entry-coverage -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: %clang_pgogen -o %t.cov -g -mllvm --profile-correlate=debug-info -mllvm -pgo-function-entry-coverage -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
// RUN: env LLVM_PROFILE_FILE=%t.cov.proflite %run %t.cov
// RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov.dSYM %t.cov.proflite
diff --git a/compiler-rt/test/profile/Linux/instrprof-debug-info-correlate-warnings.c b/compiler-rt/test/profile/Linux/instrprof-debug-info-correlate-warnings.c
index 5069c6340b64fd2..25022f241a6d281 100644
--- a/compiler-rt/test/profile/Linux/instrprof-debug-info-correlate-warnings.c
+++ b/compiler-rt/test/profile/Linux/instrprof-debug-info-correlate-warnings.c
@@ -1,6 +1,6 @@
// Disable full debug info and verify that we get warnings during merging
-// RUN: %clang_pgogen -o %t -gline-tables-only -mllvm --debug-info-correlate -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: %clang_pgogen -o %t -gline-tables-only -mllvm --profile-correlate=debug-info -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
// RUN: env LLVM_PROFILE_FILE=%t.proflite %run %t
// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t %t.proflite --max-debug-info-correlation-warnings=2 2>&1 >/dev/null | FileCheck %s --check-prefixes=CHECK,LIMIT --implicit-check-not=warning
// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t %t.proflite --max-debug-info-correlation-warnings=0 2>&1 >/dev/null | FileCheck %s --check-prefixes=CHECK,NOLIMIT --implicit-check-not=warning
diff --git a/compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c b/compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
index a918d7b6299005e..ccfd65b6f4c4b16 100644
--- a/compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
+++ b/compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
@@ -3,19 +3,19 @@
// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t.normal
// RUN: llvm-profdata merge -o %t.normal.profdata %t.profraw
-// RUN: %clang_pgogen -o %t.d4 -g -gdwarf-4 -mllvm --debug-info-correlate -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: %clang_pgogen -o %t.d4 -g -gdwarf-4 -mllvm --profile-correlate=debug-info -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
// RUN: env LLVM_PROFILE_FILE=%t.d4.proflite %run %t.d4
// RUN: llvm-profdata merge -o %t.d4.profdata --debug-info=%t.d4 %t.d4.proflite
// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.d4.profdata)
-// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: %clang_pgogen -o %t -g -mllvm --profile-correlate=debug-info -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
// RUN: env LLVM_PROFILE_FILE=%t.proflite %run %t
// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t %t.proflite
// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata)
-// RUN: %clang_pgogen -o %t.cov -g -mllvm --debug-info-correlate -mllvm -pgo-function-entry-coverage -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: %clang_pgogen -o %t.cov -g -mllvm --profile-correlate=debug-info -mllvm -pgo-function-entry-coverage -mllvm --disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp %S/../Inputs/instrprof-debug-info-correlate-foo.cpp
// RUN: env LLVM_PROFILE_FILE=%t.cov.proflite %run %t.cov
// RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov %t.cov.proflite
diff --git a/compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c b/compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c
index 226d678aca347a4..93bf40f98d3ab62 100644
--- a/compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c
+++ b/compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c
@@ -1,10 +1,10 @@
-// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %s
+// RUN: %clang_pgogen -o %t -g -mllvm --profile-correlate=debug-info -mllvm --disable-vp=true %s
// RUN: llvm-profdata show --debug-info=%t --detailed-summary --show-prof-sym-list | FileCheck %s
// RUN: llvm-profdata show --debug-info=%t --show-format=yaml | FileCheck %s --match-full-lines --check-prefix YAML
-// RUN: %clang_pgogen -o %t.no.dbg -mllvm --debug-info-correlate -mllvm --disable-vp=true %s
+// RUN: %clang_pgogen -o %t.no.dbg -mllvm --profile-correlate=debug-info -mllvm --disable-vp=true %s
// RUN: not llvm-profdata show --debug-info=%t.no.dbg 2>&1 | FileCheck %s --check-prefix NO-DBG
-// NO-DBG: unable to correlate profile: could not find any profile metadata in debug info
+// NO-DBG: unable to correlate profile: could not find any profile data metadata in correlated file
// YAML: Probes:
// YAML: - Function Name: a
diff --git a/llvm/docs/CommandGuide/llvm-profdata.rst b/llvm/docs/CommandGuide/llvm-profdata.rst
index be42733ca140567..73759f14b49a9d8 100644
--- a/llvm/docs/CommandGuide/llvm-profdata.rst
+++ b/llvm/docs/CommandGuide/llvm-profdata.rst
@@ -195,7 +195,7 @@ OPTIONS
.. option:: --debug-info=<path>
Specify the executable or ``.dSYM`` that contains debug info for the raw profile.
- When ``-debug-info-correlate`` was used for instrumentation, use this option
+ When ``-profile-correlate=debug-info`` was used for instrumentation, use this option
to correlate the raw profile.
.. option:: --temporal-profile-trace-reservoir-size
@@ -346,7 +346,7 @@ OPTIONS
.. option:: --debug-info=<path>
Specify the executable or ``.dSYM`` that contains debug info for the raw profile.
- When ``-debug-info-correlate`` was used for instrumentation, use this option
+ When ``-profile-correlate=debug-info`` was used for instrumentation, use this option
to show the correlated functions from the raw profile.
.. option:: --covered
diff --git a/llvm/include/llvm/ProfileData/InstrProfCorrelator.h b/llvm/include/llvm/ProfileData/InstrProfCorrelator.h
index dd062951f277c1c..a3a0805a294a20c 100644
--- a/llvm/include/llvm/ProfileData/InstrProfCorrelator.h
+++ b/llvm/include/llvm/ProfileData/InstrProfCorrelator.h
@@ -31,8 +31,11 @@ class ObjectFile;
/// to their functions.
class InstrProfCorrelator {
public:
+ /// Indicate which kind correlator to use.
+ enum ProfCorrelatorKind { NONE, DEBUG_INFO };
+
static llvm::Expected<std::unique_ptr<InstrProfCorrelator>>
- get(StringRef DebugInfoFilename);
+ get(StringRef Filename, ProfCorrelatorKind FileKind);
/// Construct a ProfileData vector used to correlate raw instrumentation data
/// to their functions.
@@ -104,7 +107,7 @@ class InstrProfCorrelator {
private:
static llvm::Expected<std::unique_ptr<InstrProfCorrelator>>
- get(std::unique_ptr<MemoryBuffer> Buffer);
+ get(std::unique_ptr<MemoryBuffer> Buffer, ProfCorrelatorKind FileKind);
const InstrProfCorrelatorKind Kind;
};
@@ -128,7 +131,7 @@ class InstrProfCorrelatorImpl : public InstrProfCorrelator {
static llvm::Expected<std::unique_ptr<InstrProfCorrelatorImpl<IntPtrT>>>
get(std::unique_ptr<InstrProfCorrelator::Context> Ctx,
- const object::ObjectFile &Obj);
+ const object::ObjectFile &Obj, ProfCorrelatorKind FileKind);
protected:
std::vector<RawInstrProf::ProfileData<IntPtrT>> Data;
@@ -138,6 +141,8 @@ class InstrProfCorrelatorImpl : public InstrProfCorrelator {
int MaxWarnings,
InstrProfCorrelator::CorrelationData *Data = nullptr) = 0;
+ virtual Error correlateProfileNameImpl() = 0;
+
Error dumpYaml(int MaxWarnings, raw_ostream &OS) override;
void addProbe(StringRef FunctionName, uint64_t CFGHash, IntPtrT CounterOffset,
@@ -205,6 +210,8 @@ class DwarfInstrProfCorrelator : public InstrProfCorrelatorImpl<IntPtrT> {
void correlateProfileDataImpl(
int MaxWarnings,
InstrProfCorrelator::CorrelationData *Data = nullptr) override;
+
+ Error correlateProfileNameImpl() override;
};
} // end namespace llvm
diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h
index 5f54cbeb1b01eda..dc0e921d82768b6 100644
--- a/llvm/include/llvm/ProfileData/InstrProfReader.h
+++ b/llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -374,6 +374,8 @@ class RawInstrProfReader : public InstrProfReader {
...
[truncated]
|
david-xl
approved these changes
Oct 19, 2023
ellishg
reviewed
Oct 23, 2023
ellishg
reviewed
Oct 24, 2023
| @@ -55,6 +56,7 @@ | |||
| #include "llvm/Target/TargetOptions.h" | |||
| #include "llvm/TargetParser/SubtargetFeature.h" | |||
| #include "llvm/TargetParser/Triple.h" | |||
| #include "llvm/Transforms/HipStdPar/HipStdPar.h" | |||
There was a problem hiding this comment.
It's not added. It's reordered by git-clang-format.
Comment on lines
29
to
42
| cl::opt<bool> DebugInfoCorrelate( | ||
| "debug-info-correlate", | ||
| cl::desc("Use debug info to correlate profiles (Deprecated). Use " | ||
| "-profile-correlate=debug-info instead."), | ||
| cl::init(false)); | ||
|
|
||
| cl::opt<InstrProfCorrelator::ProfCorrelatorKind> ProfileCorrelate( | ||
| "profile-correlate", | ||
| cl::desc("Use debug info or binary file to correlate profiles."), | ||
| cl::init(InstrProfCorrelator::NONE), | ||
| cl::values(clEnumValN(InstrProfCorrelator::NONE, "", | ||
| "No profile correlation"), | ||
| clEnumValN(InstrProfCorrelator::DEBUG_INFO, "debug-info", | ||
| "Use debug info to correlate"))); |
There was a problem hiding this comment.
This patch moves these options from InstrProfiling.cpp to InstrProfCorrelator.cpp, but this file doesn't actually use these flags. Is there any reason why we can't keep the flags in InstrProfiling.cpp?
There was a problem hiding this comment.
Because -profile-correlate=binary (though not added in this patch yet) will be used by TargetLoweringObjectFileImpl.cpp, if keeping those flags definitions in InstrProfiling.cpp, I got a linker error:
[4167/5493] Linking CXX executable unittests/Target/PowerPC/PowerPCTests
...
ld.lld: error: undefined symbol: llvm::ProfileCorrelate
>>> referenced by TargetLoweringObjectFileImpl.cpp
>>> TargetLoweringObjectFileImpl.cpp.o:(selectExplicitSectionGlobal(llvm::GlobalObject const*, llvm::SectionKind, llvm::TargetMachine const&, llvm::MCContext&, llvm::Mangler&, unsigned int&, bool, bool)) in archive lib/libLLVMCodeGen.a
There was a problem hiding this comment.
Do you mean Keeping DebugInfoCorrelate option can cause the linker error?
There was a problem hiding this comment.
No, if I kept ProfileCorrelate option in InstrProfiling.cpp but used it inTargetLoweringObjectFileImpl.cpp for binary correlation (#69493) will cause linking error when linking unittests/Target/PowerPC/PowerPCTests.
Yeah, we can keep DebugInfoCorrelate option in InstrProfiling.cpp but I think put them together makes it easier to understand -debug-info-correlate is an alias of -profile-correlate=binary and will be deprecated later.
There was a problem hiding this comment.
Use extern decl and add some comments.
There was a problem hiding this comment.
so keeping the option in InstrProfCorrelator.cpp works fine without linker error? That works for me.
There was a problem hiding this comment.
What if we move it to PGOInstrumentation.cpp?
There was a problem hiding this comment.
so keeping the option in InstrProfCorrelator.cpp works fine without linker error? That works for me.
Yes, PowerPCTests will linked successfully.
What if we move it to
PGOInstrumentation.cpp?
It still doesn't work. I guess the flags needs to be in one of those files:
There was a problem hiding this comment.
I guess the root of the problem is that Instrumentation depends on ProfileData, so we can't have InstrProfCorrelator.cpp use a symbol defined in Instrumentation. That is unfortunate because -debug-info-correlate (-profile-correlate=debug-info) is a codegen flag while InstrProfCorrelator.cpp is primarily used by llvm-prodata which will not use -debug-info-correlate.
I really think -debug-info-correlate does not belong in InstrProfCorrelator.cpp. I'm fine with keeping InstrProfCorrelator::ProfCorrelatorKind in this file, but the the flags should stay in InstrProfiling.cpp and if needed we can pass ProfCorrelatorKind as a parameter.
There was a problem hiding this comment.
I guess the root of the problem is that
Instrumentationdepends onProfileData, so we can't haveInstrProfCorrelator.cppuse a symbol defined inInstrumentation.
Probably I didn't make it clear. We can keep -debug-info-correlate in InstrProfiling.cpp.
In my PR, I added a check for -profile-correlate=binary in TargetLoweringObjectFileImpl.cpp: , to determine if we should mark profile name & data sections as strippable, which requires -profile-correlate flag being defined in TargetLoweringObjectFileImpl.cpp in order to linked successfully with other targets that doesn't use InstrProfiling.cpp. That makes two flags separated in two files, so I moved them together to InstrProfCorrelator.cpp.
That is unfortunate because
-debug-info-correlate(-profile-correlate=debug-info) is a codegen flag whileInstrProfCorrelator.cppis primarily used byllvm-prodatawhich will not use-debug-info-correlate.
I really think-debug-info-correlatedoes not belong inInstrProfCorrelator.cpp. I'm fine with keepingInstrProfCorrelator::ProfCorrelatorKindin this file, but the the flags should stay inInstrProfiling.cppand if needed we can passProfCorrelatorKindas a parameter.
I didn't think about that before. I agree it makes less sense to put codegen flags in InstrProfCorrelator.cpp. I think this can be avoided by moving this flag to llvm/lib/CodeGen/CommandFlags.cpp. I'll take a look later.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Two `OpOperand`s are the same if they belong to the same owner and have the same operand number. There are currently no comparison operators defined on `OpOperand` and we work around this in multiple places by comparing pointers. Note: `OpOperand`s are stored in an op, so it is valid to compare their pointers to determine if they are the same operand. E.g., `getOperandNumber` is also implemented via pointer arithmetics.
GPRF64 represents a pair of registers. We were only copying the even part. We need to copy the odd part too.
…nAnalysis.cpp (NFC)
…tic assertion regexes in our test suite" Fails like https://lab.llvm.org/buildbot/#/builders/238/builds/6278/steps/11/logs/stdio on many bots. This reverts commit 166b3a8.
…ring (llvm#70317) The front-end is making implicit conversions explicit in assignment and structure constructors. While this generally helps and is needed by semantics to fold structure constructors correctly, this is incorrect when the LHS or component is an allocatable. The RHS may have non default lower bounds that should be propagated to the LHS, and making the conversion explicit changes the semantics. In the structure constructor, the situation is even worse since Fortran 2018 7.5.10 point 7 allows the value to be a reference to an unallocated allocatable, and adding an explicit conversion in semantics will cause a segfault. This patch removes the explicit convert in semantics when the LHS/component is a whole allocatable, and update lowering to deal with the conversion insertion, dealing with preserving the lower bounds and the tricky structure constructor case.
Solves crash mentioned in llvm#65920.
…xed,Scalable}. NFC
While extracting the existing functionality into a function, one of the variable usages wasn't correctly updated.
…m#69323) The main README.md should probably be kept pretty short and be used to point new-comers to the most essential ways to get started on or get involved with LLVM. Therefore, this patch removes a pointer to IRC (not used very much these days), and does add pointers to office hours and online sync-ups.
Fix discrepancy from when this was forked from the SkylakeServer model Confirmed with Agner + uops.info
AArch64 backend now features v8.4a atomic Load-Acquire RCpc and Store-Release register unscaled support.
I think it follows from the HSA spec that a write to the first byte is deemed significant to the GPU in which case writing to the second short and reading back from it later would be safe. However, the examples for this all involve an atomic write to the first 32 bits and it seems a credible risk that the occasional CI errors abound invalid packets have as their root cause that the firmware notices the early write to packet->setup and treats that as a sign that the packet is ready to go. That was overly-paranoid, however in passing noticed the code in libc is genuinely invalid. The memset writes a zero to the header byte, changing it from type_invalid (1) to type_vendor (0), at which point the GPU is free to read the 64 byte packet and interpret it as a vendor packet, which is probably why libc CI periodically errors about invalid packets. Also a drive by change to do the atomic store on a uint32_t consistently. I'm not sure offhand what __atomic_store_n on a uint16_t* and an int resolves to, seems better to be unambiguous there.
To reduce the spurious test delta in an upcoming change.
…d sizes handling (llvm#70469) * Enhanced the logic of ExpandMemCmp pass to merge contiguous subsequences in LoadSequence, based on sizes allowed in `AllowedTailExpansions`. * This enhancement seeks to minimize the number of basic blocks and produce optimized code when using memcmp with non-register aligned sizes. * Enable this feature for AArch64 with memcmp sizes modulo 8 equal to 3, 5, and 6. Reapplication of llvm#69942 after fixing a bug
llvm#68733) The calls to std::construct_at might overwrite the previously set __has_value_ flag in the case where the flag is overlapping with the actual value or error being stored (since we use [[no_unique_address]]). To fix this issue, this patch ensures that we initialize the __has_value_ flag after we call std::construct_at. Fixes llvm#68552
This patch backports a one-liner `std::to_underlying` that came with C++23. This is useful for refactoring unscoped enums into scoped enums, because the latter are not implicitly convertible to integer types. I followed libc++ implementation, but I consider their testing too heavy for us, so I wrote a simpler set of tests.
…lvm#70706) Builds on llvm#67982 which recently introduced the nneg flag on a zext instruction. InstCombine is one of our largest canonicalizers of zext from non-negative sext instructions, so set the flag there.
ZequanWu
requested review from
JDevlieghere,
Endilll,
nikic and
a team
as code owners
|
Sorry for the spam. I tried to resolve the conflict by rebasing and push, but it adds all commits between second last change and last change into this pr. I'll close it and restart a new one. |
Yeah I also don't have a ton of experience with GitHub, I think most of us are still learning :) |
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.
Refactor some code from #69493.