Author: Mingming Liu Date: 2025-09-16T12:51:22-07:00 New Revision: c5b558385b956faf99348b3f0de91926061afcfb
URL: https://github.com/llvm/llvm-project/commit/c5b558385b956faf99348b3f0de91926061afcfb DIFF: https://github.com/llvm/llvm-project/commit/c5b558385b956faf99348b3f0de91926061afcfb.diff LOG: Revert "[NFCI][Globals] In GlobalObjects::setSectionPrefix, do conditional up…" This reverts commit 027bccc4692923d0f1ba3d4d970071f747c2255c. Added: Modified: llvm/include/llvm/IR/GlobalObject.h llvm/lib/CodeGen/CodeGenPrepare.cpp llvm/lib/CodeGen/StaticDataAnnotator.cpp llvm/lib/IR/Globals.cpp llvm/lib/Transforms/Instrumentation/MemProfUse.cpp llvm/unittests/IR/CMakeLists.txt Removed: llvm/unittests/IR/GlobalObjectTest.cpp ################################################################################ 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 diff erent 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 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
