[NFC][MemProf] Move types shared between Analysis, ProfileData and ModuleSummary (Core) to a separate header #140505
Conversation
This file contains hidden or 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
This was referenced May 19, 2025
Author
llvmbot
added
PGO
Member
|
@llvm/pr-subscribers-pgo @llvm/pr-subscribers-llvm-analysis Author: Snehasish Kumar (snehasish) ChangesFull diff: https://github.com/llvm/llvm-project/pull/140505.diff 4 Files Affected:
diff --git a/llvm/include/llvm/Analysis/MemoryProfileInfo.h b/llvm/include/llvm/Analysis/MemoryProfileInfo.h
index 1d98f86f50484..33d59efe8d77e 100644
--- a/llvm/include/llvm/Analysis/MemoryProfileInfo.h
+++ b/llvm/include/llvm/Analysis/MemoryProfileInfo.h
@@ -14,7 +14,8 @@
#define LLVM_ANALYSIS_MEMORYPROFILEINFO_H
#include "llvm/IR/Metadata.h"
-#include "llvm/IR/ModuleSummaryIndex.h"
+#include "llvm/ProfileData/MemProfCommon.h"
+#include "llvm/IR/InstrTypes.h"
#include <map>
namespace llvm {
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 65e428a3adea7..77430c5cb5ea1 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -27,6 +27,7 @@
#include "llvm/IR/ConstantRange.h"
#include "llvm/IR/GlobalValue.h"
#include "llvm/IR/Module.h"
+#include "llvm/ProfileData/MemProfCommon.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/InterleavedRange.h"
@@ -306,13 +307,7 @@ template <> struct DenseMapInfo<ValueInfo> {
static unsigned getHashValue(ValueInfo I) { return hash_value(I.getRef()); }
};
-// For optional hinted size reporting, holds a pair of the full stack id
-// (pre-trimming, from the full context in the profile), and the associated
-// total profiled size.
-struct ContextTotalSize {
- uint64_t FullStackId;
- uint64_t TotalSize;
-};
+
/// Summary of memprof callsite metadata.
struct CallsiteInfo {
@@ -350,19 +345,6 @@ inline raw_ostream &operator<<(raw_ostream &OS, const CallsiteInfo &SNI) {
return OS;
}
-// Allocation type assigned to an allocation reached by a given context.
-// More can be added, now this is cold, notcold and hot.
-// Values should be powers of two so that they can be ORed, in particular to
-// track allocations that have different behavior with different calling
-// contexts.
-enum class AllocationType : uint8_t {
- None = 0,
- NotCold = 1,
- Cold = 2,
- Hot = 4,
- All = 7 // This should always be set to the OR of all values.
-};
-
/// Summary of a single MIB in a memprof metadata on allocations.
struct MIBInfo {
// The allocation type for this profiled context.
diff --git a/llvm/include/llvm/ProfileData/MemProfCommon.h b/llvm/include/llvm/ProfileData/MemProfCommon.h
new file mode 100644
index 0000000000000..4097ccb651188
--- /dev/null
+++ b/llvm/include/llvm/ProfileData/MemProfCommon.h
@@ -0,0 +1,44 @@
+//===- MemProfCommon.h - MemProf support ----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains common types used by different parts of the MemProf code.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_PROFILEDATA_MEMPROFCOMMON_H
+#define LLVM_PROFILEDATA_MEMPROFCOMMON_H
+
+#include <cstdint>
+
+namespace llvm {
+
+// For optional hinted size reporting, holds a pair of the full stack id
+// (pre-trimming, from the full context in the profile), and the associated
+// total profiled size.
+struct ContextTotalSize {
+ uint64_t FullStackId;
+ uint64_t TotalSize;
+};
+
+// Allocation type assigned to an allocation reached by a given context.
+// More can be added, now this is cold, notcold and hot.
+// Values should be powers of two so that they can be ORed, in particular to
+// track allocations that have different behavior with different calling
+// contexts.
+enum class AllocationType : uint8_t {
+ None = 0,
+ NotCold = 1,
+ Cold = 2,
+ Hot = 4,
+ All = 7 // This should always be set to the OR of all values.
+};
+
+} // namespace llvm
+
+#endif // LLVM_PROFILEDATA_MEMPROFCOMMON_H
+
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index 5982476f3994e..6538311571529 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -46,6 +46,7 @@
#include "llvm/Transforms/Utils/ModuleUtils.h"
#include <map>
#include <set>
+#include <unordered_set>
using namespace llvm;
using namespace llvm::memprof;
|
Member
|
@llvm/pr-subscribers-llvm-ir Author: Snehasish Kumar (snehasish) ChangesFull diff: https://github.com/llvm/llvm-project/pull/140505.diff 4 Files Affected:
diff --git a/llvm/include/llvm/Analysis/MemoryProfileInfo.h b/llvm/include/llvm/Analysis/MemoryProfileInfo.h
index 1d98f86f50484..33d59efe8d77e 100644
--- a/llvm/include/llvm/Analysis/MemoryProfileInfo.h
+++ b/llvm/include/llvm/Analysis/MemoryProfileInfo.h
@@ -14,7 +14,8 @@
#define LLVM_ANALYSIS_MEMORYPROFILEINFO_H
#include "llvm/IR/Metadata.h"
-#include "llvm/IR/ModuleSummaryIndex.h"
+#include "llvm/ProfileData/MemProfCommon.h"
+#include "llvm/IR/InstrTypes.h"
#include <map>
namespace llvm {
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 65e428a3adea7..77430c5cb5ea1 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -27,6 +27,7 @@
#include "llvm/IR/ConstantRange.h"
#include "llvm/IR/GlobalValue.h"
#include "llvm/IR/Module.h"
+#include "llvm/ProfileData/MemProfCommon.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/InterleavedRange.h"
@@ -306,13 +307,7 @@ template <> struct DenseMapInfo<ValueInfo> {
static unsigned getHashValue(ValueInfo I) { return hash_value(I.getRef()); }
};
-// For optional hinted size reporting, holds a pair of the full stack id
-// (pre-trimming, from the full context in the profile), and the associated
-// total profiled size.
-struct ContextTotalSize {
- uint64_t FullStackId;
- uint64_t TotalSize;
-};
+
/// Summary of memprof callsite metadata.
struct CallsiteInfo {
@@ -350,19 +345,6 @@ inline raw_ostream &operator<<(raw_ostream &OS, const CallsiteInfo &SNI) {
return OS;
}
-// Allocation type assigned to an allocation reached by a given context.
-// More can be added, now this is cold, notcold and hot.
-// Values should be powers of two so that they can be ORed, in particular to
-// track allocations that have different behavior with different calling
-// contexts.
-enum class AllocationType : uint8_t {
- None = 0,
- NotCold = 1,
- Cold = 2,
- Hot = 4,
- All = 7 // This should always be set to the OR of all values.
-};
-
/// Summary of a single MIB in a memprof metadata on allocations.
struct MIBInfo {
// The allocation type for this profiled context.
diff --git a/llvm/include/llvm/ProfileData/MemProfCommon.h b/llvm/include/llvm/ProfileData/MemProfCommon.h
new file mode 100644
index 0000000000000..4097ccb651188
--- /dev/null
+++ b/llvm/include/llvm/ProfileData/MemProfCommon.h
@@ -0,0 +1,44 @@
+//===- MemProfCommon.h - MemProf support ----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains common types used by different parts of the MemProf code.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_PROFILEDATA_MEMPROFCOMMON_H
+#define LLVM_PROFILEDATA_MEMPROFCOMMON_H
+
+#include <cstdint>
+
+namespace llvm {
+
+// For optional hinted size reporting, holds a pair of the full stack id
+// (pre-trimming, from the full context in the profile), and the associated
+// total profiled size.
+struct ContextTotalSize {
+ uint64_t FullStackId;
+ uint64_t TotalSize;
+};
+
+// Allocation type assigned to an allocation reached by a given context.
+// More can be added, now this is cold, notcold and hot.
+// Values should be powers of two so that they can be ORed, in particular to
+// track allocations that have different behavior with different calling
+// contexts.
+enum class AllocationType : uint8_t {
+ None = 0,
+ NotCold = 1,
+ Cold = 2,
+ Hot = 4,
+ All = 7 // This should always be set to the OR of all values.
+};
+
+} // namespace llvm
+
+#endif // LLVM_PROFILEDATA_MEMPROFCOMMON_H
+
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index 5982476f3994e..6538311571529 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -46,6 +46,7 @@
#include "llvm/Transforms/Utils/ModuleUtils.h"
#include <map>
#include <set>
+#include <unordered_set>
using namespace llvm;
using namespace llvm::memprof;
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
snehasish
force-pushed
the
users/snehasish/05-19-_nfc_memprof_move_types_shared_between_analysis_profiledata_and_modulesummary_core_to_a_separate_header
branch
from
d4413c6 to
8751ae1
Compare
teresajohnson
approved these changes
May 19, 2025
Contributor
teresajohnson
left a comment
teresajohnson
left a comment
There was a problem hiding this comment.
lgtm with one question
snehasish
force-pushed
the
users/snehasish/05-16-_nfc_memprof_add_the_llvm_license_text_and_minor_clean_up
branch
from
532d85a to
36aeb3a
Compare
snehasish
force-pushed
the
users/snehasish/05-19-_nfc_memprof_move_types_shared_between_analysis_profiledata_and_modulesummary_core_to_a_separate_header
branch
from
8751ae1 to
305e2bd
Compare
Author
Merge activity
|
snehasish
force-pushed
the
users/snehasish/05-16-_nfc_memprof_add_the_llvm_license_text_and_minor_clean_up
branch
from
36aeb3a to
1b82b50
Compare
Base automatically changed from
users/snehasish/05-16-_nfc_memprof_add_the_llvm_license_text_and_minor_clean_up
to
main
May 19, 2025 23:24
…duleSummary (Core) to a separate header
snehasish
force-pushed
the
users/snehasish/05-19-_nfc_memprof_move_types_shared_between_analysis_profiledata_and_modulesummary_core_to_a_separate_header
branch
from
305e2bd to
f8b6b3c
Compare
snehasish
deleted the
users/snehasish/05-19-_nfc_memprof_move_types_shared_between_analysis_profiledata_and_modulesummary_core_to_a_separate_header
branch
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/5660 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/138/builds/13356 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/205/builds/9796 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/203/builds/11005 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/206/builds/518 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/126/builds/3652 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/76/builds/9719 Here is the relevant piece of the build log for the reference |
chapuni
reviewed
May 20, 2025
Contributor
chapuni
left a comment
chapuni
left a comment
There was a problem hiding this comment.
Looks like you introduced weird deps. I suggest reverting.
|
|
||
| LINK_COMPONENTS | ||
| BitstreamReader | ||
| Core |
Contributor
There was a problem hiding this comment.
Looks like ProfileData still depends on Core.
Author
Fixed in #140650 |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/7114 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/17712 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/17855 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/151/builds/5804 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/97/builds/6708 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/80/builds/13550 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/118/builds/6373 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/130/builds/13243 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/3692 Here is the relevant piece of the build log for the reference |
Contributor
|
This is breaking our internal builds. Please revert. |
Author
Contributor
|
I think the issue is when modules are turned on: fatal error: cyclic dependency in module 'LLVM_Object': LLVM_Object -> LLVM_IR -> LLVM_ProfileData -> LLVM_Object So the fix doesn't address this. |
Author
Thanks, I was able to reproduce this. Taking a look now. |
Contributor
|
@snehasish Thanks for the revert. |
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.
Part of a larger refactoring with the following goals