https://github.com/minglotus-6 closed
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
@@ -1362,8 +1372,8 @@ remapSamples(const sampleprof::FunctionSamples ,
BodySample.second.getSamples());
for (const auto : BodySample.second.getCallTargets()) {
Result.addCalledTargetSamples(BodySample.first.LineOffset,
-
@@ -0,0 +1,145 @@
+// REQUIRES: lld-available
+
+// RUN: rm -rf %t && split-file %s %t && cd %t
minglotus-6 wrote:
done.
https://github.com/llvm/llvm-project/pull/66825
___
cfe-commits mailing list
@@ -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
+
+//
@@ -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
+ //
@@ -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
+
+//
@@ -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
+ //
@@ -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
@@ -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
+
+//
@@ -1362,8 +1372,8 @@ remapSamples(const sampleprof::FunctionSamples ,
BodySample.second.getSamples());
for (const auto : 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
+ //
@@ -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
+ //
@@ -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
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/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
minglotus-6 wrote:
> I think we should resort to scripts and profraw in LLVM if we don't have
> support for textual format. This is the case for memprof for example.
Got it. I updated the test cases. Now compiler-rt test provides raw-related
test coverage, and IR test provides test coverage
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 ,
StringRef PGOFuncName) {
return Error::success();
}
+uint64_t InstrProfSymtab::getVTableHashFromAddress(uint64_t Address) {
+ finalizeSymtab();
snehasish wrote:
Summarizing offline
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
minglotus-6 wrote:
> My concern with this approach is that compiler-rt is treated as a different
> project and updating the code within LLVM makes it easy to miss running the
> test locally for the other project. I think such issues will be caught by the
> buildbot but having it flagged
@@ -560,6 +602,28 @@ Error InstrProfSymtab::addFuncWithName(Function ,
StringRef PGOFuncName) {
return Error::success();
}
+uint64_t InstrProfSymtab::getVTableHashFromAddress(uint64_t Address) {
+ finalizeSymtab();
minglotus-6 wrote:
Added a comment.
@@ -560,6 +602,28 @@ Error InstrProfSymtab::addFuncWithName(Function ,
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
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
>
https://github.com/minglotus-6 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/minglotus-6 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/minglotus-6 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
@@ -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
@@ -605,6 +703,19 @@ Function* InstrProfSymtab::getFunction(uint64_t
FuncMD5Hash) {
return nullptr;
}
+GlobalVariable *
+InstrProfSymtab::getGlobalVariable(uint64_t GlobalVariableMD5Hash) {
+ finalizeSymtab();
minglotus-6 wrote:
Indeed. With a DenseMap
@@ -538,14 +541,30 @@ Error RawInstrProfReader::readNextHeader(const
char *CurrentPos) {
template
Error RawInstrProfReader::createSymtab(InstrProfSymtab ) {
- if (Error E = Symtab.create(StringRef(NamesStart, NamesEnd - NamesStart)))
+ if (Error E =
@@ -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
@@ -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
@@ -567,6 +643,21 @@ Error InstrProfSymtab::create(const NameIterRange
) {
return Error::success();
}
+template
+Error InstrProfSymtab::create(const FuncNameIterRange ,
+ const VTableNameIterRange ) {
+ for (auto Name : FuncIterRange)
@@ -560,6 +602,28 @@ Error InstrProfSymtab::addFuncWithName(Function ,
StringRef PGOFuncName) {
return Error::success();
}
+uint64_t InstrProfSymtab::getVTableHashFromAddress(uint64_t Address) {
+ finalizeSymtab();
minglotus-6 wrote:
This is to make sure
@@ -459,6 +472,16 @@ Error InstrProfSymtab::create(Module , bool InLTO) {
if (Error E = addFuncWithName(F, getPGOFuncName(F, InLTO)))
return E;
}
+
+ SmallVector Types;
+ for (GlobalVariable : M.globals()) {
+if (!G.hasName())
+ continue;
+
https://github.com/minglotus-6 commented:
> 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
@@ -490,6 +591,23 @@ Error InstrProfSymtab::addFuncWithName(Function ,
StringRef PGOFuncName) {
return Error::success();
}
+uint64_t InstrProfSymtab::getVTableHashFromAddress(uint64_t Address) {
+ finalizeSymtab();
+ auto It = lower_bound(
+ VTableAddrRangeToMD5Map,
@@ -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
@@ -378,6 +384,13 @@ std::string getPGOFuncName(const Function , bool InLTO,
uint64_t Version) {
return getPGOFuncName(F.getName(), GlobalValue::ExternalLinkage, "");
}
+std::string getPGOName(const GlobalVariable , bool InLTO) {
+ // PGONameMetadata should be set by
@@ -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
https://github.com/minglotus-6 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
@@ -429,20 +439,36 @@ uint64_t ComputeHash(StringRef K);
class InstrProfSymtab {
public:
using AddrHashMap = std::vector>;
+ using RangeHashMap =
+ std::vector, uint64_t>>;
minglotus-6 wrote:
done, and making the struct definition private to class
@@ -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
@@ -560,6 +602,28 @@ Error InstrProfSymtab::addFuncWithName(Function ,
StringRef PGOFuncName) {
return Error::success();
}
+uint64_t InstrProfSymtab::getVTableHashFromAddress(uint64_t Address) {
+ finalizeSymtab();
snehasish wrote:
Like the lookup for
@@ -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
@@ -490,6 +591,23 @@ Error InstrProfSymtab::addFuncWithName(Function ,
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
) {
return Error::success();
}
+template
+Error InstrProfSymtab::create(const FuncNameIterRange ,
+ const VTableNameIterRange ) {
+ for (auto Name : FuncIterRange)
@@ -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.
@@ -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
);
-
- /// Update the symtab by adding \p FuncName
@@ -378,6 +384,13 @@ std::string getPGOFuncName(const Function , bool InLTO,
uint64_t Version) {
return getPGOFuncName(F.getName(), GlobalValue::ExternalLinkage, "");
}
+std::string getPGOName(const GlobalVariable , bool InLTO) {
+ // PGONameMetadata should be set by
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
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
@@ -459,6 +472,16 @@ Error InstrProfSymtab::create(Module , bool InLTO) {
if (Error E = addFuncWithName(F, getPGOFuncName(F, InLTO)))
return E;
}
+
+ SmallVector Types;
+ for (GlobalVariable : M.globals()) {
+if (!G.hasName())
+ continue;
+
@@ -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
minglotus-6 wrote:
Besides the [non-elf
warning](https://github.com/llvm/llvm-project/pull/66825#issuecomment-1968344800),
I did some clean-ups (e.g., [delete braces around one-line
if/loop](https://github.com/llvm/llvm-project/pull/66825#issuecomment-1968344800)
and merges.
Planning to
minglotus-6 wrote:
New changes since last review besides clang-formats
1. Merged profile format change from main branch.
2. In instrumentation pass, emit unsupported warnings for non-ELF object file
formats.
- Updated vtable_profile.ll and add vtable_prof_unsupported.ll as test
cases.
https://github.com/minglotus-6 ready_for_review
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/minglotus-6 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
68 matches
Mail list logo
