https://github.com/snehasish approved this pull request.
lgtm
https://github.com/llvm/llvm-project/pull/119791
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling :
BoolFOption<"debug-info-for-profiling",
PosFlag,
NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"],
"fprofile-generate-cold-function-coverage">,
snehasish wrote:
> Is it that the b
@@ -1784,6 +1784,12 @@ defm debug_info_for_profiling :
BoolFOption<"debug-info-for-profiling",
PosFlag,
NegFlag>;
+def fprofile_generate_cold_function_coverage : Flag<["-"],
"fprofile-generate-cold-function-coverage">,
snehasish wrote:
> also to central
snehasish wrote:
> The major motivation is to detect dead functions for the services that are
> optimized with sampling PGO.
For your use case, can you use
[ProfileSymbolList](https://github.com/llvm/llvm-project/blob/32ffc9fdc2cd422c88c926b862adb3de726e3888/llvm/include/llvm/ProfileData/Sampl
@@ -431,23 +441,34 @@ class InstrProfSymtab {
using AddrHashMap = std::vector>;
private:
+ using AddrIntervalMap =
+ IntervalMap>;
StringRef Data;
uint64_t Address = 0;
- // Unique name strings.
+ // Unique name strings. Used to ensure entries in MD5NameMap (a
@@ -0,0 +1,145 @@
+// REQUIRES: lld-available
+
+// RUN: rm -rf %t && split-file %s %t && cd %t
+//
+// RUN: %clangxx_pgogen -fuse-ld=lld -O2 -g -fprofile-generate=. -mllvm
-enable-vtable-value-profiling test.cpp -o test
+// RUN: env LLVM_PROFILE_FILE=test.profraw ./test
@@ -0,0 +1,145 @@
+// REQUIRES: lld-available
+
+// RUN: rm -rf %t && split-file %s %t && cd %t
+//
+// RUN: %clangxx_pgogen -fuse-ld=lld -O2 -g -fprofile-generate=. -mllvm
-enable-vtable-value-profiling test.cpp -o test
+// RUN: env LLVM_PROFILE_FILE=test.profraw ./test
+
+// Sh
@@ -1362,8 +1372,8 @@ remapSamples(const sampleprof::FunctionSamples &Samples,
BodySample.second.getSamples());
for (const auto &Target : BodySample.second.getCallTargets()) {
Result.addCalledTargetSamples(BodySample.first.LineOffset,
-
https://github.com/snehasish approved this pull request.
lgtm
https://github.com/llvm/llvm-project/pull/66825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -676,24 +722,66 @@ TEST_P(InstrProfReaderWriterTest, icall_data_read_write) {
Expected R = Reader->getInstrProfRecord("caller", 0x1234);
ASSERT_THAT_ERROR(R.takeError(), Succeeded());
+
+ // Test the number of instrumented indirect call sites and the number of
+ // pr
@@ -676,24 +722,66 @@ TEST_P(InstrProfReaderWriterTest, icall_data_read_write) {
Expected R = Reader->getInstrProfRecord("caller", 0x1234);
ASSERT_THAT_ERROR(R.takeError(), Succeeded());
+
+ // Test the number of instrumented indirect call sites and the number of
+ // pr
@@ -0,0 +1,145 @@
+// REQUIRES: lld-available
+
+// RUN: rm -rf %t && split-file %s %t && cd %t
+//
+// RUN: %clangxx_pgogen -fuse-ld=lld -O2 -g -fprofile-generate=. -mllvm
-enable-vtable-value-profiling test.cpp -o test
+// RUN: env LLVM_PROFILE_FILE=test.profraw ./test
+
+// Sh
@@ -0,0 +1,145 @@
+// REQUIRES: lld-available
+
+// RUN: rm -rf %t && split-file %s %t && cd %t
snehasish wrote:
I think you can skip this RUN line and just use `%s` as the input file in the
line pgogen invocation below.
https://github.com/llvm/llvm-project/pul
https://github.com/snehasish edited
https://github.com/llvm/llvm-project/pull/66825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/snehasish edited
https://github.com/llvm/llvm-project/pull/66825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -560,6 +602,28 @@ Error InstrProfSymtab::addFuncWithName(Function &F,
StringRef PGOFuncName) {
return Error::success();
}
+uint64_t InstrProfSymtab::getVTableHashFromAddress(uint64_t Address) {
+ finalizeSymtab();
snehasish wrote:
Summarizing offline d
https://github.com/snehasish commented:
Responses to a couple of other comments -
> * For LLVM IR test coverage, store textual profiles in the repo, and run
> `llvm-profdata merge` to convert it to indexed profiles.
> * To have test coverage on raw profiles (generate raw profiles or convert it
@@ -560,6 +602,28 @@ Error InstrProfSymtab::addFuncWithName(Function &F,
StringRef PGOFuncName) {
return Error::success();
}
+uint64_t InstrProfSymtab::getVTableHashFromAddress(uint64_t Address) {
+ finalizeSymtab();
snehasish wrote:
Add a comment why its
@@ -1237,6 +1259,101 @@ void InstrLowerer::maybeSetComdat(GlobalVariable *GV,
Function *Fn,
GV->setLinkage(GlobalValue::InternalLinkage);
}
+static inline bool shouldRecordVTableAddr(GlobalVariable *GV) {
+ if (!profDataReferencedByCode(*GV->getParent()))
+return fal
snehasish wrote:
> > Here are some things I noticed, haven't looked at the tests yet.
>
> @snehasish Thanks for the review!
>
> Talking about tests, I wonder if it looks better if I change the current LLVM
> IR test under `llvm/test/tools/llvm-profdata/` into a compiler-rt test under
> `compi
@@ -429,20 +439,36 @@ uint64_t ComputeHash(StringRef K);
class InstrProfSymtab {
public:
using AddrHashMap = std::vector>;
+ using RangeHashMap =
+ std::vector, uint64_t>>;
private:
StringRef Data;
uint64_t Address = 0;
- // Unique name strings.
+ // Unique n
@@ -560,6 +602,28 @@ Error InstrProfSymtab::addFuncWithName(Function &F,
StringRef PGOFuncName) {
return Error::success();
}
+uint64_t InstrProfSymtab::getVTableHashFromAddress(uint64_t Address) {
+ finalizeSymtab();
snehasish wrote:
Like the lookup for G
@@ -429,20 +439,36 @@ uint64_t ComputeHash(StringRef K);
class InstrProfSymtab {
public:
using AddrHashMap = std::vector>;
+ using RangeHashMap =
snehasish wrote:
Hmm, it's a little strange to have HashMap in the name and the underlying data
structure is a
@@ -490,6 +591,23 @@ Error InstrProfSymtab::addFuncWithName(Function &F,
StringRef PGOFuncName) {
return Error::success();
}
+uint64_t InstrProfSymtab::getVTableHashFromAddress(uint64_t Address) {
+ finalizeSymtab();
+ auto It = lower_bound(
+ VTableAddrRangeToMD5Map
@@ -567,6 +643,21 @@ Error InstrProfSymtab::create(const NameIterRange
&IterRange) {
return Error::success();
}
+template
+Error InstrProfSymtab::create(const FuncNameIterRange &FuncIterRange,
+ const VTableNameIterRange &VTableIterRange) {
+
@@ -327,6 +327,11 @@ extern cl::opt PGOViewCounts;
// Defined in Analysis/BlockFrequencyInfo.cpp: -view-bfi-func-name=
extern cl::opt ViewBlockFreqFuncName;
+extern cl::opt DebugInfoCorrelate;
snehasish wrote:
Add a comment for this like the ones above.
htt
@@ -496,32 +532,70 @@ class InstrProfSymtab {
/// Create InstrProfSymtab from a set of names iteratable from
/// \p IterRange. This interface is used by IndexedProfReader.
- template Error create(const NameIterRange
&IterRange);
-
- /// Update the symtab by adding \p F
@@ -378,6 +384,13 @@ std::string getPGOFuncName(const Function &F, bool InLTO,
uint64_t Version) {
return getPGOFuncName(F.getName(), GlobalValue::ExternalLinkage, "");
}
+std::string getPGOName(const GlobalVariable &V, bool InLTO) {
+ // PGONameMetadata should be set by c
@@ -429,20 +439,36 @@ uint64_t ComputeHash(StringRef K);
class InstrProfSymtab {
public:
using AddrHashMap = std::vector>;
+ using RangeHashMap =
+ std::vector, uint64_t>>;
snehasish wrote:
Can you change the element type to a structure with 3 elements
@@ -1237,6 +1259,101 @@ void InstrLowerer::maybeSetComdat(GlobalVariable *GV,
Function *Fn,
GV->setLinkage(GlobalValue::InternalLinkage);
}
+static inline bool shouldRecordVTableAddr(GlobalVariable *GV) {
+ if (!profDataReferencedByCode(*GV->getParent()))
+return fal
@@ -1237,6 +1259,101 @@ void InstrLowerer::maybeSetComdat(GlobalVariable *GV,
Function *Fn,
GV->setLinkage(GlobalValue::InternalLinkage);
}
+static inline bool shouldRecordVTableAddr(GlobalVariable *GV) {
+ if (!profDataReferencedByCode(*GV->getParent()))
+return fal
@@ -1237,6 +1259,101 @@ void InstrLowerer::maybeSetComdat(GlobalVariable *GV,
Function *Fn,
GV->setLinkage(GlobalValue::InternalLinkage);
}
+static inline bool shouldRecordVTableAddr(GlobalVariable *GV) {
+ if (!profDataReferencedByCode(*GV->getParent()))
+return fal
@@ -1237,6 +1259,101 @@ void InstrLowerer::maybeSetComdat(GlobalVariable *GV,
Function *Fn,
GV->setLinkage(GlobalValue::InternalLinkage);
}
+static inline bool shouldRecordVTableAddr(GlobalVariable *GV) {
+ if (!profDataReferencedByCode(*GV->getParent()))
+return fal
@@ -16,23 +16,72 @@
#include
namespace llvm {
-// Visitor class that finds all indirect call.
+// Visitor class that finds indirect calls or instructions that gives vtable
+// value, depending on Type.
struct PGOIndirectCallVisitor : public InstVisitor {
+ enum class Instru
https://github.com/snehasish edited
https://github.com/llvm/llvm-project/pull/66825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/snehasish commented:
Here are some things I noticed, haven't looked at the tests yet.
https://github.com/llvm/llvm-project/pull/66825
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/list
@@ -459,6 +472,16 @@ Error InstrProfSymtab::create(Module &M, bool InLTO) {
if (Error E = addFuncWithName(F, getPGOFuncName(F, InLTO)))
return E;
}
+
+ SmallVector Types;
+ for (GlobalVariable &G : M.globals()) {
+if (!G.hasName())
+ continue;
+Types.
@@ -605,6 +703,19 @@ Function* InstrProfSymtab::getFunction(uint64_t
FuncMD5Hash) {
return nullptr;
}
+GlobalVariable *
+InstrProfSymtab::getGlobalVariable(uint64_t GlobalVariableMD5Hash) {
+ finalizeSymtab();
snehasish wrote:
Why do we need to finalizeSy
snehasish wrote:
Any measurements of impact to compile time, memory etc on a reasonable size
benchmark (e.g. clang)?
https://github.com/llvm/llvm-project/pull/81545
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin
@@ -0,0 +1,38 @@
+// RUN: %clang_profgen %s --target=ppc64le-unknown-linux-gnu -S \
+// RUN:-emit-llvm -o - | FileCheck %s --check-prefix=PROFGEN
+// RUN: %clang_profgen -o %t %s
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
snehasish wrote:
Good point, c
@@ -0,0 +1,38 @@
+// RUN: %clang_profgen %s --target=ppc64le-unknown-linux-gnu -S \
+// RUN:-emit-llvm -o - | FileCheck %s --check-prefix=PROFGEN
+// RUN: %clang_profgen -o %t %s
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
snehasish wrote:
I think you n
https://github.com/snehasish approved this pull request.
lgtm but please wait for other reviewers to chime in. Thanks!
https://github.com/llvm/llvm-project/pull/76471
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bi
https://github.com/snehasish edited
https://github.com/llvm/llvm-project/pull/76471
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1364,12 +1364,22 @@ static void InitializePredefinedMacros(const TargetInfo
&TI,
TI.getTargetDefines(LangOpts, Builder);
}
+static void InitializePGOProfileMacros(const CodeGenOptions &CodeGenOpts,
+ MacroBuilder &Builder) {
+ if (
@@ -12,6 +12,19 @@
#include "InstrProfilingPort.h"
#include
+// Make sure __LLVM_INSTR_PROFILE_GENERATE is always defined before
+// including instr_prof_interface.h so the interface functions are
+// declared correctly for the runtime. Additionally, make sure
+// that __LLVM
@@ -0,0 +1,17 @@
+// RUN: %clang_pgogen -o %t %s
+// RUN: not %t
+// RUN: %clang -o %t %s
+// RUN: %t
+
+__attribute__((weak)) void __llvm_profile_reset_counters(void);
+
+__attribute__((noinline)) int bar() { return 4; }
+int foo() {
+ if (__llvm_profile_reset_counters) {
+_
https://github.com/snehasish commented:
Thanks for your patience with my comments!
https://github.com/llvm/llvm-project/pull/76471
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/snehasish edited
https://github.com/llvm/llvm-project/pull/76471
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1364,12 +1364,22 @@ static void InitializePredefinedMacros(const TargetInfo
&TI,
TI.getTargetDefines(LangOpts, Builder);
}
+static void InitializePGOProfileMacros(const CodeGenOptions &CodeGenOpts,
snehasish wrote:
Yes, my previous comment about other
https://github.com/snehasish edited
https://github.com/llvm/llvm-project/pull/76471
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -100,12 +103,6 @@ ValueProfNode *__llvm_profile_begin_vnodes();
ValueProfNode *__llvm_profile_end_vnodes();
uint32_t *__llvm_profile_begin_orderfile();
-/*!
- * \brief Clear profile counters to zero.
- *
- */
-void __llvm_profile_reset_counters(void);
sneha
@@ -44,6 +44,7 @@ endif(COMPILER_RT_BUILD_ORC)
if (COMPILER_RT_BUILD_PROFILE)
set(PROFILE_HEADERS
profile/InstrProfData.inc
+profile/instr_prof_interface.h
snehasish wrote:
Good point, looks like we treat `interface` filenames with snake case as
opp
@@ -100,12 +103,6 @@ ValueProfNode *__llvm_profile_begin_vnodes();
ValueProfNode *__llvm_profile_end_vnodes();
uint32_t *__llvm_profile_begin_orderfile();
-/*!
- * \brief Clear profile counters to zero.
- *
- */
-void __llvm_profile_reset_counters(void);
sneha
@@ -1364,12 +1364,22 @@ static void InitializePredefinedMacros(const TargetInfo
&TI,
TI.getTargetDefines(LangOpts, Builder);
}
+static void InitializePGOProfileMacros(const CodeGenOptions &CodeGenOpts,
snehasish wrote:
There are IR PGO users (e.g. Rust) wh
@@ -44,6 +44,7 @@ endif(COMPILER_RT_BUILD_ORC)
if (COMPILER_RT_BUILD_PROFILE)
set(PROFILE_HEADERS
profile/InstrProfData.inc
+profile/instr_prof_interface.h
snehasish wrote:
Can you rename the header to follow the existing CamelCase convention?
http
@@ -0,0 +1,26 @@
+// RUN: %clang_profgen %s -S -emit-llvm -o - | FileCheck %s
--check-prefix=PROFGEN
+// RUN: %clang_profgen -o %t %s
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-profdata merge -o %t.profdata %t.profraw
+// RUN: %clang_profuse=%t.profdata %s -S
@@ -100,12 +103,6 @@ ValueProfNode *__llvm_profile_begin_vnodes();
ValueProfNode *__llvm_profile_end_vnodes();
uint32_t *__llvm_profile_begin_orderfile();
-/*!
- * \brief Clear profile counters to zero.
- *
- */
-void __llvm_profile_reset_counters(void);
sneha
snehasish wrote:
@teresajohnson I mentioned the same thing on
[discourse](https://discourse.llvm.org/t/pgo-are-the-llvm-profile-functions-stable-c-apis-across-llvm-releases/75832/5)
but it seems like linking on AIX does not support this model.
https://github.com/llvm/llvm-project/pull/76471
__
snehasish wrote:
> change the matcher (the latter may be the easiest).
My response assumed that it was already using the IRPGOFuncName for matching.
Yes, I think we should make the switch then.
> It seems dwarf don't record linkage type.
Yes, you're right. As an alternative can we use the sym
snehasish wrote:
@lifengxiang1025 thanks for flagging this issue. I think it's best to not rely
on unique-internal-linkage-name here. Instead we should extend the logic in
RawMemProfReader.cpp to include ";" if the function is internal
linkage as expected by IRPGOFuncName. Can you try this ap
@@ -453,11 +471,94 @@ Error InstrProfSymtab::create(Module &M, bool InLTO) {
if (Error E = addFuncWithName(F, getPGOFuncName(F, InLTO)))
return E;
}
+
+ SmallVector Types;
+ for (GlobalVariable &G : M.globals()) {
+if (!G.hasName())
+ continue;
+Types
@@ -123,19 +137,29 @@ static int needsCounterPadding(void) {
COMPILER_RT_VISIBILITY
void __llvm_profile_get_padding_sizes_for_counters(
uint64_t DataSize, uint64_t CountersSize, uint64_t NumBitmapBytes,
-uint64_t NamesSize, uint64_t *PaddingBytesBeforeCounters,
-uin
@@ -12,27 +12,78 @@
#ifndef LLVM_ANALYSIS_INDIRECTCALLVISITOR_H
#define LLVM_ANALYSIS_INDIRECTCALLVISITOR_H
+#include "llvm/ADT/SetVector.h"
#include "llvm/IR/InstVisitor.h"
#include
namespace llvm {
-// Visitor class that finds all indirect call.
+// Visitor class that
@@ -489,30 +520,66 @@ class InstrProfSymtab {
/// \p IterRange. This interface is used by IndexedProfReader.
template Error create(const NameIterRange
&IterRange);
- /// Update the symtab by adding \p FuncName to the table. This interface
- /// is used by the raw and t
@@ -1070,8 +1084,138 @@ void InstrProfiling::maybeSetComdat(GlobalVariable *GV,
Function *Fn,
GV->setLinkage(GlobalValue::InternalLinkage);
}
-GlobalVariable *InstrProfiling::setupProfileSection(InstrProfInstBase *Inc,
-
@@ -275,18 +282,28 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const
__llvm_profile_data *DataBegin,
const uint64_t NumBitmapBytes =
__llvm_profile_get_num_bitmap_bytes(BitmapBegin, BitmapEnd);
const uint64_t NamesSize = __llvm_profile_get_name_size(NamesBegin,
Na
@@ -1441,6 +1531,9 @@ void OverlapStats::dump(raw_fd_ostream &OS) const {
case IPVK_MemOPSize:
strncpy(ProfileKindName, "MemOP", 19);
break;
+case IPVK_VTableTarget:
+ strncpy(ProfileKindName, "VTable", 6);
snehasish wrote:
I think thi
@@ -1070,8 +1084,138 @@ void InstrProfiling::maybeSetComdat(GlobalVariable *GV,
Function *Fn,
GV->setLinkage(GlobalValue::InternalLinkage);
}
-GlobalVariable *InstrProfiling::setupProfileSection(InstrProfInstBase *Inc,
-
https://github.com/snehasish requested changes to this pull request.
It would be nice to split this patch into at least two parts:
1. Add basic support for Darwin (malloc implementation, build ids, raw format
etc)
2. Record and dump references to static data in the binary
I think there are a n
snehasish wrote:
> @david-xl It's interesting to know this. How is that going on your end?
Currently we are exploring the design space to accommodate the variety of
platforms and FDO types we use internally. This is a priority for us though so
we should have some updates to share externally by
Author: Snehasish Kumar
Date: 2023-03-13T20:09:46Z
New Revision: debe80cb8d50275849d8d0b9357321eb7985b854
URL:
https://github.com/llvm/llvm-project/commit/debe80cb8d50275849d8d0b9357321eb7985b854
DIFF:
https://github.com/llvm/llvm-project/commit/debe80cb8d50275849d8d0b9357321eb7985b854.diff
LO
Author: Snehasish Kumar
Date: 2023-03-13T19:28:38Z
New Revision: 287177a47a396ca6cc0bef7696108cdaa0c68e5f
URL:
https://github.com/llvm/llvm-project/commit/287177a47a396ca6cc0bef7696108cdaa0c68e5f
DIFF:
https://github.com/llvm/llvm-project/commit/287177a47a396ca6cc0bef7696108cdaa0c68e5f.diff
LO
Author: Snehasish Kumar
Date: 2023-03-09T19:54:23Z
New Revision: e99b5ad38381ab263820b23a184d217a4112519c
URL:
https://github.com/llvm/llvm-project/commit/e99b5ad38381ab263820b23a184d217a4112519c
DIFF:
https://github.com/llvm/llvm-project/commit/e99b5ad38381ab263820b23a184d217a4112519c.diff
LO
Author: Snehasish Kumar
Date: 2020-09-18T15:08:00-07:00
New Revision: b86f1af423952d9f1dbe105b651b948ce0e1e8d0
URL:
https://github.com/llvm/llvm-project/commit/b86f1af423952d9f1dbe105b651b948ce0e1e8d0
DIFF:
https://github.com/llvm/llvm-project/commit/b86f1af423952d9f1dbe105b651b948ce0e1e8d0.dif
Author: Snehasish Kumar
Date: 2020-09-15T12:41:58-07:00
New Revision: f1a3ab904439a63b21ba1c4521765c46630687c6
URL:
https://github.com/llvm/llvm-project/commit/f1a3ab904439a63b21ba1c4521765c46630687c6
DIFF:
https://github.com/llvm/llvm-project/commit/f1a3ab904439a63b21ba1c4521765c46630687c6.dif
Author: Snehasish Kumar
Date: 2020-09-10T00:15:33-07:00
New Revision: 157cd93b48a90f484e9eb2ed9997e0372b9c7ebb
URL:
https://github.com/llvm/llvm-project/commit/157cd93b48a90f484e9eb2ed9997e0372b9c7ebb
DIFF:
https://github.com/llvm/llvm-project/commit/157cd93b48a90f484e9eb2ed9997e0372b9c7ebb.dif
76 matches
Mail list logo
