Revert "[NFCI][Globals] In GlobalObjects::setSectionPrefix, do conditional update if existing prefix is not equivalent to the new one. Returns whether prefix changed." #159159
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
…ional up…" This reverts commit 027bccc.
mingmingl-llvm
deleted the
revert-158460-users/mingmingl-llvm/unittest
branch
Member
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-ir Author: Mingming Liu (mingmingl-llvm) ChangesReverts llvm/llvm-project#158460 due to buildbot failures Full diff: https://github.com/llvm/llvm-project/pull/159159.diff 7 Files Affected:
diff --git a/llvm/include/llvm/IR/GlobalObject.h b/llvm/include/llvm/IR/GlobalObject.h
index e273387807cf6..08a02b42bdc14 100644
--- a/llvm/include/llvm/IR/GlobalObject.h
+++ b/llvm/include/llvm/IR/GlobalObject.h
@@ -121,10 +121,8 @@ class GlobalObject : public GlobalValue {
/// appropriate default object file section.
LLVM_ABI void setSection(StringRef S);
- /// If existing prefix is different from \p Prefix, set it to \p Prefix. If \p
- /// Prefix is empty, the set clears the existing metadata. Returns true if
- /// section prefix changed and false otherwise.
- LLVM_ABI bool setSectionPrefix(StringRef Prefix);
+ /// Set the section prefix for this global object.
+ LLVM_ABI void setSectionPrefix(StringRef Prefix);
/// Get the section prefix for this global object.
LLVM_ABI std::optional<StringRef> getSectionPrefix() const;
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 92d87681c9adc..9db4c9e5e2807 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -583,23 +583,23 @@ bool CodeGenPrepare::_run(Function &F) {
// if requested.
if (BBSectionsGuidedSectionPrefix && BBSectionsProfileReader &&
BBSectionsProfileReader->isFunctionHot(F.getName())) {
- EverMadeChange |= F.setSectionPrefix("hot");
+ F.setSectionPrefix("hot");
} else if (ProfileGuidedSectionPrefix) {
// The hot attribute overwrites profile count based hotness while profile
// counts based hotness overwrite the cold attribute.
// This is a conservative behabvior.
if (F.hasFnAttribute(Attribute::Hot) ||
PSI->isFunctionHotInCallGraph(&F, *BFI))
- EverMadeChange |= F.setSectionPrefix("hot");
+ F.setSectionPrefix("hot");
// If PSI shows this function is not hot, we will placed the function
// into unlikely section if (1) PSI shows this is a cold function, or
// (2) the function has a attribute of cold.
else if (PSI->isFunctionColdInCallGraph(&F, *BFI) ||
F.hasFnAttribute(Attribute::Cold))
- EverMadeChange |= F.setSectionPrefix("unlikely");
+ F.setSectionPrefix("unlikely");
else if (ProfileUnknownInSpecialSection && PSI->hasPartialSampleProfile() &&
PSI->isFunctionHotnessUnknown(F))
- EverMadeChange |= F.setSectionPrefix("unknown");
+ F.setSectionPrefix("unknown");
}
/// This optimization identifies DIV instructions that can be
diff --git a/llvm/lib/CodeGen/StaticDataAnnotator.cpp b/llvm/lib/CodeGen/StaticDataAnnotator.cpp
index 53a9ab4dbda02..2d9b489a80acb 100644
--- a/llvm/lib/CodeGen/StaticDataAnnotator.cpp
+++ b/llvm/lib/CodeGen/StaticDataAnnotator.cpp
@@ -91,7 +91,8 @@ bool StaticDataAnnotator::runOnModule(Module &M) {
if (SectionPrefix.empty())
continue;
- Changed |= GV.setSectionPrefix(SectionPrefix);
+ GV.setSectionPrefix(SectionPrefix);
+ Changed = true;
}
return Changed;
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 1a7a5c5fbad6b..11d33e262fecb 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -288,22 +288,10 @@ void GlobalObject::setSection(StringRef S) {
setGlobalObjectFlag(HasSectionHashEntryBit, !S.empty());
}
-bool GlobalObject::setSectionPrefix(StringRef Prefix) {
- StringRef ExistingPrefix;
- if (std::optional<StringRef> MaybePrefix = getSectionPrefix())
- ExistingPrefix = *MaybePrefix;
-
- if (ExistingPrefix == Prefix)
- return false;
-
- if (Prefix.empty()) {
- setMetadata(LLVMContext::MD_section_prefix, nullptr);
- return true;
- }
+void GlobalObject::setSectionPrefix(StringRef Prefix) {
MDBuilder MDB(getContext());
setMetadata(LLVMContext::MD_section_prefix,
MDB.createGlobalObjectSectionPrefix(Prefix));
- return true;
}
std::optional<StringRef> GlobalObject::getSectionPrefix() const {
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
index c86092bd51eda..ecb2f2dbc552b 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp
@@ -848,12 +848,13 @@ bool MemProfUsePass::annotateGlobalVariables(
// So we just print out the static data section prefix in LLVM_DEBUG.
if (Record && Record->AccessCount > 0) {
++NumOfMemProfHotGlobalVars;
- Changed |= GVar.setSectionPrefix("hot");
+ GVar.setSectionPrefix("hot");
+ Changed = true;
LLVM_DEBUG(dbgs() << "Global variable " << Name
<< " is annotated as hot\n");
} else if (DataAccessProf->isKnownColdSymbol(Name)) {
++NumOfMemProfColdGlobalVars;
- Changed |= GVar.setSectionPrefix("unlikely");
+ GVar.setSectionPrefix("unlikely");
Changed = true;
LLVM_DEBUG(dbgs() << "Global variable " << Name
<< " is annotated as unlikely\n");
diff --git a/llvm/unittests/IR/CMakeLists.txt b/llvm/unittests/IR/CMakeLists.txt
index d62ce66ef9d34..8b7bd3997ea27 100644
--- a/llvm/unittests/IR/CMakeLists.txt
+++ b/llvm/unittests/IR/CMakeLists.txt
@@ -28,7 +28,6 @@ add_llvm_unittest(IRTests
DominatorTreeBatchUpdatesTest.cpp
DroppedVariableStatsIRTest.cpp
FunctionTest.cpp
- GlobalObjectTest.cpp
PassBuilderCallbacksTest.cpp
IRBuilderTest.cpp
InstructionsTest.cpp
diff --git a/llvm/unittests/IR/GlobalObjectTest.cpp b/llvm/unittests/IR/GlobalObjectTest.cpp
deleted file mode 100644
index 0e16d01e759de..0000000000000
--- a/llvm/unittests/IR/GlobalObjectTest.cpp
+++ /dev/null
@@ -1,80 +0,0 @@
-//===- GlobalObjectTest.cpp - Global object unit tests --------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#include "llvm/IR/GlobalObject.h"
-#include "llvm/AsmParser/Parser.h"
-#include "llvm/IR/Module.h"
-#include "llvm/Support/SourceMgr.h"
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
-using namespace llvm;
-namespace {
-using testing::Eq;
-using testing::Optional;
-using testing::StrEq;
-
-static std::unique_ptr<Module> parseIR(LLVMContext &C, const char *IR) {
- SMDiagnostic Err;
- std::unique_ptr<Module> Mod = parseAssemblyString(IR, Err, C);
- if (!Mod)
- Err.print("GlobalObjectTests", errs());
- return Mod;
-}
-
-static LLVMContext C;
-static std::unique_ptr<Module> M;
-
-class GlobalObjectTest : public testing::Test {
-public:
- static void SetUpTestSuite() {
- M = parseIR(C, R"(
-@foo = global i32 3, !section_prefix !0
-@bar = global i32 0
-
-!0 = !{!"section_prefix", !"hot"}
-)");
- }
-};
-
-TEST_F(GlobalObjectTest, SectionPrefix) {
- GlobalVariable *Foo = M->getGlobalVariable("foo");
-
- // Initial section prefix is hot.
- ASSERT_NE(Foo, nullptr);
- ASSERT_THAT(Foo->getSectionPrefix(), Optional(StrEq("hot")));
-
- // Test that set method returns false since existing section prefix is hot.
- EXPECT_FALSE(Foo->setSectionPrefix("hot"));
-
- // Set prefix from hot to unlikely.
- Foo->setSectionPrefix("unlikely");
- EXPECT_THAT(Foo->getSectionPrefix(), Optional(StrEq("unlikely")));
-
- // Set prefix to empty is the same as clear.
- Foo->setSectionPrefix("");
- // Test that section prefix is cleared.
- EXPECT_THAT(Foo->getSectionPrefix(), Eq(std::nullopt));
-
- GlobalVariable *Bar = M->getGlobalVariable("bar");
-
- // Initial section prefix is empty.
- ASSERT_NE(Bar, nullptr);
- ASSERT_THAT(Bar->getSectionPrefix(), Eq(std::nullopt));
-
- // Test that set method returns false since Bar doesn't have prefix metadata.
- EXPECT_FALSE(Bar->setSectionPrefix(""));
-
- // Set from empty to hot.
- EXPECT_TRUE(Bar->setSectionPrefix("hot"));
- EXPECT_THAT(Bar->getSectionPrefix(), Optional(StrEq("hot")));
-
- // Test that set method returns true and section prefix is cleared.
- EXPECT_TRUE(Bar->setSectionPrefix(""));
- EXPECT_THAT(Bar->getSectionPrefix(), Eq(std::nullopt));
-}
-} // namespace
|
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/14677 Here is the relevant piece of the build log for the reference |
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.
Reverts #158460 due to buildbot failures